Skip to content

fix(cloud): normalize EC curve casing so get_policy/set_policy round-trips #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TomaszSwierszczCA wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(cloud): normalize EC curve casing so get_policy/set_policy round-trips #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion tests/test_local_methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
CertificateRevokeError)
from vcert.http_status import HTTPStatus
from vcert.pem import parse_pem, Certificate
from vcert.policy.pm_cloud import CertificateAuthorityDetails, CertificateAuthorityInfo
from vcert.policy.pm_cloud import (CertificateAuthorityDetails, CertificateAuthorityInfo, build_policy_spec,
validate_policy_spec, get_invalid_cloud_ec_value)
from vcert.policy.policy_spec import DEFAULT_CA, Policy, PolicySpecification

pkcs12_enc_cert = """-----BEGIN CERTIFICATE-----
Expand Down Expand Up @@ -198,6 +199,28 @@ def test_parse_cloud_zone1(self):
# self.assertEqual(p.key_types[0].key_type, KeyType.ECDSA)
# self.assertEqual(p.key_types[0].key_curves, ["p521"])

def test_ec_curve_casing_roundtrip(self):
# E6: KeyType lowercases EC curves, so build_policy_spec used to emit "p256"/... which
# validate_policy_spec then rejected ("not supported by VaaS") - breaking an EC
# get_policy -> set_policy round-trip (Cloud and NGTS share this code). build_policy_spec must
# emit uppercase curves and get_invalid_cloud_ec_value must be case-insensitive.
conn = CloudConnection(token="")
cit = conn._parse_policy_response_to_object({
"id": "cit-ec",
"certificateAuthority": "DIGICERT",
"subjectCNRegexes": [".*"], "subjectORegexes": [".*"], "subjectOURegexes": [".*"],
"subjectSTRegexes": [".*"], "subjectLRegexes": [".*"], "subjectCValues": [".*"],
"sanRegexes": [".*"],
"keyTypes": [{"keyType": "EC", "keyCurves": ["P256", "P384", "P521"]}],
})
info = CertificateAuthorityInfo("DIGICERT", "acct", "Product")
ps = build_policy_spec(cit, info)
self.assertEqual(ps.policy.key_pair.elliptic_curves, ["P256", "P384", "P521"])
validate_policy_spec(ps) # round-trip: build output must pass its own validation
# get_invalid_cloud_ec_value is case-insensitive (also accepts a hand-written lowercase spec)
self.assertIsNone(get_invalid_cloud_ec_value(["p256", "P384", "ed25519"]))
self.assertEqual(get_invalid_cloud_ec_value(["bogus"]), "bogus")

def test_parse_tpp_zone1(self):
conn = TPPConnection(url="http://example.com/", user="", password="")
z = conn._parse_zone_data_to_object(json.loads(POLICY_TPP1))
Expand Down
14 changes: 8 additions & 6 deletions vcert/policy/pm_cloud.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ def build_policy_spec(cit, ca_info, subject_cn_to_str=True):
if kt.key_type.upper() == KeyType.RSA.upper():
rsa_key_sizes.append(kt.option)
elif kt.key_type.upper() == KeyType.ECDSA.upper():
elliptic_curves.append(kt.option)
# KeyType lowercases the curve (e.g. "p256"); emit the API/spec casing ("P256") so a
# get_policy -> set_policy round-trip validates against supported_elliptic_curves.
elliptic_curves.append(kt.option.upper())
# Only include one instance of the KeyType
if kt.key_type.upper() not in key_types:
key_types.append(kt.key_type.upper())
Expand Down Expand Up @@ -178,7 +180,7 @@ def build_policy_spec(cit, ca_info, subject_cn_to_str=True):
if kt.key_type == KeyType.RSA:
dkp.rsa_key_size = kt.option
elif kt.key_type == KeyType.ECDSA:
dkp.elliptic_curve = kt.option
dkp.elliptic_curve = kt.option.upper()

d.key_pair = dkp if create_dkp else None

Expand Down Expand Up @@ -385,7 +387,7 @@ def get_invalid_cloud_ec_value(elliptic_curves):
:rtype: str
"""
for v in elliptic_curves:
if v not in supported_elliptic_curves:
if str(v).upper() not in supported_elliptic_curves:
return v

return None
Expand Down Expand Up @@ -605,9 +607,9 @@ def build_cit_request(ps, ca_details):
ec_kt['keyType'] = KeyType.ECDSA.upper()

if ps.policy and ps.policy.key_pair and len(ps.policy.key_pair.elliptic_curves) > 0:
ec_kt['keyCurves'] = ps.policy.key_pair.elliptic_curves
ec_kt['keyCurves'] = [c.upper() for c in ps.policy.key_pair.elliptic_curves]
elif ps.defaults and ps.defaults.key_pair and ps.defaults.key_pair.elliptic_curve:
ec_kt['keyCurves'] = [ps.defaults.key_pair.elliptic_curve]
ec_kt['keyCurves'] = [ps.defaults.key_pair.elliptic_curve.upper()]
else:
ec_kt['keyCurves'] = ['P256']

Expand Down Expand Up @@ -659,7 +661,7 @@ def build_cit_request(ps, ca_details):
r_key['length'] = 2048
elif default_kt == KeyType.ECDSA.upper():
if default_kp.elliptic_curve:
r_key['curve'] = default_kp.elliptic_curve
r_key['curve'] = default_kp.elliptic_curve.upper()
else:
r_key['curve'] = 'P256'

Expand Down