Rev 6468: (vila) Provide an ``ssl.ca_certs`` default value for supported platforms. in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/2.5/

Patch Queue Manager pqm at pqm.ubuntu.com
Tue Jan 31 18:25:58 UTC 2012


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/2.5/

------------------------------------------------------------
revno: 6468 [merge]
revision-id: pqm at pqm.ubuntu.com-20120131182557-ywfu6m6vm89u525v
parent: pqm at pqm.ubuntu.com-20120131175852-emlorbqnif85eq89
parent: v.ladeuil+lp at free.fr-20120131170022-gbthyl1gbocu3s5m
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: 2.5
timestamp: Tue 2012-01-31 18:25:57 +0000
message:
  (vila) Provide an ``ssl.ca_certs`` default value for supported platforms.
   (Vincent Ladeuil)
modified:
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/tests/test_https_urllib.py test_https_urllib.py-20111220105828-v3g3fknv8inj2jqv-1
  bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
  doc/en/release-notes/bzr-2.5.txt bzr2.5.txt-20110708125756-587p0hpw7oke4h05-1
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2012-01-27 15:47:12 +0000
+++ b/bzrlib/errors.py	2012-01-31 18:25:57 +0000
@@ -1761,7 +1761,8 @@
 
 class ConfigOptionValueError(BzrError):
 
-    _fmt = """Bad value "%(value)s" for option "%(name)s"."""
+    _fmt = ('Bad value "%(value)s" for option "%(name)s".\n'
+            'See ``bzr help %(name)s``')
 
     def __init__(self, name, value):
         BzrError.__init__(self, name=name, value=value)

=== modified file 'bzrlib/tests/test_https_urllib.py'
--- a/bzrlib/tests/test_https_urllib.py	2012-01-19 15:27:47 +0000
+++ b/bzrlib/tests/test_https_urllib.py	2012-01-31 16:36:53 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2011 Canonical Ltd
+# Copyright (C) 2011,2012 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -41,18 +41,10 @@
     def get_stack(self, content):
         return config.MemoryStack(content.encode('utf-8'))
 
-    def test_default_raises_value_error(self):
-        stack = self.get_stack("")
-        self.overrideAttr(_urllib2_wrappers, "DEFAULT_CA_PATH",
-                "/i-do-not-exist")
-        self.assertRaises(ValueError, stack.get, 'ssl.ca_certs')
-
     def test_default_exists(self):
-        self.build_tree(['cacerts.pem'])
+        """Check that the default we provide exists for the tested platform."""
         stack = self.get_stack("")
-        path = os.path.join(self.test_dir, "cacerts.pem")
-        self.overrideAttr(_urllib2_wrappers, "DEFAULT_CA_PATH", path)
-        self.assertEquals(path, stack.get('ssl.ca_certs'))
+        self.assertPathExists(stack.get('ssl.ca_certs'))
 
     def test_specified(self):
         self.build_tree(['cacerts.pem'])
@@ -61,14 +53,15 @@
         self.assertEquals(path, stack.get('ssl.ca_certs'))
 
     def test_specified_doesnt_exist(self):
-        path = os.path.join(self.test_dir, "nonexisting.pem")
-        stack = self.get_stack("ssl.ca_certs = %s\n" % path)
+        stack = self.get_stack('')
+        # Disable the default value mechanism to force the behavior we want
+        self.overrideAttr(_urllib2_wrappers.opt_ssl_ca_certs, 'default',
+                          os.path.join(self.test_dir, u"nonexisting.pem"))
         self.warnings = []
         def warning(*args):
             self.warnings.append(args[0] % args[1:])
         self.overrideAttr(trace, 'warning', warning)
-        self.assertEquals(_urllib2_wrappers.DEFAULT_CA_PATH,
-                          stack.get('ssl.ca_certs'))
+        self.assertEquals(None, stack.get('ssl.ca_certs'))
         self.assertLength(1, self.warnings)
         self.assertContainsRe(self.warnings[0],
                               "is not valid for \"ssl.ca_certs\"")
@@ -83,8 +76,6 @@
     def test_from_string(self):
         stack = config.MemoryStack("ssl.cert_reqs = none\n")
         self.assertEquals(ssl.CERT_NONE, stack.get("ssl.cert_reqs"))
-        stack = config.MemoryStack("ssl.cert_reqs = optional\n")
-        self.assertEquals(ssl.CERT_OPTIONAL, stack.get("ssl.cert_reqs"))
         stack = config.MemoryStack("ssl.cert_reqs = required\n")
         self.assertEquals(ssl.CERT_REQUIRED, stack.get("ssl.cert_reqs"))
         stack = config.MemoryStack("ssl.cert_reqs = invalid\n")

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2012-01-20 09:19:14 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2012-01-31 17:00:22 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2011 Canonical Ltd
+# Copyright (C) 2006-2012 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -74,14 +74,36 @@
 import ssl
 """)
 
-DEFAULT_CA_PATH = u"/etc/ssl/certs/ca-certificates.crt"
 
+# Note for packagers: if there is no package providing certs for your platform,
+# the curl project produces http://curl.haxx.se/ca/cacert.pem weekly.
+_ssl_ca_certs_known_locations = [
+    u'/etc/ssl/certs/ca-certificates.crt', # Ubuntu/debian/gentoo
+    u'/etc/pki/tls/certs/ca-bundle.crt', # Fedora/CentOS/RH
+    u'/etc/ssl/ca-bundle.pem', # OpenSuse
+    u'/etc/ssl/cert.pem', # OpenSuse
+    u"/usr/local/share/certs/ca-root-nss.crt", # FreeBSD
+    # XXX: Needs checking, can't trust the interweb ;) -- vila 2012-01-25
+    u'/etc/openssl/certs/ca-certificates.crt', # Solaris
+    ]
 
 def default_ca_certs():
-    if not os.path.exists(DEFAULT_CA_PATH):
-        raise ValueError("default ca certs path %s does not exist" %
-            DEFAULT_CA_PATH)
-    return DEFAULT_CA_PATH
+    if sys.platform == 'win32':
+        return os.path.join(os.path.dirname(sys.executable), u"ca_bundle.crt")
+    elif sys.platform == 'darwin':
+        # FIXME: Needs some default value for osx, waiting for osx installers
+        # guys feedback -- vila 2012-01-25
+        pass
+    else:
+        # Try known locations for friendly OSes providing the root certificates
+        # without making them hard to use for any https client.
+        for path in _ssl_ca_certs_known_locations:
+            if os.path.exists(path):
+                # First found wins
+                return path
+    # A default path that makes sense and will be mentioned in the error
+    # presented to the user, even if not correct for all platforms
+    return _ssl_ca_certs_known_locations[0]
 
 
 def ca_certs_from_store(path):
@@ -90,16 +112,11 @@
     return path
 
 
-def default_cert_reqs():
-    return u"required"
-
-
 def cert_reqs_from_store(unicode_str):
     import ssl
     try:
         return {
             "required": ssl.CERT_REQUIRED,
-            "optional": ssl.CERT_OPTIONAL,
             "none": ssl.CERT_NONE
             }[unicode_str]
     except KeyError:
@@ -112,10 +129,15 @@
         invalid='warning',
         help="""\
 Path to certification authority certificates to trust.
+
+This should be a valid path to a bundle containing all root Certificate
+Authorities used to verify an https server certificate.
+
+Use ssl.cert_reqs=none to disable certificate verification.
 """)
 
 opt_ssl_cert_reqs = config.Option('ssl.cert_reqs',
-        default=default_cert_reqs,
+        default=u"required",
         from_unicode=cert_reqs_from_store,
         invalid='error',
         help="""\
@@ -123,8 +145,7 @@
 
 Possible values:
  * none: Certificates ignored
- * optional: Certificates not required, but validated if provided
- * required: Certificates required, and validated
+ * required: Certificates required and validated
 """)
 
 checked_kerberos = False
@@ -448,34 +469,34 @@
     def connect_to_origin(self):
         # FIXME JRV 2011-12-18: Use location config here?
         config_stack = config.GlobalStack()
-        if self.ca_certs is None:
-            ca_certs = config_stack.get('ssl.ca_certs')
-        else:
-            ca_certs = self.ca_certs
         cert_reqs = config_stack.get('ssl.cert_reqs')
         if cert_reqs == ssl.CERT_NONE:
-            trace.warning("not checking SSL certificates for %s: %d",
+            trace.warning("Not checking SSL certificate for %s: %d",
                 self.host, self.port)
+            ca_certs = None
         else:
+            if self.ca_certs is None:
+                ca_certs = config_stack.get('ssl.ca_certs')
+            else:
+                ca_certs = self.ca_certs
             if ca_certs is None:
                 trace.warning(
-                    "no valid trusted SSL CA certificates file set. See "
+                    "No valid trusted SSL CA certificates file set. See "
                     "'bzr help ssl.ca_certs' for more information on setting "
-                    "trusted CA's.")
+                    "trusted CAs.")
         try:
             ssl_sock = ssl.wrap_socket(self.sock, self.key_file, self.cert_file,
                 cert_reqs=cert_reqs, ca_certs=ca_certs)
         except ssl.SSLError, e:
-            if e.errno != ssl.SSL_ERROR_SSL:
-                raise
             trace.note(
-                "To disable SSL certificate verification, use "
-                "-Ossl.cert_reqs=none. See ``bzr help ssl.ca_certs`` for "
-                "more information on specifying trusted CA certificates.")
+                "\n"
+                "See `bzr help ssl.ca_certs` for how to specify trusted CA"
+                "certificates.\n"
+                "Pass -Ossl.cert_reqs=none to disable certificate "
+                "verification entirely.\n")
             raise
-        peer_cert = ssl_sock.getpeercert()
-        if (cert_reqs == ssl.CERT_REQUIRED or
-            (cert_reqs == ssl.CERT_OPTIONAL and peer_cert)):
+        if cert_reqs == ssl.CERT_REQUIRED:
+            peer_cert = ssl_sock.getpeercert()
             match_hostname(peer_cert, self.host)
 
         # Wrap the ssl socket before anybody use it

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt	2012-01-31 17:28:26 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt	2012-01-31 18:25:57 +0000
@@ -56,6 +56,10 @@
 * ``bzr branch`` now fetches revisions when branching into an empty
   control directory. (Jelmer Vernooij, #905594)
 
+* A sane default is provided for ``ssl.ca_certs`` which should points to the
+  Certificate Authority bundle for supported platforms.
+  (Vincent Ladeuil, #920455)
+
 * ``bzr branch`` generates correct target branch locations again if not
   specified. (Jelmer Vernooij, #919218)
 




More information about the bazaar-commits mailing list