Skip to content
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

Certificate V2 x509_pem type element validation #240

Merged
merged 2 commits into from
Jan 7, 2025
Merged

Certificate V2 x509_pem type element validation #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
90227c4
Certificate V2 x509_pem type element validation
amendelzon Jan 6, 2025
0ec0053
Changes as per PR review
amendelzon Jan 7, 2025
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
36 changes: 32 additions & 4 deletions middleware/admin/certificate_v2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
# SOFTWARE.

import re
from pathlib import Path
import base64
import ecdsa
import hashlib
from datetime import datetime, UTC
from pathlib import Path
from cryptography import x509
from cryptography.hazmat.primitives.serialization import PublicFormat, Encoding
from cryptography.hazmat.primitives.asymmetric.ec import SECP256R1
from cryptography.hazmat.primitives.asymmetric import ec
from .certificate_v1 import HSMCertificate
from .utils import is_nonempty_hex_string
from sgx.envelope import SgxQuote, SgxReportBody
Expand Down Expand Up @@ -254,13 +255,40 @@ def certificate(self):
return self._certificate

def is_valid(self, certifier):
return True
try:
# IMPORTANT: for now, we only allow verifying the validity of an
# HSMCertificateV2ElementX509 using another HSMCertificateV2ElementX509
# instance as certifier. That way, we simplify the validation procedure
# and ensure maximum use of the underlying library's capabilities
# (cryptography)
if not isinstance(certifier, type(self)):
return False

subject = self.certificate
issuer = certifier.certificate
now = datetime.now(UTC)

# 1. Check validity period
if subject.not_valid_before_utc > now or subject.not_valid_after_utc < now:
return False

# 2. Verify the signature
issuer.public_key().verify(
subject.signature,
subject.tbs_certificate_bytes,
ec.ECDSA(subject.signature_hash_algorithm)
)

return True

except Exception:
return False

def get_pubkey(self):
try:
public_key = self.certificate.public_key()

if not isinstance(public_key.curve, SECP256R1):
if not isinstance(public_key.curve, ec.SECP256R1):
raise ValueError("Certificate does not have a NIST P-256 public key")

public_bytes = public_key.public_bytes(
Expand Down
3 changes: 3 additions & 0 deletions middleware/admin/verify_sgx_attestation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ def do_verify_attestation(options):
info(f"Attempting to gather root authority from {root_authority}...")
try:
root_of_trust = get_root_of_trust(root_authority)
info("Attempting to validate self-signed root authority...")
if not root_of_trust.is_valid(root_of_trust):
raise ValueError("Failed to validate self-signed root of trust")
except Exception as e:
raise AdminError(f"Invalid root authority {root_authority}: {e}")
info(f"Using {root_authority} as root authority")
Expand Down
107 changes: 100 additions & 7 deletions middleware/tests/admin/test_certificate_v2_element_x509.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
# SOFTWARE.

from datetime import datetime, timedelta, UTC
from unittest import TestCase
from unittest.mock import Mock, patch
from ecdsa import NIST256p
Expand Down Expand Up @@ -111,7 +112,7 @@ def test_certificate(self, load_pem_x509_certificate):
)
self.assertEqual(1, load_pem_x509_certificate.call_count)

def setup_is_valid_mocks(self, load_pem_x509_certificate, VerifyingKey):
def setup_pubkey_mocks(self, load_pem_x509_certificate, VerifyingKey):
self.pubkey = Mock()
self.pubkey.curve = SECP256R1()
self.pubkey.public_bytes.return_value = "the-public-bytes"
Expand All @@ -123,7 +124,7 @@ def setup_is_valid_mocks(self, load_pem_x509_certificate, VerifyingKey):
@patch("admin.certificate_v2.ecdsa.VerifyingKey")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_get_pubkey_ok(self, load_pem_x509_certificate, VerifyingKey):
self.setup_is_valid_mocks(load_pem_x509_certificate, VerifyingKey)
self.setup_pubkey_mocks(load_pem_x509_certificate, VerifyingKey)

self.assertEqual("the-expected-pubkey", self.elem.get_pubkey())
self.pubkey.public_bytes.assert_called_with(
Expand All @@ -133,7 +134,7 @@ def test_get_pubkey_ok(self, load_pem_x509_certificate, VerifyingKey):
@patch("admin.certificate_v2.ecdsa.VerifyingKey")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_get_pubkey_err_load_cert(self, load_pem_x509_certificate, VerifyingKey):
self.setup_is_valid_mocks(load_pem_x509_certificate, VerifyingKey)
self.setup_pubkey_mocks(load_pem_x509_certificate, VerifyingKey)
load_pem_x509_certificate.side_effect = Exception("blah blah")

with self.assertRaises(ValueError) as e:
Expand All @@ -146,7 +147,7 @@ def test_get_pubkey_err_load_cert(self, load_pem_x509_certificate, VerifyingKey)
@patch("admin.certificate_v2.ecdsa.VerifyingKey")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_get_pubkey_err_get_pub(self, load_pem_x509_certificate, VerifyingKey):
self.setup_is_valid_mocks(load_pem_x509_certificate, VerifyingKey)
self.setup_pubkey_mocks(load_pem_x509_certificate, VerifyingKey)
self.cert.public_key.side_effect = Exception("blah blah")

with self.assertRaises(ValueError) as e:
Expand All @@ -160,7 +161,7 @@ def test_get_pubkey_err_get_pub(self, load_pem_x509_certificate, VerifyingKey):
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_get_pubkey_err_pub_notnistp256(self, load_pem_x509_certificate,
VerifyingKey):
self.setup_is_valid_mocks(load_pem_x509_certificate, VerifyingKey)
self.setup_pubkey_mocks(load_pem_x509_certificate, VerifyingKey)
self.pubkey.curve = "somethingelse"

with self.assertRaises(ValueError) as e:
Expand All @@ -173,7 +174,7 @@ def test_get_pubkey_err_pub_notnistp256(self, load_pem_x509_certificate,
@patch("admin.certificate_v2.ecdsa.VerifyingKey")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_get_pubkey_err_public_bytes(self, load_pem_x509_certificate, VerifyingKey):
self.setup_is_valid_mocks(load_pem_x509_certificate, VerifyingKey)
self.setup_pubkey_mocks(load_pem_x509_certificate, VerifyingKey)
self.pubkey.public_bytes.side_effect = Exception("blah blah")

with self.assertRaises(ValueError) as e:
Expand All @@ -188,7 +189,7 @@ def test_get_pubkey_err_public_bytes(self, load_pem_x509_certificate, VerifyingK
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_get_pubkey_err_ecdsafromstring(self, load_pem_x509_certificate,
VerifyingKey):
self.setup_is_valid_mocks(load_pem_x509_certificate, VerifyingKey)
self.setup_pubkey_mocks(load_pem_x509_certificate, VerifyingKey)
VerifyingKey.from_string.side_effect = Exception("blah blah")

with self.assertRaises(ValueError) as e:
Expand All @@ -198,3 +199,95 @@ def test_get_pubkey_err_ecdsafromstring(self, load_pem_x509_certificate,
self.pubkey.public_bytes.assert_called_with(
Encoding.X962, PublicFormat.CompressedPoint)
VerifyingKey.from_string.assert_called_with("the-public-bytes", NIST256p)

def setup_is_valid_mocks(self, load_pem_x509_certificate, ec):
self.certifier = HSMCertificateV2ElementX509({
"name": "mock-certifier",
"signed_by": "someone-else",
"message": "Y2VydGlmaWVy"
})

self.mock_certifier = Mock()
self.mock_elem = Mock()

def load_mock(data):
if b"Y2VydGlmaWVy" in data:
return self.mock_certifier
return self.mock_elem

load_pem_x509_certificate.side_effect = load_mock

self.now = datetime.now(UTC)
one_week = timedelta(weeks=1)
self.mock_elem.not_valid_before_utc = self.now - one_week
self.mock_elem.not_valid_after_utc = self.now + one_week
self.mock_certifier_pk = Mock()
self.mock_certifier.public_key.return_value = self.mock_certifier_pk
self.mock_elem.signature = "the-signature"
self.mock_elem.tbs_certificate_bytes = "the-fingerprint"
self.mock_elem.signature_hash_algorithm = "the-signature-hash-algo"
ec.ECDSA.return_value = "the-ecdsa-algo"

@patch("admin.certificate_v2.ec")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_is_valid_ok(self, load_pem_x509_certificate, ec):
self.setup_is_valid_mocks(load_pem_x509_certificate, ec)

self.assertTrue(self.elem.is_valid(self.certifier))

self.mock_certifier_pk.verify.assert_called_with(
"the-signature",
"the-fingerprint",
"the-ecdsa-algo"
)
ec.ECDSA.assert_called_with("the-signature-hash-algo")

@patch("admin.certificate_v2.ec")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_is_valid_before_in_future(self, load_pem_x509_certificate, ec):
self.setup_is_valid_mocks(load_pem_x509_certificate, ec)
self.mock_elem.not_valid_before_utc = self.now + \
timedelta(minutes=1)

self.assertFalse(self.elem.is_valid(self.certifier))

self.mock_certifier_pk.verify.assert_not_called()
ec.ECDSA.assert_not_called()

@patch("admin.certificate_v2.ec")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_is_valid_after_in_past(self, load_pem_x509_certificate, ec):
self.setup_is_valid_mocks(load_pem_x509_certificate, ec)
self.mock_elem.not_valid_after_utc = self.now - \
timedelta(minutes=1)

self.assertFalse(self.elem.is_valid(self.certifier))

self.mock_certifier_pk.verify.assert_not_called()
ec.ECDSA.assert_not_called()

@patch("admin.certificate_v2.ec")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_is_valid_signature_invalid(self, load_pem_x509_certificate, ec):
self.setup_is_valid_mocks(load_pem_x509_certificate, ec)
self.mock_certifier_pk.verify.side_effect = RuntimeError("wrong signature")

self.assertFalse(self.elem.is_valid(self.certifier))

self.mock_certifier_pk.verify.assert_called_with(
"the-signature",
"the-fingerprint",
"the-ecdsa-algo"
)
ec.ECDSA.assert_called_with("the-signature-hash-algo")

@patch("admin.certificate_v2.ec")
@patch("admin.certificate_v2.x509.load_pem_x509_certificate")
def test_is_valid_x509_error(self, load_pem_x509_certificate, ec):
self.setup_is_valid_mocks(load_pem_x509_certificate, ec)
load_pem_x509_certificate.side_effect = ValueError("a random error")

self.assertFalse(self.elem.is_valid(self.certifier))

self.mock_certifier_pk.verify.assert_not_called()
ec.ECDSA.assert_not_called()
54 changes: 47 additions & 7 deletions middleware/tests/admin/test_verify_sgx_attestation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ def setUp(self):

def configure_mocks(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head):
get_root_of_trust.return_value = "the-root-of-trust"
self.root_of_trust = Mock()
self.root_of_trust.is_valid.return_value = True
get_root_of_trust.return_value = self.root_of_trust
load_pubkeys.return_value = self.public_keys
self.mock_certificate = Mock()
self.mock_certificate.validate_and_get_values.return_value = self.validate_result
Expand All @@ -116,10 +118,11 @@ def test_verify_attestation(self, get_root_of_trust, load_pubkeys,
get_root_of_trust.assert_called_with(custom_root)
else:
get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_called_with(self.certification_path)
self.mock_certificate.validate_and_get_values \
.assert_called_with("the-root-of-trust")
.assert_called_with(self.root_of_trust)
head.assert_called_with([
"powHSM verified with public keys:"
] + self.expected_pubkeys_output + [
Expand All @@ -135,6 +138,36 @@ def test_verify_attestation(self, get_root_of_trust, load_pubkeys,
"Timestamp: 205",
], fill="-")

def test_verify_attestation_err_get_root(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head, _):
self.configure_mocks(get_root_of_trust, load_pubkeys, HSMCertificate, head)
get_root_of_trust.side_effect = ValueError("root of trust error")

with self.assertRaises(AdminError) as e:
do_verify_attestation(self.options)
self.assertIn("root of trust error", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_not_called()
load_pubkeys.assert_not_called()
HSMCertificate.from_jsonfile.assert_not_called()
self.mock_certificate.validate_and_get_values.assert_not_called()

def test_verify_attestation_err_root_invalid(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head, _):
self.configure_mocks(get_root_of_trust, load_pubkeys, HSMCertificate, head)
self.root_of_trust.is_valid.return_value = False

with self.assertRaises(AdminError) as e:
do_verify_attestation(self.options)
self.assertIn("self-signed root of trust", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_not_called()
HSMCertificate.from_jsonfile.assert_not_called()
self.mock_certificate.validate_and_get_values.assert_not_called()

def test_verify_attestation_err_load_pubkeys(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head, _):
self.configure_mocks(get_root_of_trust, load_pubkeys, HSMCertificate, head)
Expand All @@ -145,6 +178,7 @@ def test_verify_attestation_err_load_pubkeys(self, get_root_of_trust, load_pubke
self.assertIn("pubkeys error", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_not_called()
self.mock_certificate.validate_and_get_values.assert_not_called()
Expand All @@ -159,6 +193,7 @@ def test_verify_attestation_err_load_cert(self, get_root_of_trust, load_pubkeys,
self.assertIn("load cert error", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_called_with(self.certification_path)
self.mock_certificate.validate_and_get_values.assert_not_called()
Expand All @@ -173,10 +208,11 @@ def test_verify_attestation_validation_noquote(self, get_root_of_trust, load_pub
self.assertIn("does not contain", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_called_with(self.certification_path)
self.mock_certificate.validate_and_get_values \
.assert_called_with("the-root-of-trust")
.assert_called_with(self.root_of_trust)

def test_verify_attestation_validation_failed(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head, _):
Expand All @@ -190,10 +226,11 @@ def test_verify_attestation_validation_failed(self, get_root_of_trust, load_pubk
self.assertIn("validation error", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_called_with(self.certification_path)
self.mock_certificate.validate_and_get_values \
.assert_called_with("the-root-of-trust")
.assert_called_with(self.root_of_trust)

def test_verify_attestation_invalid_header(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head, _):
Expand All @@ -205,10 +242,11 @@ def test_verify_attestation_invalid_header(self, get_root_of_trust, load_pubkeys
self.assertIn("message header", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_called_with(self.certification_path)
self.mock_certificate.validate_and_get_values \
.assert_called_with("the-root-of-trust")
.assert_called_with(self.root_of_trust)

def test_verify_attestation_invalid_message(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head, _):
Expand All @@ -220,10 +258,11 @@ def test_verify_attestation_invalid_message(self, get_root_of_trust, load_pubkey
self.assertIn("parsing", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_called_with(self.certification_path)
self.mock_certificate.validate_and_get_values \
.assert_called_with("the-root-of-trust")
.assert_called_with(self.root_of_trust)

def test_verify_attestation_pkh_mismatch(self, get_root_of_trust, load_pubkeys,
HSMCertificate, head, _):
Expand All @@ -235,7 +274,8 @@ def test_verify_attestation_pkh_mismatch(self, get_root_of_trust, load_pubkeys,
self.assertIn("hash mismatch", str(e.exception))

get_root_of_trust.assert_called_with(DEFAULT_ROOT_AUTHORITY)
self.root_of_trust.is_valid.assert_called_with(self.root_of_trust)
load_pubkeys.assert_called_with(self.pubkeys_path)
HSMCertificate.from_jsonfile.assert_called_with(self.certification_path)
self.mock_certificate.validate_and_get_values \
.assert_called_with("the-root-of-trust")
.assert_called_with(self.root_of_trust)