Patchwork [5,of,5] sslutil: document and slightly refactor validation logic

login
register
mail settings
Submitter Gregory Szorc
Date April 10, 2016, 6:04 p.m.
Message ID <429e6599133e741c020d.1460311477@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/14491/
State Accepted
Headers show

Comments

Gregory Szorc - April 10, 2016, 6:04 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1460311378 25200
#      Sun Apr 10 11:02:58 2016 -0700
# Node ID 429e6599133e741c020dbc79c580f30a8b47c3b8
# Parent  3d9c3f8bfed01b5ab163bc95180576a79e6016ef
sslutil: document and slightly refactor validation logic

This main purpose of this patch is to make it clearer that fingerprint
pinning takes precedence over CA verification. This will make
subsequent refactoring to the validation code easier to read.
Pierre-Yves David - April 11, 2016, 6:03 a.m.
On 04/10/2016 11:04 AM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1460311378 25200
> #      Sun Apr 10 11:02:58 2016 -0700
> # Node ID 429e6599133e741c020dbc79c580f30a8b47c3b8
> # Parent  3d9c3f8bfed01b5ab163bc95180576a79e6016ef
> sslutil: document and slightly refactor validation logic

Pushed, thanks.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -259,47 +259,53 @@  def sslkwargs(ui, host):
 
 class validator(object):
     def __init__(self, ui, host):
         self.ui = ui
         self.host = host
 
     def __call__(self, sock, strict=False):
         host = self.host
-        cacerts = self.ui.config('web', 'cacerts')
-        hostfingerprints = self.ui.configlist('hostfingerprints', host)
 
         if not sock.cipher(): # work around http://bugs.python.org/issue13721
             raise error.Abort(_('%s ssl connection error') % host)
         try:
             peercert = sock.getpeercert(True)
             peercert2 = sock.getpeercert()
         except AttributeError:
             raise error.Abort(_('%s ssl connection error') % host)
 
         if not peercert:
             raise error.Abort(_('%s certificate error: '
                                'no certificate received') % host)
+
+        # If a certificate fingerprint is pinned, use it and only it to
+        # validate the remote cert.
+        hostfingerprints = self.ui.configlist('hostfingerprints', host)
         peerfingerprint = util.sha1(peercert).hexdigest()
         nicefingerprint = ":".join([peerfingerprint[x:x + 2]
             for x in xrange(0, len(peerfingerprint), 2)])
         if hostfingerprints:
             fingerprintmatch = False
             for hostfingerprint in hostfingerprints:
                 if peerfingerprint.lower() == \
                         hostfingerprint.replace(':', '').lower():
                     fingerprintmatch = True
                     break
             if not fingerprintmatch:
                 raise error.Abort(_('certificate for %s has unexpected '
                                    'fingerprint %s') % (host, nicefingerprint),
                                  hint=_('check hostfingerprint configuration'))
             self.ui.debug('%s certificate matched fingerprint %s\n' %
                           (host, nicefingerprint))
-        elif cacerts != '!':
+            return
+
+        # No pinned fingerprint. Establish trust by looking at the CAs.
+        cacerts = self.ui.config('web', 'cacerts')
+        if cacerts != '!':
             msg = _verifycert(peercert2, host)
             if msg:
                 raise error.Abort(_('%s certificate error: %s') % (host, msg),
                                  hint=_('configure hostfingerprint %s or use '
                                         '--insecure to connect insecurely') %
                                       nicefingerprint)
             self.ui.debug('%s certificate successfully verified\n' % host)
         elif strict: