Skip to content

Commit 0bb17c4

Browse files
fix(lib.validation): properly validate ca-signed keypairs (ansible#652)
Change the mechanism we use to validate keypairs to not assume that a certificate is self-signed. This causes some valid keypairs to fail. AAP-33294
1 parent aa092d6 commit 0bb17c4

File tree

3 files changed

+87
-5
lines changed

3 files changed

+87
-5
lines changed

ansible_base/lib/testing/fixtures.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,72 @@ def rsa_keypair(rsa_keypair_factory):
233233
return rsa_keypair_factory()
234234

235235

236+
@copy_fixture(copies=3)
237+
@pytest.fixture
238+
def rsa_keypair_with_signed_cert(rsa_keypair_factory):
239+
root_keypair = rsa_keypair_factory()
240+
root_private_key = serialization.load_pem_private_key(root_keypair.private.encode("utf-8"), password=None)
241+
root_public_key = serialization.load_pem_public_key(root_keypair.public.encode("utf-8"))
242+
rsa_keypair = rsa_keypair_factory()
243+
public_key = serialization.load_pem_public_key(rsa_keypair.public.encode("utf-8"))
244+
issuer = x509.Name(
245+
[
246+
x509.NameAttribute(NameOID.COUNTRY_NAME, u"US"),
247+
x509.NameAttribute(NameOID.STATE_OR_PROVINCE_NAME, u"California"),
248+
x509.NameAttribute(NameOID.LOCALITY_NAME, u"San Francisco"),
249+
x509.NameAttribute(NameOID.ORGANIZATION_NAME, u"My Company"),
250+
x509.NameAttribute(NameOID.COMMON_NAME, u"mycompany.com"),
251+
]
252+
)
253+
subject = x509.Name(
254+
[
255+
x509.NameAttribute(NameOID.COUNTRY_NAME, u"US"),
256+
x509.NameAttribute(NameOID.STATE_OR_PROVINCE_NAME, u"California"),
257+
x509.NameAttribute(NameOID.LOCALITY_NAME, u"San Francisco"),
258+
x509.NameAttribute(NameOID.ORGANIZATION_NAME, u"My Company"),
259+
x509.NameAttribute(NameOID.COMMON_NAME, u"qa.mycompany.com"),
260+
]
261+
)
262+
root_certificate = (
263+
x509.CertificateBuilder()
264+
.subject_name(issuer)
265+
.issuer_name(issuer)
266+
.public_key(root_public_key)
267+
.serial_number(x509.random_serial_number())
268+
.not_valid_before(datetime.utcnow())
269+
.not_valid_after(datetime.utcnow() + timedelta(days=365))
270+
.add_extension(
271+
x509.SubjectAlternativeName([x509.DNSName(u"mycompany.com")]),
272+
critical=False,
273+
)
274+
.sign(root_private_key, hashes.SHA256())
275+
)
276+
certificate = (
277+
x509.CertificateBuilder()
278+
.subject_name(subject)
279+
.issuer_name(issuer)
280+
.public_key(public_key)
281+
.serial_number(x509.random_serial_number())
282+
.not_valid_before(datetime.utcnow())
283+
.not_valid_after(datetime.utcnow() + timedelta(days=365))
284+
.add_extension(
285+
x509.SubjectAlternativeName([x509.DNSName(u"qa.mycompany.com")]),
286+
critical=False,
287+
)
288+
.sign(root_private_key, hashes.SHA256())
289+
)
290+
291+
root_certificate_bytes = root_certificate.public_bytes(serialization.Encoding.PEM).decode("utf-8")
292+
certificate_bytes = certificate.public_bytes(serialization.Encoding.PEM).decode("utf-8")
293+
294+
RSAKeyPairWithCert = namedtuple("RSAKeyPairWithCert", ["private", "public", "certificate"])
295+
CertificateChain = namedtuple("CertificateChain", ["root", "subordinate"])
296+
297+
root_keypair_with_cert = RSAKeyPairWithCert(private=root_keypair.private, public=root_keypair.public, certificate=root_certificate_bytes)
298+
subordinate_keypair_with_cert = RSAKeyPairWithCert(private=rsa_keypair.private, public=rsa_keypair.public, certificate=certificate_bytes)
299+
return CertificateChain(root=root_keypair_with_cert, subordinate=subordinate_keypair_with_cert)
300+
301+
236302
@copy_fixture(copies=3)
237303
@pytest.fixture
238304
def rsa_keypair_with_cert(rsa_keypair_factory):

ansible_base/lib/utils/validation.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import base64
22
import binascii
33
import re
4+
import secrets
45
from urllib.parse import urlparse, urlunsplit
56

67
from cryptography.exceptions import InvalidSignature
@@ -79,7 +80,7 @@ def validate_cert_with_key(public_cert_string, private_key_string):
7980
# None if one of the parameters wasn't set
8081
# False if we failed to load an item (should be pre-tried by your serializer)
8182
# A ValidationError exception if the key/value don't match
82-
# True if everything checks out
83+
# True if everything checks out, meaning that the certificate and private key form a valid keypair
8384

8485
if not private_key_string or not public_cert_string:
8586
return None
@@ -92,11 +93,16 @@ def validate_cert_with_key(public_cert_string, private_key_string):
9293
except Exception:
9394
return False
9495

96+
# Generate nonce for keypair verification
97+
nonce = secrets.token_bytes(64)
98+
signature = private_key.sign(nonce, padding.PKCS1v15(), public_cert.signature_hash_algorithm)
99+
95100
try:
96-
# We have both pieces of the puzzle, lets make sure they interlock
97-
private_key.public_key().verify(
98-
public_cert.signature,
99-
public_cert.tbs_certificate_bytes,
101+
# We have both pieces of the puzzle, lets make sure they interlock;
102+
# do so by verifying the nonce we just signed can be verified by the provided certificate
103+
public_cert.public_key().verify(
104+
signature,
105+
nonce,
100106
# Depends on the algorithm used to create the certificate
101107
padding.PKCS1v15(),
102108
public_cert.signature_hash_algorithm,

test_app/tests/lib/utils/test_validation.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,16 @@ def test_validate_cert_with_key_mismatch(rsa_keypair_with_cert_1, rsa_keypair_wi
101101
assert "The certificate and private key do not match" in str(e.value)
102102

103103

104+
def test_validate_cert_with_signed_certificate(rsa_keypair_with_signed_cert_1):
105+
"""
106+
Ensure that validate_cert_with_key raises a ValidationError when the cert and key don't match.
107+
"""
108+
keypair = rsa_keypair_with_signed_cert_1.root
109+
assert validate_cert_with_key(keypair.certificate, keypair.private)
110+
keypair = rsa_keypair_with_signed_cert_1.subordinate
111+
assert validate_cert_with_key(keypair.certificate, keypair.private)
112+
113+
104114
def test_validate_image_data_with_valid_data():
105115
"""
106116
Ensure that validate_image_data accepts valid data.

0 commit comments

Comments
 (0)