diff --git a/CHANGES b/CHANGES index de03fd7..66b3133 100644 --- a/CHANGES +++ b/CHANGES @@ -5,6 +5,8 @@ Version [unreleased] -------------------- - Added check for how `activate` is called. +- Added `--with-certifi` to download packages with the certifi certificate + bundle `#388 `_ Version 1.3.1 ------------- diff --git a/README.rst b/README.rst index 09880b8..c88bf31 100644 --- a/README.rst +++ b/README.rst @@ -330,6 +330,16 @@ Other options ``--ignore_ssl_certs`` Ignore SSL certificates for package downloads. **UNSAFE - use at your own risk**. +``--with-certifi`` + Use the `certifi `_ certificate bundle for + package downloads instead of the system certificate store. Useful when the + system store is missing or outdated. If certifi is not installed, a warning is + printed and the system store is used. Ignored when ``--ignore_ssl_certs`` is + given. The same result can be achieved without this option by pointing + ``SSL_CERT_FILE`` at the bundle:: + + $ SSL_CERT_FILE=$(python -c 'import certifi; print(certifi.where())') nodeenv env + ``--version`` Show program version and exit. @@ -351,6 +361,7 @@ These are the available options and their defaults:: make = 'make' prebuilt = True ignore_ssl_certs = False + with_certifi = False mirror = None Alternatives diff --git a/nodeenv.py b/nodeenv.py index 7158c09..999770f 100644 --- a/nodeenv.py +++ b/nodeenv.py @@ -59,6 +59,9 @@ is_CYGWIN = platform.system().startswith(('CYGWIN', 'MSYS')) ignore_ssl_certs = False +# SSL context backed by the certifi bundle, built once by main() +# when --with-certifi is given and certifi is importable +certifi_context = None # --------------------------------------------------------- # Utils @@ -101,6 +104,7 @@ class Config(object): make = 'make' prebuilt = True ignore_ssl_certs = False + with_certifi = False mirror = None @classmethod @@ -368,6 +372,12 @@ def make_parser(): action='store_true', default=Config.ignore_ssl_certs, help='Ignore certificates for package downloads. - UNSAFE -') + parser.add_argument( + '--with-certifi', dest='with_certifi', + action='store_true', default=Config.with_certifi, + help='Use the certifi certificate bundle for package downloads, ' + 'if certifi is installed. Ignored with --ignore_ssl_certs.') + parser.add_argument( metavar='DEST_DIR', dest='env_dir', nargs='?', help='Destination directory') @@ -644,6 +654,24 @@ def download_node_src(node_url, src_dir, args): archive.extractall(src_dir, extract_list) +def make_certifi_context(): + """ + Build an SSL context backed by the certifi bundle. + + Returns None if certifi is not installed, so that downloads keep + using the system certificate store. + """ + try: + import certifi + except ImportError: + logger.warning( + 'certifi is not installed, --with-certifi is ignored: ' + 'falling back to the system certificate store') + return None + + return ssl.create_default_context(cafile=certifi.where()) + + def urlopen(url): home_url = "https://github.com/ekalinin/nodeenv/" headers = {'User-Agent': 'nodeenv/%s (%s)' % (nodeenv_version, home_url)} @@ -654,6 +682,11 @@ def urlopen(url): context = ssl.SSLContext(ssl.PROTOCOL_TLS) context.verify_mode = ssl.CERT_NONE return urllib2.urlopen(req, context=context) + + # Use certifi certificates if they were requested and are available + if certifi_context is not None: + return urllib2.urlopen(req, context=certifi_context) + return urllib2.urlopen(req) # --------------------------------------------------------- @@ -1132,8 +1165,11 @@ def main(): global src_base_url global ignore_ssl_certs + global certifi_context ignore_ssl_certs = args.ignore_ssl_certs + if args.with_certifi and not ignore_ssl_certs: + certifi_context = make_certifi_context() src_domain = None if args.mirror: diff --git a/setup.py b/setup.py index 2977546..0ad5e07 100644 --- a/setup.py +++ b/setup.py @@ -37,6 +37,7 @@ def read_file(file_name): author='Eugene Kalinin', author_email='e.v.kalinin@gmail.com', install_requires=[], + extras_require={'certifi': ['certifi']}, python_requires=( ">=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,!=3.6.*" ), diff --git a/tests/nodeenv_test.py b/tests/nodeenv_test.py index b834c3f..4d812b4 100644 --- a/tests/nodeenv_test.py +++ b/tests/nodeenv_test.py @@ -12,6 +12,7 @@ import sys import sysconfig import platform +import ssl import zipfile try: @@ -1619,3 +1620,60 @@ def test_install_npm_win_zip_extraction(self): # Verify extraction mock_zip.extractall.assert_called_once_with(src_dir) + + +class TestCertifi: + """Tests for the --with-certifi option""" + + def test_urlopen_without_certifi(self): + """No SSL context is passed when certifi is not in use""" + with mock.patch.object(nodeenv, 'ignore_ssl_certs', False), \ + mock.patch.object(nodeenv, 'certifi_context', None), \ + mock.patch.object(nodeenv.urllib2, 'urlopen') as m_urlopen: + nodeenv.urlopen('https://nodejs.org/dist/index.json') + + assert m_urlopen.call_args[1] == {} + + def test_urlopen_with_certifi(self): + """The context built by main() is reused for every download""" + with mock.patch.object(nodeenv, 'ignore_ssl_certs', False), \ + mock.patch.object(nodeenv, 'certifi_context', + mock.sentinel.certifi_context), \ + mock.patch.object(nodeenv.urllib2, 'urlopen') as m_urlopen: + nodeenv.urlopen('https://nodejs.org/dist/index.json') + + context = m_urlopen.call_args[1]['context'] + assert context is mock.sentinel.certifi_context + + def test_urlopen_ignore_ssl_certs_wins(self): + """--ignore_ssl_certs takes precedence over --with-certifi""" + with mock.patch.object(nodeenv, 'ignore_ssl_certs', True), \ + mock.patch.object(nodeenv, 'certifi_context', + mock.sentinel.certifi_context), \ + mock.patch.object(nodeenv.urllib2, 'urlopen') as m_urlopen: + nodeenv.urlopen('https://nodejs.org/dist/index.json') + + assert m_urlopen.call_args[1]['context'].verify_mode == ssl.CERT_NONE + + def test_make_certifi_context(self): + certifi = mock.Mock() + certifi.where.return_value = '/path/to/cacert.pem' + + with mock.patch.dict(sys.modules, {'certifi': certifi}), \ + mock.patch.object(nodeenv.ssl, + 'create_default_context') as m_context: + assert nodeenv.make_certifi_context() is m_context.return_value + + m_context.assert_called_once_with(cafile='/path/to/cacert.pem') + + def test_make_certifi_context_without_certifi(self): + """A missing certifi is reported instead of silently ignored""" + with mock.patch.dict(sys.modules, {'certifi': None}), \ + mock.patch.object(nodeenv.logger, 'warning') as m_warning: + assert nodeenv.make_certifi_context() is None + + assert 'certifi is not installed' in m_warning.call_args[0][0] + + def test_with_certifi_is_configurable(self): + """with_certifi can be set from the config file, like other options""" + assert 'with_certifi' in nodeenv.Config._default