Patchwork [9,of,9,V3] sslutil: print the fingerprint from the last hash used

login
register
mail settings
Submitter Gregory Szorc
Date June 2, 2016, 3:16 a.m.
Message ID <49700bf26fb680531e84.1464837383@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15335/
State Superseded
Commit 1b3a0b0c414faa3d6d4dbcf4c5abbbe18aa9efd4
Delegated to: Yuya Nishihara
Headers show

Comments

Gregory Szorc - June 2, 2016, 3:16 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1464836833 25200
#      Wed Jun 01 20:07:13 2016 -0700
# Node ID 49700bf26fb680531e84197f49684c7431f905a9
# Parent  8f3c8829b5bbaeb6c7f344126d1b1d4135d15803
sslutil: print the fingerprint from the last hash used

Before, we would always print the unprefixed SHA-1 fingerprint when
fingerprint comparison failed. Now, we print the fingerprint of the
last hash used, including the prefix if necessary. This helps ensure
that the printed hash type matches what is in the user configuration.

There are still some cases where this can print a mismatched hash type.
e.g. if there are both SHA-1 and SHA-256 fingerprints in the config,
we could print a SHA-1 hash if it comes after the SHA-256 hash. But
I'm inclined to ignore this edge case.

While I was here, the "section" variable assignment has been moved to
just above where it is used because it is now only needed for this
error message and it makes the code easier to read.
Yuya Nishihara - June 3, 2016, 2:35 p.m.
On Wed, 01 Jun 2016 20:16:23 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1464836833 25200
> #      Wed Jun 01 20:07:13 2016 -0700
> # Node ID 49700bf26fb680531e84197f49684c7431f905a9
> # Parent  8f3c8829b5bbaeb6c7f344126d1b1d4135d15803
> sslutil: print the fingerprint from the last hash used

> -    legacyfingerprint = fmtfingerprint(peerfingerprints['sha1'])
>      nicefingerprint = 'sha256:%s' % fmtfingerprint(peerfingerprints['sha256'])
>  
> -    if settings['legacyfingerprint']:
> -        section = 'hostfingerprint'
> -    else:
> -        section = 'hostsecurity'
> -
>      if settings['certfingerprints']:
>          for hash, fingerprint in settings['certfingerprints']:
>              if peerfingerprints[hash].lower() == fingerprint:
>                  ui.debug('%s certificate matched fingerprint %s:%s\n' %
>                           (host, hash, fmtfingerprint(fingerprint)))
>                  return
>  
> +        # Pinned fingerprint didn't match. This is a fatal error.
> +        nice = fmtfingerprint(peerfingerprints[hash])
> +        if settings['legacyfingerprint']:
> +            section = 'hostfingerprint'
> +        else:
> +            section = 'hostsecurity'
> +            nice = '%s:%s' % (hash, nice)

I think it's better to use peerfingerprints['sha1'] explicitly for legacy
'hostfingerprint'. It's unclear to me to presume 'hostfingerprint' is appended
last and the loop ends with hash='sha1'.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -372,33 +372,34 @@  def validatesocket(sock):
         'sha1': util.sha1(peercert).hexdigest(),
         'sha256': util.sha256(peercert).hexdigest(),
         'sha512': util.sha512(peercert).hexdigest(),
     }
 
     def fmtfingerprint(s):
         return ':'.join([s[x:x + 2] for x in range(0, len(s), 2)])
 
-    legacyfingerprint = fmtfingerprint(peerfingerprints['sha1'])
     nicefingerprint = 'sha256:%s' % fmtfingerprint(peerfingerprints['sha256'])
 
-    if settings['legacyfingerprint']:
-        section = 'hostfingerprint'
-    else:
-        section = 'hostsecurity'
-
     if settings['certfingerprints']:
         for hash, fingerprint in settings['certfingerprints']:
             if peerfingerprints[hash].lower() == fingerprint:
                 ui.debug('%s certificate matched fingerprint %s:%s\n' %
                          (host, hash, fmtfingerprint(fingerprint)))
                 return
 
+        # Pinned fingerprint didn't match. This is a fatal error.
+        nice = fmtfingerprint(peerfingerprints[hash])
+        if settings['legacyfingerprint']:
+            section = 'hostfingerprint'
+        else:
+            section = 'hostsecurity'
+            nice = '%s:%s' % (hash, nice)
         raise error.Abort(_('certificate for %s has unexpected '
-                           'fingerprint %s') % (host, legacyfingerprint),
+                           'fingerprint %s') % (host, nice),
                           hint=_('check %s configuration') % section)
 
     if not sock._hgstate['caloaded']:
         ui.warn(_('warning: certificate for %s not verified '
                   '(set hostsecurity.%s:certfingerprints=%s or web.cacerts '
                   'config settings)\n') % (host, host, nicefingerprint))
         return
 
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -309,17 +309,17 @@  Fingerprints
 - multiple fingerprints specified and none match
 
   $ hg --config 'hostfingerprints.localhost=deadbeefdeadbeefdeadbeefdeadbeefdeadbeef, aeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' -R copy-pull id https://localhost:$HGPORT/ --insecure
   abort: certificate for localhost has unexpected fingerprint 91:4f:1a:ff:87:24:9c:09:b6:85:9b:88:b1:90:6d:30:75:64:91:ca
   (check hostfingerprint configuration)
   [255]
 
   $ hg --config 'hostsecurity.localhost:fingerprints=sha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbeef, sha1:aeadbeefdeadbeefdeadbeefdeadbeefdeadbeef' -R copy-pull id https://localhost:$HGPORT/
-  abort: certificate for localhost has unexpected fingerprint 91:4f:1a:ff:87:24:9c:09:b6:85:9b:88:b1:90:6d:30:75:64:91:ca
+  abort: certificate for localhost has unexpected fingerprint sha1:91:4f:1a:ff:87:24:9c:09:b6:85:9b:88:b1:90:6d:30:75:64:91:ca
   (check hostsecurity configuration)
   [255]
 
 - fails when cert doesn't match hostname (port is ignored)
   $ hg -R copy-pull id https://localhost:$HGPORT1/ --config hostfingerprints.localhost=914f1aff87249c09b6859b88b1906d30756491ca
   abort: certificate for localhost has unexpected fingerprint 28:ff:71:bf:65:31:14:23:ad:62:92:b4:0e:31:99:18:fc:83:e3:9b
   (check hostfingerprint configuration)
   [255]