From d56ab83f9756045bae2196812eff42d32b4ec009 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jan 30 2020 19:45:35 +0000 Subject: [PATCH 1/4] Move the pool and hash tables for SNI into mod_nss.c The variable was effectively being re-declared with every import of mod_nss.h. Use extern instead. --- diff --git a/mod_nss.c b/mod_nss.c index 6a15501..d24b3b3 100644 --- a/mod_nss.c +++ b/mod_nss.c @@ -17,6 +17,11 @@ #include #include "sslerr.h" +/* pool and hash to store ServerName and NSSNickname pairs for SNI */ +apr_pool_t *mp; +apr_hash_t *ht; + + /* * the table of configuration directives we provide */ diff --git a/mod_nss.h b/mod_nss.h index 0a894ef..980fc24 100644 --- a/mod_nss.h +++ b/mod_nss.h @@ -375,10 +375,6 @@ typedef struct { * for cipher definitions see nss_engine_cipher.h */ -/* pool and hash to store ServerName and NSSNickname pairs for SNI */ -apr_pool_t *mp; -apr_hash_t *ht; - /* Compatibility between Apache 2.0.x and 2.2.x. The numeric version of * the version first appeared in Apache 2.0.56-dev. I picked 2.0.55 as it * is the last version without this define. This is used for more than just diff --git a/nss_engine_init.c b/nss_engine_init.c index 0793479..61e2f49 100644 --- a/nss_engine_init.c +++ b/nss_engine_init.c @@ -45,6 +45,11 @@ char* INTERNAL_TOKEN_NAME = "internal "; extern cipher_properties ciphers_def[]; +/* pool and hash to store ServerName and NSSNickname pairs for SNI */ +extern apr_pool_t *mp; +extern apr_hash_t *ht; + + /* See if a uid or gid can read a file or directory at a given path. * * Return 0 on failure or file doesn't exist diff --git a/nss_util.c b/nss_util.c index cbe65de..c63a69b 100644 --- a/nss_util.c +++ b/nss_util.c @@ -17,6 +17,10 @@ #include "ap_mpm.h" #include "apr_thread_mutex.h" +/* pool and hash to store ServerName and NSSNickname pairs for SNI */ +extern apr_pool_t *mp; +extern apr_hash_t *ht; + /* _________________________________________________________________ ** ** Utility Functions From 32f272303444b70f7ae0dd91c7e5092a7e983e64 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jan 30 2020 19:46:44 +0000 Subject: [PATCH 2/4] Fixes to unit tests so they pass with openssl 1.1.1a --- diff --git a/gencert.in b/gencert.in index 0fd1c67..2b06ad0 100755 --- a/gencert.in +++ b/gencert.in @@ -38,6 +38,11 @@ getFQDN() { echo $maxhost return fi + hostname=$(python -c 'import socket; print(socket.getfqdn())') + if [ $? == 0 ]; then + echo $hostname + return + fi defhost=`hostname` if [ -e /usr/bin/host -o -e /bin/host ]; then hosthost=`host $defhost | grep -v "not found" | awk '{print $1}'` diff --git a/nss_engine_cipher.h b/nss_engine_cipher.h index 83321c2..41b1e02 100644 --- a/nss_engine_cipher.h +++ b/nss_engine_cipher.h @@ -86,7 +86,7 @@ typedef struct #define SSLV3 0x00000002L #define TLSV1 SSLV3 #define TLSV1_2 0x00000004L -#define TLSV1_3 0x00000005L +#define TLSV1_3 0x00000008L /* the table itself is defined in nss_engine_cipher.c */ #if 0 diff --git a/test/test.py b/test/test.py index 7160a26..20fd3d2 100644 --- a/test/test.py +++ b/test/test.py @@ -1,5 +1,6 @@ from test_config import Declarative, write_template_file, restart_apache from test_config import stop_apache +from test_util import run from variable import ENABLE_SERVER_DHE import ssl import requests.exceptions @@ -17,6 +18,16 @@ except ImportError: from urllib3.packages.ssl_match_hostname import CertificateError +def www1_defined(): + """Dumb test to see if www1.example.com is a known host to see + whether the proxy tests should be executed or not. + """ + (out, err, rc) = run(["/usr/bin/ping", + "-w", "2", + "-c", "1", "www1.example.com"]) + return rc == 0 + + class test_suite1(Declarative): @classmethod def setUpClass(cls): @@ -232,21 +243,26 @@ class test_suite1(Declarative): expected=200, ), - dict( - desc='SNI request when SNI is disabled', - request=('/index.html', - {'host': 'www1.example.com', 'port': 8000}), - expected=requests.exceptions.SSLError(), - expected_str='doesn\'t match', - ), + ] - dict( - desc='Reverse proxy request when SNI is disabled', - request=('/proxy/index.html', {}), - expected=400, - ), + if www1_defined(): + tests.append( + dict( + desc='SNI request when SNI is disabled', + request=('/index.html', + {'host': 'www1.example.com', 'port': 8000}), + expected=requests.exceptions.SSLError(), + expected_str='doesn\'t match', + ), + ) - ] + tests.append( + dict( + desc='Reverse proxy request when SNI is disabled', + request=('/proxy/index.html', {}), + expected=400, + ), + ) if ENABLE_SERVER_DHE: tests.append( diff --git a/test/test_cipher.py b/test/test_cipher.py index 69de7dc..0e3c690 100644 --- a/test/test_cipher.py +++ b/test/test_cipher.py @@ -45,6 +45,7 @@ CIPHERS_NOT_IN_NSS = [ 'ECDHE-RSA-CAMELLIA128-SHA256', 'DHE-RSA-CAMELLIA128-SHA256', 'DHE-RSA-CAMELLIA256-SHA256', + 'TLS_AES_128_CCM_SHA256', ] CIPHERS_NOT_IN_OPENSSL = [ @@ -59,7 +60,7 @@ CIPHERS_NOT_IN_OPENSSL = [ ] OPENSSL_CIPHERS_IGNORE = ":-SSLv2:-KRB5:-PSK:-ADH:-DSS:-SEED:-IDEA" \ - ":-SRP:-AESCCM:-AESCCM8" + ":-SRP:-AESCCM:-AESCCM8:-RC4:-ARIA" if ENABLE_SERVER_DHE == 0: OPENSSL_CIPHERS_IGNORE += ':-DH' @@ -76,8 +77,13 @@ def openssl_tls13(): (out, err, rc) = run([openssl, 'ciphers', 'tls1_3']) return rc == 0 +def openssl_has_ciphersuites(): + (out, err, rc) = run(["openssl", "ciphers", "-ciphersuites", "", "AES"]) + return rc == 0 + OPENSSL_CHACHA20 = openssl_CHACHA20() OPENSSL_TLS13 = openssl_tls13() +OPENSSL_HAS_CIPHERSUITES = openssl_has_ciphersuites() tls13_ciphers = [ 'TLS-AES-128-GCM-SHA256', @@ -86,12 +92,21 @@ tls13_ciphers = [ ] -def assert_equal_openssl(ciphers): +def assert_equal_openssl(ciphers, tls13=False): nss_ciphers = ciphers + ":-EXP:-LOW:-RC4:-EDH" ossl_ciphers = ciphers + OPENSSL_CIPHERS_IGNORE + + if not tls13 and OPENSSL_HAS_CIPHERSUITES: + # Disable TLSv1.3 ciphers to match default output in openssl ciphers + nss_ciphers = nss_ciphers + ":-TLSv1.3" (nss, err, rc) = run([exe, "--o", nss_ciphers]) assert rc == 0 - (ossl, err, rc) = run([openssl, "ciphers", ossl_ciphers]) + if not tls13 and OPENSSL_HAS_CIPHERSUITES: + # Disable TLSv1.3 ciphers to match previous behavior + cmd = [openssl, "ciphers", "-ciphersuites", "", ossl_ciphers] + else: + cmd = [openssl, "ciphers", ossl_ciphers] + (ossl, err, rc) = run(cmd) assert rc == 0 nss_list = nss.strip().split(':') @@ -134,9 +149,9 @@ def assert_equal_openssl(ciphers): elif len(ossl_list) > len(nss_list): diff = set(ossl_list) - set(nss_list) else: - diff = '' + diff = None - assert nss_list == ossl_list, '%r != %r. Difference %r' % ( + assert diff is None, '%r != %r. Difference %r' % ( ':'.join(nss_list), ':'.join(ossl_list), diff) @@ -228,6 +243,10 @@ class test_ciphers(object): def test_TLSv12(self): assert_equal_openssl("TLSv1.2") + def test_TLSv13(self): + if OPENSSL_TLS13: + assert_equal_openssl("TLSv1.3", tls13=True) + def test_NULL(self): assert_equal_openssl("NULL") From 90ff8eca5e9ae7a7c02a0f95b47515aa099ba712 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jan 30 2020 19:46:51 +0000 Subject: [PATCH 3/4] Port to using python 3 for use in the test framework --- diff --git a/Makefile.am b/Makefile.am index 2009060..1946d00 100644 --- a/Makefile.am +++ b/Makefile.am @@ -100,14 +100,14 @@ EXTRA_DIST = *.h *.8 LICENSE test docs check: cd test; \ rm -rf work; \ - nosetests -v test_cipher.py; \ + nosetests-3 -v test_cipher.py; \ if [ `id -u` != 0 ]; then \ ./setup.sh -s 1 dbm:; \ - DBPREFIX=dbm: nosetests -v test.py; \ + DBPREFIX=dbm: nosetests-3 -v test.py; \ sleep 5; \ rm -rf work; \ ./setup.sh -s 1 sql:; \ - DBPREFIX=sql: nosetests -v test.py; \ + DBPREFIX=sql: nosetests-3 -v test.py; \ cd ..; \ else \ echo "Skipping live tests as they cannot be run as root"; \ @@ -117,7 +117,7 @@ checksni: cd test; \ rm -rf work; \ ./setup.sh -s 25; \ - nosetests -v testsni.py; \ + nosetests-3 -v testsni.py; \ cd .. diff --git a/gencert.in b/gencert.in index 2b06ad0..4c26c6c 100755 --- a/gencert.in +++ b/gencert.in @@ -38,7 +38,7 @@ getFQDN() { echo $maxhost return fi - hostname=$(python -c 'import socket; print(socket.getfqdn())') + hostname=$(python3 -c 'import socket; print(socket.getfqdn())') if [ $? == 0 ]; then echo $hostname return diff --git a/test/test.py b/test/test.py index 20fd3d2..1b2cb66 100644 --- a/test/test.py +++ b/test/test.py @@ -222,14 +222,6 @@ class test_suite1(Declarative): ), dict( - desc='Try SSLv3 client on 1.2-only VH', - request=('/protocoltls12/index.html', - {'port': 8001, - 'ssl_version': ssl.PROTOCOL_SSLv3}), - expected=requests.exceptions.SSLError(), - ), - - dict( desc='Try TLSv1 client on 1.2-only VH', request=('/protocoltls12/index.html', {'port': 8001, diff --git a/test/test_config.py b/test/test_config.py index f3091e8..749d041 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -61,7 +61,8 @@ def template_file(infilename, vars): def write_template_file(infilename, outfilename, vars): """Read a file and perform template substitutions""" - replacevars = dict(default_vars.items() + vars.items()) + replacevars = dict(default_vars.items()) + replacevars.update(vars.items()) with open(outfilename, 'w') as f: f.write('%s\n' % template_file(infilename, replacevars)) @@ -171,17 +172,16 @@ class Declarative(object): name = klass.__name__ try: output = self.make_request(uri, options) - except StandardError, e: - pass + except Exception as e: + if not isinstance(e, klass): + if expected_str not in str(e): + raise AssertionError( + UNEXPECTED % (uri, name, options, e.__class__.__name__, e) + ) else: raise AssertionError( EXPECTED % (uri, name, options, output) ) - if not isinstance(e, klass): - if expected_str not in str(e): - raise AssertionError( - UNEXPECTED % (uri, name, options, e.__class__.__name__, e) - ) def check_result(self, nice, uri, options, expected, cipher=None, protocol=None, content=None): diff --git a/test/test_request.py b/test/test_request.py index 8b6dae2..6235e6f 100644 --- a/test/test_request.py +++ b/test/test_request.py @@ -3,7 +3,6 @@ import socket import requests -import urlparse import logging import socket from requests.packages.urllib3.util import get_host @@ -227,7 +226,7 @@ try: 'ssl_version': ssl.PROTOCOL_SSLv23, 'ciphers': 'HIGH'}) cipher = r.raw._pool._get_conn().client_cipher -except requests.exceptions.SSLError, e: +except requests.exceptions.SSLError as e: print e.message else: print r.status_code diff --git a/test/test_util.py b/test/test_util.py index 101b6a2..95144fc 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -1,3 +1,4 @@ +import locale import socket import time import subprocess @@ -24,7 +25,7 @@ def host_port_open(host, port, socket_type=socket.SOCK_STREAM, s.recv(512) return True - except socket.error, e: + except socket.error: pass finally: if s: @@ -81,7 +82,11 @@ def run(args): p.wait() raise - return (stdout, stderr, p.returncode) + encoding = locale.getpreferredencoding() + output = stdout.decode(encoding) + error_output = stderr.decode(encoding) + + return (output, error_output, p.returncode) def assert_equal(got, expected): From 788c1f93c7f455f8d795948ab62333a6ac02ee53 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Jan 30 2020 20:04:10 +0000 Subject: [PATCH 4/4] Fix some python coding style issues This fixes most but not all of the flake8 errors. This is all test framework and I'm not concerned with perfectly formatted code here. --- diff --git a/test/msupn.py b/test/msupn.py index cb60224..7ce56af 100644 --- a/test/msupn.py +++ b/test/msupn.py @@ -16,6 +16,7 @@ class UTF8StringTagged0(char.GeneralString): tagSet = char.UTF8String.tagSet.tagExplicitly( tag.Tag(tag.tagClassContext, tag.tagFormatSimple, 0)) + id_msupn_san = univ.ObjectIdentifier('1.3.6.1.4.1.311.20.2.3') name = UTF8StringTagged0("john.doe@EXAMPLE.COM") diff --git a/test/test.py b/test/test.py index 1b2cb66..d7e2552 100644 --- a/test/test.py +++ b/test/test.py @@ -6,17 +6,6 @@ import ssl import requests.exceptions import os -try: - # python3.2+ - from ssl import CertificateError -except ImportError: - try: - # Older python where the backport from pypi is installed - from backports.ssl_match_hostname import CertificateError - except ImportError: - # Other older python we use the urllib3 bundled copy - from urllib3.packages.ssl_match_hostname import CertificateError - def www1_defined(): """Dumb test to see if www1.example.com is a known host to see diff --git a/test/test_cipher.py b/test/test_cipher.py index 0e3c690..eeb118b 100644 --- a/test/test_cipher.py +++ b/test/test_cipher.py @@ -1,7 +1,5 @@ from test_util import run, assert_equal import os -import nose -from nose.tools import make_decorator # This file is auto-generated by configure from variable import ( @@ -77,10 +75,12 @@ def openssl_tls13(): (out, err, rc) = run([openssl, 'ciphers', 'tls1_3']) return rc == 0 + def openssl_has_ciphersuites(): (out, err, rc) = run(["openssl", "ciphers", "-ciphersuites", "", "AES"]) return rc == 0 + OPENSSL_CHACHA20 = openssl_CHACHA20() OPENSSL_TLS13 = openssl_tls13() OPENSSL_HAS_CIPHERSUITES = openssl_has_ciphersuites() @@ -307,9 +307,6 @@ class test_ciphers(object): def test_kEECDH(self): assert_equal_openssl("kEECDH") - def test_AECDH(self): - assert_equal_openssl("AECDH") - def test_EECDH(self): assert_equal_openssl("EECDH") diff --git a/test/test_config.py b/test/test_config.py index 749d041..9b186fc 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -17,7 +17,6 @@ import os import re -import ssl import time import string import requests @@ -73,8 +72,9 @@ def stop_apache(): # no try/except, just let it fail os.chdir('work/httpd') - p = subprocess.Popen(['./stop'], - close_fds=True) + subprocess.Popen(['./stop'], + close_fds=True) + os.chdir(cwd) def restart_apache(): @@ -83,14 +83,15 @@ def restart_apache(): # no try/except, just let it fail os.chdir('work/httpd') - p = subprocess.Popen(['./stop'], - close_fds=True) + subprocess.Popen(['./stop'], + close_fds=True) time.sleep(5) - p = subprocess.Popen(['./start'], - close_fds=True) + subprocess.Popen(['./start'], + close_fds=True) os.chdir(cwd) test_util.wait_for_open_ports(FQDN, DEF_PORT) + EXPECTED = """Expected %r to raise %s. options = %r output = %r""" @@ -176,7 +177,8 @@ class Declarative(object): if not isinstance(e, klass): if expected_str not in str(e): raise AssertionError( - UNEXPECTED % (uri, name, options, e.__class__.__name__, e) + UNEXPECTED % (uri, name, options, + e.__class__.__name__, e) ) else: raise AssertionError( @@ -185,14 +187,13 @@ class Declarative(object): def check_result(self, nice, uri, options, expected, cipher=None, protocol=None, content=None): - name = expected.__class__.__name__ request = self.make_request(uri, options) has_sni = options.get('sni', False) if content and content not in request.content: - raise AssertionError( - 'Expected %s not in %s' % (content, request.content) - ) + raise AssertionError( + 'Expected %s not in %s' % (content, request.content) + ) if cipher: if has_sni: raise AssertionError('Cannot do cipher tests in SNI') @@ -211,7 +212,7 @@ class Declarative(object): (protocol, client_cipher[1]) ) if expected != request.status_code: - raise AssertionError( - 'Expected status %s, got %s' % - (expected, request.status_code) - ) + raise AssertionError( + 'Expected status %s, got %s' % + (expected, request.status_code) + ) diff --git a/test/test_request.py b/test/test_request.py index 6235e6f..fdedd48 100644 --- a/test/test_request.py +++ b/test/test_request.py @@ -1,13 +1,12 @@ # # Override a slew of methods to have more control over SSL -import socket import requests import logging import socket +import ssl from requests.packages.urllib3.util import get_host from requests.packages.urllib3.util.timeout import Timeout -from requests.packages.urllib3.contrib import pyopenssl from requests.packages.urllib3.connectionpool import ( HTTPConnectionPool, HTTPSConnectionPool, VerifiedHTTPSConnection) @@ -25,29 +24,16 @@ except ImportError: 'DH+CHACHA20:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:' 'RSA+AESGCM:RSA+AES:!aNULL:!eNULL:!MD5') -# Don't bend over backwards for ssl support, assume it is there. -import ssl -try: # Python 3 - from http.client import HTTPConnection, HTTPException - from http.client import HTTP_PORT, HTTPS_PORT - from http.client import HTTPSConnection -except ImportError: - from httplib import HTTPConnection, HTTPException - from httplib import HTTP_PORT, HTTPS_PORT - from httplib import HTTPSConnection - try: # python3.2+ - from ssl import match_hostname, CertificateError + from ssl import match_hostname except ImportError: try: # Older python where the backport from pypi is installed - from backports.ssl_match_hostname import ( - match_hostname, CertificateError) + from backports.ssl_match_hostname import match_hostname except ImportError: # Other older python we use the urllib3 bundled copy - from urllib3.packages.ssl_match_hostname import ( - match_hostname, CertificateError) + from urllib3.packages.ssl_match_hostname import match_hostname SAVE_DEFAULT_SSL_CIPHER_LIST = DEFAULT_SSL_CIPHER_LIST @@ -64,7 +50,7 @@ def connection_from_url(url, **kw): :param url: Absolute URL string that must include the scheme. Port is optional. - :param \**kw: + :param **kw: Passes additional parameters to the constructor of the appropriate :class:`.ConnectionPool`. Useful for specifying things like timeout, maxsize, headers, etc. @@ -217,6 +203,7 @@ class MyAdapter(requests.adapters.HTTPAdapter): else: # huh? Do nothing pass + """ s = requests.Session() s.mount('https://', MyAdapter()) diff --git a/test/test_util.py b/test/test_util.py index 95144fc..9d68888 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -66,14 +66,11 @@ def run(args): :param args: List of arguments for the command """ - p_in = None p_out = None p_out = subprocess.PIPE p_err = subprocess.PIPE - arg_string = ' '.join(shell_quote(a) for a in args) - try: p = subprocess.Popen(args, stdout=p_out, stderr=p_err, close_fds=True) diff --git a/test/testsni.py b/test/testsni.py index 23bbd8a..469ca63 100644 --- a/test/testsni.py +++ b/test/testsni.py @@ -1,6 +1,5 @@ from test_config import Declarative, write_template_file, restart_apache from test_config import stop_apache -import ssl import requests.exceptions import os