From 047ff7b55f207976f35e29583197b032877f4688 Mon Sep 17 00:00:00 2001 From: Brian Duffy Date: Thu, 1 Feb 2018 23:50:02 +0000 Subject: [PATCH 1/7] [bugfix-pem-validation] Signed-off-by: Brian Duffy --- awx/main/validators.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/awx/main/validators.py b/awx/main/validators.py index 5af078a861..0c6a9dc569 100644 --- a/awx/main/validators.py +++ b/awx/main/validators.py @@ -21,7 +21,7 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): """ Validate the given PEM data is valid and contains the required numbers of keys and certificates. - + Return a list of PEM objects, where each object is a dict with the following keys: - 'all': The entire string for the PEM object including BEGIN/END lines. @@ -48,24 +48,32 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): # Build regular expressions for matching each object in the PEM file. pem_obj_re = re.compile( - r'^(-{4,}) *BEGIN ([A-Z ]+?) *\1[\r\n]+' + - r'(.+?)[\r\n]+\1 *END \2 *\1[\r\n]?(.*?)$', re.DOTALL, + r'^-{5}\ *BEGIN ([A-Z ]+?)\ *-{5}' + + r'\s*(.+?)\s*' + + r'-{5}\ *END [A-Z ]+?\ *-{5}' + + r'\s?(.*?)$', + re.DOTALL ) pem_obj_header_re = re.compile(r'^(.+?):\s*?(.+?)(\\??)$') pem_objects = [] key_count, cert_count = 0, 0 + + # Strip leading whitespaces at the start of the PEM data data = data.lstrip() + while data: match = pem_obj_re.match(data) if not match: raise ValidationError(_('Invalid certificate or key: %s...') % data[:100]) - data = match.group(4).lstrip() + + # The rest of the PEM data to process + data = match.group(3).lstrip() # Check PEM object type, check key type if private key. pem_obj_info = {} pem_obj_info['all'] = match.group(0) - pem_obj_info['type'] = pem_obj_type = match.group(2) + pem_obj_info['type'] = pem_obj_type = match.group(1) if pem_obj_type.endswith('PRIVATE KEY'): key_count += 1 pem_obj_info['type'] = 'PRIVATE KEY' @@ -80,7 +88,7 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): raise ValidationError(_('Unsupported PEM object type: "%s"') % pem_obj_type) # Ensure that this PEM object is valid base64 data. - pem_obj_info['data'] = match.group(3) + pem_obj_info['data'] = match.group(2) base64_data = '' line_continues = False for line in pem_obj_info['data'].splitlines(): @@ -186,4 +194,3 @@ def vars_validate_or_raise(vars_str): return vars_str except ParseError as e: raise RestValidationError(str(e)) - From 68057560e5aada5c530b66c3c003e1b08574e156 Mon Sep 17 00:00:00 2001 From: Brian Duffy Date: Fri, 2 Feb 2018 01:20:31 +0000 Subject: [PATCH 2/7] [bugfix-pem-validation] added unit test to simulate catted data Signed-off-by: Brian Duffy --- awx/main/tests/data/ssh.py | 4 ++++ awx/main/tests/unit/test_validators.py | 15 +++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/awx/main/tests/data/ssh.py b/awx/main/tests/data/ssh.py index b3f5e8b675..77f02eb14d 100644 --- a/awx/main/tests/data/ssh.py +++ b/awx/main/tests/data/ssh.py @@ -193,3 +193,7 @@ L5Hj+B02+FAiz8zVGumbVykvPtzgTb0E+0rJKNO0/EgGqWsk/oC0 """ TEST_SSH_KEY_DATA_UNLOCK = 'unlockme' + +TEST_CATTED_SSH_KEY_DATA = """ +-----BEGIN RSA PRIVATE KEY----- MIIEogIBAAKCAQEA1T4za6qBbHxFpN5f9eFvA74MFjrsjcp1uvzOaE23AYKMDEJg hJ6dqQ7GwHLNIeIeumqDFmODauIzrgSDJTT5+NG30Rr+rRi0zDkrkBAj/AtA+SaV hbzqB6ZSd7LaMly9XAc+82OKlNpuWS9hPmFaSShzDTXRu5RRyvm4NDCAOGDu5hyV R2pV/ffKDNfNkChnqzvRRW9laQcVmliZhlTGn7nPZ+JbjpwEy0nwW+4zoAiEvwnT 52N4xTqIcYOnXtGiaf13dh7FkUfYmS0tzF3+h8QRKwtIm4y+sq84R/kr79/0t5aR UpJynNrECajzmArpL4IjXKTPIyUpTKirJgGnCwIDAQABAoIBAC6bbbm2hpsjfkVO pUKkhxMWUqX5MwK6oYjBAIwjkEAwPFPhnh7eXC87H42oidVCCt1LsmMOVQbjcdAz BEb5kTkk/Twi3k8O+1U3maHfJT5NZ2INYNjeNXh+jb/Dw5UGWAzpOIUR2JQ4Oa4c gPCVbppW0O6uOKz6+fWXJv+hKiUoBCC0TiY52iseHJdUOaKNxYRD2IyIzCAxFSd5 tZRaARIYDsugXp3E/TdbsVWA7bmjIBOXq+SquTrlB8x7j3B7+Pi09nAJ2U/uV4PH E+/2Fl009ywfmqancvnhwnz+GQ5jjP+gTfghJfbO+Z6M346rS0Vw+osrPgfyudNH lCswHOECgYEA/Cfq25gDP07wo6+wYWbx6LIzj/SSZy/Ux9P8zghQfoZiPoaq7BQB PAzwLNt7JWST8U11LZA8/wo6ch+HSTMk+m5ieVuru2cHxTDqeNlh94eCrNwPJ5ay A5U6LxAuSCTAzp+rv6KQUx1JcKSEHuh+nRYTKvUDE6iA6YtPLO96lLUCgYEA2H5r OPX2M4w1Q9zjol77lplbPRdczXNd0PIzhy8Z2ID65qvmr1nxBG4f2H96ykW8CKLX NvSXreNZ1BhOXc/3Hv+3mm46iitB33gDX4mlV4Jyo/w5IWhUKRyoW6qXquFFsScx RzTrx/9M+aZeRRLdsBk27HavFEg6jrbQ0SleZL8CgYAaM6Op8d/UgkVrHOR9Go9k mK/W85kK8+NuaE7Ksf57R0eKK8AzC9kc/lMuthfTyOG+n0ff1i8gaVWtai1Ko+/h vfqplacAsDIUgYK70AroB8LCZ5ODj5sr2CPVpB7LDFakod7c6O2KVW6+L7oy5AHU HOkc+5y4PDg5DGrLxo68SQKBgAlGoWF3aG0c/MtDk51JZI43U+lyLs++ua5SMlMA eaMFI7rucpvgxqrh7Qthqukvw7a7A22fXUBeFWM5B2KNnpD9c+hyAKAa6l+gzMQz KZpuRGsyS2BbEAAS8kO7M3Rm4o2MmFfstI2FKs8nibJ79HOvIONQ0n+T+K5Utu2/ UAQRAoGAFB4fiIyQ0nYzCf18Z4Wvi/qeIOW+UoBonIN3y1h4wruBywINHxFMHx4a VImJ6R09hoJ9D3Mxli3xF/8JIjfTG5fBSGrGnuofl14d/XtRDXbT2uhVXrIkeLL/ ojODwwEx0VhxIRUEjPTvEl6AFSRRcBp3KKzQ/cu7vafY6GTlOUI= -----END RSA PRIVATE KEY----- +""" diff --git a/awx/main/tests/unit/test_validators.py b/awx/main/tests/unit/test_validators.py index bf08f6a0d5..3500f8d023 100644 --- a/awx/main/tests/unit/test_validators.py +++ b/awx/main/tests/unit/test_validators.py @@ -12,12 +12,25 @@ from awx.main.tests.data.ssh import ( TEST_OPENSSH_KEY_DATA, TEST_OPENSSH_KEY_DATA_LOCKED, TEST_SSH_CERT_KEY, + TEST_CATTED_SSH_KEY_DATA, ) from rest_framework.serializers import ValidationError as RestValidationError import pytest +def test_valid_catted_rsa_key(): + valid_key = TEST_CATTED_SSH_KEY_DATA + pem_objects = validate_private_key(valid_key) + assert pem_objects[0]['key_type'] == 'rsa' + assert not pem_objects[0]['key_enc'] + with pytest.raises(ValidationError): + validate_certificate(valid_key) + pem_objects = validate_ssh_private_key(valid_key) + assert pem_objects[0]['key_type'] == 'rsa' + assert not pem_objects[0]['key_enc'] + + def test_valid_rsa_key(): valid_key = TEST_SSH_KEY_DATA pem_objects = validate_private_key(valid_key) @@ -126,5 +139,3 @@ def test_valid_vars(var_str): def test_invalid_vars(var_str): with pytest.raises(RestValidationError): vars_validate_or_raise(var_str) - - From 7d956a3b68df567bec157908e208a1e6bdc7e95d Mon Sep 17 00:00:00 2001 From: Brian Duffy Date: Sat, 10 Feb 2018 01:08:29 +0000 Subject: [PATCH 3/7] [bugfix-pem-validation] update from code review --- awx/main/tests/unit/test_validators.py | 28 +++++++++++++++++--------- awx/main/validators.py | 22 +++++++++++--------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/awx/main/tests/unit/test_validators.py b/awx/main/tests/unit/test_validators.py index 3500f8d023..2221f241da 100644 --- a/awx/main/tests/unit/test_validators.py +++ b/awx/main/tests/unit/test_validators.py @@ -19,6 +19,24 @@ from rest_framework.serializers import ValidationError as RestValidationError import pytest +def test_invalid_keys(): + invalid_keys = [ + "---BEGIN FOO -----foobar-----END FOO----", + "-----BEGIN FOO---foobar-----END FOO----", + "-----BEGIN FOO-----foobar---END FOO----", + "----- BEGIN FOO ----- foobar ----- FAIL FOO ----", + "----- FAIL FOO ----- foobar ----- END FOO ----", + "----BEGIN FOO----foobar----END FOO--------BEGIN FOO----foobar----END FOO----" + ] + for invalid_key in invalid_keys: + with pytest.raises(ValidationError): + validate_private_key(invalid_key) + with pytest.raises(ValidationError): + validate_certificate(invalid_key) + with pytest.raises(ValidationError): + validate_ssh_private_key(invalid_key) + + def test_valid_catted_rsa_key(): valid_key = TEST_CATTED_SSH_KEY_DATA pem_objects = validate_private_key(valid_key) @@ -55,16 +73,6 @@ def test_valid_locked_rsa_key(): assert pem_objects[0]['key_enc'] -def test_invalid_rsa_key(): - invalid_key = TEST_SSH_KEY_DATA.replace('-----END', '----END') - with pytest.raises(ValidationError): - validate_private_key(invalid_key) - with pytest.raises(ValidationError): - validate_certificate(invalid_key) - with pytest.raises(ValidationError): - validate_ssh_private_key(invalid_key) - - def test_valid_openssh_key(): valid_key = TEST_OPENSSH_KEY_DATA pem_objects = validate_private_key(valid_key) diff --git a/awx/main/validators.py b/awx/main/validators.py index 0c6a9dc569..3975e6f053 100644 --- a/awx/main/validators.py +++ b/awx/main/validators.py @@ -48,11 +48,10 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): # Build regular expressions for matching each object in the PEM file. pem_obj_re = re.compile( - r'^-{5}\ *BEGIN ([A-Z ]+?)\ *-{5}' + - r'\s*(.+?)\s*' + - r'-{5}\ *END [A-Z ]+?\ *-{5}' + - r'\s?(.*?)$', - re.DOTALL + r'^-{4,} *BEGIN (?P[A-Z ]+?) *-{4,}' + + r'\s*(?P.+?)\s*' + + r'-{4,} *END [A-Z ]+? *-{4,}' + + r'(?P.*?)$', re.DOTALL ) pem_obj_header_re = re.compile(r'^(.+?):\s*?(.+?)(\\??)$') @@ -68,12 +67,13 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): raise ValidationError(_('Invalid certificate or key: %s...') % data[:100]) # The rest of the PEM data to process - data = match.group(3).lstrip() + data = match.group('next').lstrip() + print data # Check PEM object type, check key type if private key. pem_obj_info = {} pem_obj_info['all'] = match.group(0) - pem_obj_info['type'] = pem_obj_type = match.group(1) + pem_obj_info['type'] = pem_obj_type = match.group('type') if pem_obj_type.endswith('PRIVATE KEY'): key_count += 1 pem_obj_info['type'] = 'PRIVATE KEY' @@ -88,7 +88,7 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): raise ValidationError(_('Unsupported PEM object type: "%s"') % pem_obj_type) # Ensure that this PEM object is valid base64 data. - pem_obj_info['data'] = match.group(2) + pem_obj_info['data'] = match.group('data') base64_data = '' line_continues = False for line in pem_obj_info['data'].splitlines(): @@ -169,8 +169,10 @@ def validate_certificate(data): Validate that data contains one or more certificates. Adds BEGIN/END lines if necessary. """ - if 'BEGIN CERTIFICATE' not in data: - data = '-----BEGIN CERTIFICATE-----\n{}\n-----END CERTIFICATE-----\n'.format(data) + if 'BEGIN' not in data: + data = "-----BEGIN CERTIFICATE-----\n{}".format(data) + if 'END' not in data: + data = "{}\n-----END CERTIFICATE-----\n".format(data) return validate_pem(data, max_keys=0, min_certs=1) From 6b5a6e9226336c84da8a7c7a4e9c2ffaa63a6285 Mon Sep 17 00:00:00 2001 From: Brian Duffy Date: Mon, 12 Feb 2018 16:44:32 +0000 Subject: [PATCH 4/7] [bugfix-pem-validation] left a print statement in --- awx/main/validators.py | 1 - 1 file changed, 1 deletion(-) diff --git a/awx/main/validators.py b/awx/main/validators.py index 3975e6f053..ad6fdb02fd 100644 --- a/awx/main/validators.py +++ b/awx/main/validators.py @@ -68,7 +68,6 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): # The rest of the PEM data to process data = match.group('next').lstrip() - print data # Check PEM object type, check key type if private key. pem_obj_info = {} From 235213bd3b79af385e2cab5638623acd41a070e9 Mon Sep 17 00:00:00 2001 From: Brian Duffy Date: Fri, 16 Feb 2018 16:06:33 +0000 Subject: [PATCH 5/7] updated regex --- awx/main/tests/unit/test_validators.py | 13 +++++++++++-- awx/main/validators.py | 4 ++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/awx/main/tests/unit/test_validators.py b/awx/main/tests/unit/test_validators.py index 2221f241da..78c9f279cf 100644 --- a/awx/main/tests/unit/test_validators.py +++ b/awx/main/tests/unit/test_validators.py @@ -26,7 +26,7 @@ def test_invalid_keys(): "-----BEGIN FOO-----foobar---END FOO----", "----- BEGIN FOO ----- foobar ----- FAIL FOO ----", "----- FAIL FOO ----- foobar ----- END FOO ----", - "----BEGIN FOO----foobar----END FOO--------BEGIN FOO----foobar----END FOO----" + "----BEGIN FOO----foobar----END BAR----" ] for invalid_key in invalid_keys: with pytest.raises(ValidationError): @@ -37,6 +37,16 @@ def test_invalid_keys(): validate_ssh_private_key(invalid_key) +def test_invalid_rsa_key(): + invalid_key = TEST_SSH_KEY_DATA.replace('-----END', '----END') + with pytest.raises(ValidationError): + validate_private_key(invalid_key) + with pytest.raises(ValidationError): + validate_certificate(invalid_key) + with pytest.raises(ValidationError): + validate_ssh_private_key(invalid_key) + + def test_valid_catted_rsa_key(): valid_key = TEST_CATTED_SSH_KEY_DATA pem_objects = validate_private_key(valid_key) @@ -48,7 +58,6 @@ def test_valid_catted_rsa_key(): assert pem_objects[0]['key_type'] == 'rsa' assert not pem_objects[0]['key_enc'] - def test_valid_rsa_key(): valid_key = TEST_SSH_KEY_DATA pem_objects = validate_private_key(valid_key) diff --git a/awx/main/validators.py b/awx/main/validators.py index ad6fdb02fd..390a2f16e6 100644 --- a/awx/main/validators.py +++ b/awx/main/validators.py @@ -48,9 +48,9 @@ def validate_pem(data, min_keys=0, max_keys=None, min_certs=0, max_certs=None): # Build regular expressions for matching each object in the PEM file. pem_obj_re = re.compile( - r'^-{4,} *BEGIN (?P[A-Z ]+?) *-{4,}' + + r'^(?P-{4,}) *BEGIN (?P[A-Z ]+?) *(?P=dashes)' + r'\s*(?P.+?)\s*' + - r'-{4,} *END [A-Z ]+? *-{4,}' + + r'(?P=dashes) *END (?P=type) *(?P=dashes)' + r'(?P.*?)$', re.DOTALL ) pem_obj_header_re = re.compile(r'^(.+?):\s*?(.+?)(\\??)$') From 098f4eb198c518aa8c20eedbfe2b6a2a5373c210 Mon Sep 17 00:00:00 2001 From: Brian Duffy Date: Sun, 18 Feb 2018 01:46:31 +0000 Subject: [PATCH 6/7] [bugfix-pem-validation] pass flake8 --- awx/main/tests/unit/test_validators.py | 1 + 1 file changed, 1 insertion(+) diff --git a/awx/main/tests/unit/test_validators.py b/awx/main/tests/unit/test_validators.py index 78c9f279cf..b4681f4895 100644 --- a/awx/main/tests/unit/test_validators.py +++ b/awx/main/tests/unit/test_validators.py @@ -58,6 +58,7 @@ def test_valid_catted_rsa_key(): assert pem_objects[0]['key_type'] == 'rsa' assert not pem_objects[0]['key_enc'] + def test_valid_rsa_key(): valid_key = TEST_SSH_KEY_DATA pem_objects = validate_private_key(valid_key) From 4270e3a17be0f8f3663afb1197dd5691106f04ea Mon Sep 17 00:00:00 2001 From: Brian Duffy Date: Sun, 18 Feb 2018 15:11:42 +0000 Subject: [PATCH 7/7] [bugfix] updated pem validation unit tests --- awx/main/tests/unit/test_validators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tests/unit/test_validators.py b/awx/main/tests/unit/test_validators.py index b4681f4895..a44f78b53c 100644 --- a/awx/main/tests/unit/test_validators.py +++ b/awx/main/tests/unit/test_validators.py @@ -26,7 +26,7 @@ def test_invalid_keys(): "-----BEGIN FOO-----foobar---END FOO----", "----- BEGIN FOO ----- foobar ----- FAIL FOO ----", "----- FAIL FOO ----- foobar ----- END FOO ----", - "----BEGIN FOO----foobar----END BAR----" + "----BEGIN FOO----foobar----END BAR----", ] for invalid_key in invalid_keys: with pytest.raises(ValidationError):