Add support for ECC attestation
Resolves: RHEL-117442 Signed-off-by: Sergio Correia <scorreia@redhat.com>
This commit is contained in:
parent
f1b9a25332
commit
1ed9c6dfe5
382
0013-algorithms-add-support-for-specific-ECC-curve-algori.patch
Normal file
382
0013-algorithms-add-support-for-specific-ECC-curve-algori.patch
Normal file
@ -0,0 +1,382 @@
|
||||
From 7a723f0938edf9ccc597507a4230922e9235cf18 Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Correia <scorreia@redhat.com>
|
||||
Date: Wed, 24 Sep 2025 07:20:53 +0100
|
||||
Subject: [PATCH 13/18] algorithms: add support for specific ECC curve
|
||||
algorithms
|
||||
|
||||
Extended the Encrypt enum to support specific ECC curves including:
|
||||
- ecc192 (P-192)
|
||||
- ecc224 (P-224)
|
||||
- ecc256 (P-256)
|
||||
- ecc384 (P-384)
|
||||
- ecc521 (P-521)
|
||||
|
||||
This enables Keylime to accept and validate different ECC curves
|
||||
for TPM attestation operations.
|
||||
|
||||
Also, when agent reports specific algorithm like 'ecc256' but tenant
|
||||
configuration uses generic 'ecc', the is_accepted function now uses
|
||||
bidirectional normalization to properly match algorithms.
|
||||
|
||||
Signed-off-by: Sergio Correia <scorreia@redhat.com>
|
||||
---
|
||||
keylime/common/algorithms.py | 28 +++-
|
||||
test/test_algorithms.py | 275 ++++++++++++++++++++++++++++++++++-
|
||||
2 files changed, 301 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/keylime/common/algorithms.py b/keylime/common/algorithms.py
|
||||
index db12c26..bb22fb6 100644
|
||||
--- a/keylime/common/algorithms.py
|
||||
+++ b/keylime/common/algorithms.py
|
||||
@@ -9,7 +9,18 @@ def is_accepted(algorithm: str, accepted: List[Any]) -> bool:
|
||||
@param algorithm: algorithm to be checked
|
||||
@param accepted: a list of acceptable algorithms
|
||||
"""
|
||||
- return algorithm in accepted
|
||||
+ # Check direct match first.
|
||||
+ if algorithm in accepted:
|
||||
+ return True
|
||||
+
|
||||
+ # Check if any accepted algorithm normalizes to the same value as our algorithm
|
||||
+ # This handles backwards compatibility cases like "ecc" accepting "ecc256".
|
||||
+ normalized_algorithm = Encrypt.normalize(algorithm)
|
||||
+ for accepted_alg in accepted:
|
||||
+ if Encrypt.normalize(str(accepted_alg)) == normalized_algorithm:
|
||||
+ return True
|
||||
+
|
||||
+ return False
|
||||
|
||||
|
||||
class Hash(str, enum.Enum):
|
||||
@@ -74,11 +85,26 @@ class Hash(str, enum.Enum):
|
||||
class Encrypt(str, enum.Enum):
|
||||
RSA = "rsa"
|
||||
ECC = "ecc"
|
||||
+ ECC192 = "ecc192"
|
||||
+ ECC224 = "ecc224"
|
||||
+ ECC256 = "ecc256"
|
||||
+ ECC384 = "ecc384"
|
||||
+ ECC521 = "ecc521"
|
||||
|
||||
@staticmethod
|
||||
def is_recognized(algorithm: str) -> bool:
|
||||
+ # Handle aliases to match agent behavior
|
||||
+ if algorithm == "ecc":
|
||||
+ algorithm = "ecc256" # Default ECC alias maps to P-256, same as the agent.
|
||||
return algorithm in list(Encrypt)
|
||||
|
||||
+ @staticmethod
|
||||
+ def normalize(algorithm: str) -> str:
|
||||
+ """Normalize algorithm string to handle aliases, matching the agent behavior"""
|
||||
+ if algorithm == "ecc":
|
||||
+ return "ecc256" # Default ECC alias maps to P-256.
|
||||
+ return algorithm
|
||||
+
|
||||
|
||||
class Sign(str, enum.Enum):
|
||||
RSASSA = "rsassa"
|
||||
diff --git a/test/test_algorithms.py b/test/test_algorithms.py
|
||||
index b5a29c7..8a31fa9 100644
|
||||
--- a/test/test_algorithms.py
|
||||
+++ b/test/test_algorithms.py
|
||||
@@ -2,7 +2,7 @@ import os
|
||||
import tempfile
|
||||
import unittest
|
||||
|
||||
-from keylime.common.algorithms import Encrypt, Hash, Sign
|
||||
+from keylime.common.algorithms import Encrypt, Hash, Sign, is_accepted
|
||||
|
||||
|
||||
class TestHash(unittest.TestCase):
|
||||
@@ -117,11 +117,88 @@ class TestEncrypt(unittest.TestCase):
|
||||
"enc": "ecc",
|
||||
"valid": True,
|
||||
},
|
||||
+ {
|
||||
+ "enc": "ecc192",
|
||||
+ "valid": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "enc": "ecc224",
|
||||
+ "valid": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "enc": "ecc256",
|
||||
+ "valid": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "enc": "ecc384",
|
||||
+ "valid": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "enc": "ecc521",
|
||||
+ "valid": True,
|
||||
+ },
|
||||
]
|
||||
|
||||
for c in test_cases:
|
||||
self.assertEqual(Encrypt.is_recognized(c["enc"]), c["valid"], msg=f"enc = {c['enc']}")
|
||||
|
||||
+ def test_enum_membership(self):
|
||||
+ """Test that all ECC curve algorithms are members of the Encrypt enum"""
|
||||
+ self.assertTrue(Encrypt.RSA in Encrypt)
|
||||
+ self.assertTrue(Encrypt.ECC in Encrypt)
|
||||
+ self.assertTrue(Encrypt.ECC192 in Encrypt)
|
||||
+ self.assertTrue(Encrypt.ECC224 in Encrypt)
|
||||
+ self.assertTrue(Encrypt.ECC256 in Encrypt)
|
||||
+ self.assertTrue(Encrypt.ECC384 in Encrypt)
|
||||
+ self.assertTrue(Encrypt.ECC521 in Encrypt)
|
||||
+
|
||||
+ def test_normalize(self):
|
||||
+ """Test the normalize method for handling ECC aliases"""
|
||||
+ test_cases = [
|
||||
+ {
|
||||
+ "input": "ecc",
|
||||
+ "expected": "ecc256",
|
||||
+ },
|
||||
+ {
|
||||
+ "input": "ecc192",
|
||||
+ "expected": "ecc192",
|
||||
+ },
|
||||
+ {
|
||||
+ "input": "ecc224",
|
||||
+ "expected": "ecc224",
|
||||
+ },
|
||||
+ {
|
||||
+ "input": "ecc256",
|
||||
+ "expected": "ecc256",
|
||||
+ },
|
||||
+ {
|
||||
+ "input": "ecc384",
|
||||
+ "expected": "ecc384",
|
||||
+ },
|
||||
+ {
|
||||
+ "input": "ecc521",
|
||||
+ "expected": "ecc521",
|
||||
+ },
|
||||
+ {
|
||||
+ "input": "rsa",
|
||||
+ "expected": "rsa",
|
||||
+ },
|
||||
+ ]
|
||||
+
|
||||
+ for c in test_cases:
|
||||
+ self.assertEqual(Encrypt.normalize(c["input"]), c["expected"], msg=f"input = {c['input']}")
|
||||
+
|
||||
+ def test_normalize_ecc_alias_behavior(self):
|
||||
+ """Test that ECC alias normalization matches agent behavior"""
|
||||
+ # Test that "ecc" is recognized through alias handling
|
||||
+ self.assertTrue(Encrypt.is_recognized("ecc"))
|
||||
+
|
||||
+ # Test that normalize converts "ecc" to "ecc256" (P-256)
|
||||
+ self.assertEqual(Encrypt.normalize("ecc"), "ecc256")
|
||||
+
|
||||
+ # Test that direct ecc256 works
|
||||
+ self.assertTrue(Encrypt.is_recognized("ecc256"))
|
||||
+
|
||||
|
||||
class TestSign(unittest.TestCase):
|
||||
def test_is_recognized(self):
|
||||
@@ -158,3 +235,199 @@ class TestSign(unittest.TestCase):
|
||||
|
||||
for c in test_cases:
|
||||
self.assertEqual(Sign.is_recognized(c["sign"]), c["valid"], msg=f"sign = {c['sign']}")
|
||||
+
|
||||
+
|
||||
+class TestIsAccepted(unittest.TestCase):
|
||||
+ def test_direct_algorithm_matching(self):
|
||||
+ """Test that direct algorithm matches work correctly"""
|
||||
+ test_cases = [
|
||||
+ {
|
||||
+ "algorithm": "ecc256",
|
||||
+ "accepted": ["ecc256"],
|
||||
+ "expected": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "rsa",
|
||||
+ "accepted": ["rsa"],
|
||||
+ "expected": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc384",
|
||||
+ "accepted": ["ecc256", "ecc384"],
|
||||
+ "expected": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc521",
|
||||
+ "accepted": ["ecc256"],
|
||||
+ "expected": False,
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "unknown",
|
||||
+ "accepted": ["rsa", "ecc256"],
|
||||
+ "expected": False,
|
||||
+ },
|
||||
+ ]
|
||||
+
|
||||
+ for c in test_cases:
|
||||
+ result = is_accepted(c["algorithm"], c["accepted"])
|
||||
+ self.assertEqual(result, c["expected"], msg=f"algorithm='{c['algorithm']}', accepted={c['accepted']}")
|
||||
+
|
||||
+ def test_backwards_compatibility_ecc_normalization(self):
|
||||
+ """Test backwards compatibility: 'ecc' in accepted list should accept specific ECC algorithms"""
|
||||
+ test_cases = [
|
||||
+ {
|
||||
+ "algorithm": "ecc256",
|
||||
+ "accepted": ["ecc"],
|
||||
+ "expected": True,
|
||||
+ "desc": "ecc256 should be accepted when 'ecc' is in accepted list",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc384",
|
||||
+ "accepted": ["ecc"],
|
||||
+ "expected": False,
|
||||
+ "desc": "ecc384 should NOT be accepted when only 'ecc' is in accepted list (ecc maps to ecc256)",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc521",
|
||||
+ "accepted": ["ecc"],
|
||||
+ "expected": False,
|
||||
+ "desc": "ecc521 should NOT be accepted when only 'ecc' is in accepted list",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc192",
|
||||
+ "accepted": ["ecc"],
|
||||
+ "expected": False,
|
||||
+ "desc": "ecc192 should NOT be accepted when only 'ecc' is in accepted list",
|
||||
+ },
|
||||
+ ]
|
||||
+
|
||||
+ for c in test_cases:
|
||||
+ result = is_accepted(c["algorithm"], c["accepted"])
|
||||
+ self.assertEqual(
|
||||
+ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}"
|
||||
+ )
|
||||
+
|
||||
+ def test_forward_compatibility_ecc_normalization(self):
|
||||
+ """Test forward compatibility: specific ECC in accepted list should accept 'ecc' algorithm"""
|
||||
+ test_cases = [
|
||||
+ {
|
||||
+ "algorithm": "ecc",
|
||||
+ "accepted": ["ecc256"],
|
||||
+ "expected": True,
|
||||
+ "desc": "ecc should be accepted when 'ecc256' is in accepted list (both normalize to ecc256)",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc",
|
||||
+ "accepted": ["ecc384"],
|
||||
+ "expected": False,
|
||||
+ "desc": "ecc should NOT be accepted when only 'ecc384' is in accepted list",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc",
|
||||
+ "accepted": ["ecc521"],
|
||||
+ "expected": False,
|
||||
+ "desc": "ecc should NOT be accepted when only 'ecc521' is in accepted list",
|
||||
+ },
|
||||
+ ]
|
||||
+
|
||||
+ for c in test_cases:
|
||||
+ result = is_accepted(c["algorithm"], c["accepted"])
|
||||
+ self.assertEqual(
|
||||
+ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}"
|
||||
+ )
|
||||
+
|
||||
+ def test_bidirectional_algorithm_matching(self):
|
||||
+ """Test bidirectional matching scenarios that happen in real usage"""
|
||||
+ test_cases = [
|
||||
+ {
|
||||
+ "algorithm": "ecc256",
|
||||
+ "accepted": ["rsa", "ecc"],
|
||||
+ "expected": True,
|
||||
+ "desc": "Agent reports ecc256, tenant config has generic 'ecc'",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc",
|
||||
+ "accepted": ["rsa", "ecc256"],
|
||||
+ "expected": True,
|
||||
+ "desc": "Agent reports generic 'ecc', tenant config has specific 'ecc256'",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc384",
|
||||
+ "accepted": ["rsa", "ecc"],
|
||||
+ "expected": False,
|
||||
+ "desc": "Agent reports ecc384, tenant has generic 'ecc' (should not match)",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc",
|
||||
+ "accepted": ["rsa", "ecc384"],
|
||||
+ "expected": False,
|
||||
+ "desc": "Agent reports generic 'ecc', tenant has ecc384 (should not match)",
|
||||
+ },
|
||||
+ ]
|
||||
+
|
||||
+ for c in test_cases:
|
||||
+ result = is_accepted(c["algorithm"], c["accepted"])
|
||||
+ self.assertEqual(
|
||||
+ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}"
|
||||
+ )
|
||||
+
|
||||
+ def test_mixed_algorithm_types(self):
|
||||
+ """Test mixing different algorithm types in accepted list"""
|
||||
+ test_cases = [
|
||||
+ {
|
||||
+ "algorithm": "rsa",
|
||||
+ "accepted": ["ecc", "rsa"],
|
||||
+ "expected": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc256",
|
||||
+ "accepted": ["rsa", "ecc"],
|
||||
+ "expected": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc384",
|
||||
+ "accepted": ["rsa", "ecc256", "ecc384"],
|
||||
+ "expected": True,
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "unknown",
|
||||
+ "accepted": ["rsa", "ecc", "ecc384"],
|
||||
+ "expected": False,
|
||||
+ },
|
||||
+ ]
|
||||
+
|
||||
+ for c in test_cases:
|
||||
+ result = is_accepted(c["algorithm"], c["accepted"])
|
||||
+ self.assertEqual(result, c["expected"], msg=f"algorithm='{c['algorithm']}', accepted={c['accepted']}")
|
||||
+
|
||||
+ def test_edge_cases(self):
|
||||
+ """Test edge cases and boundary conditions"""
|
||||
+ test_cases = [
|
||||
+ {"algorithm": "", "accepted": ["ecc"], "expected": False, "desc": "Empty algorithm string"},
|
||||
+ {"algorithm": "ecc256", "accepted": [], "expected": False, "desc": "Empty accepted list"},
|
||||
+ {"algorithm": "ecc256", "accepted": [""], "expected": False, "desc": "Accepted list with empty string"},
|
||||
+ {
|
||||
+ "algorithm": "ECC256",
|
||||
+ "accepted": ["ecc256"],
|
||||
+ "expected": False,
|
||||
+ "desc": "Case sensitivity - uppercase should not match",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc256",
|
||||
+ "accepted": ["ecc"],
|
||||
+ "expected": True,
|
||||
+ "desc": "ecc256 algorithm should match ecc in accepted list",
|
||||
+ },
|
||||
+ {
|
||||
+ "algorithm": "ecc",
|
||||
+ "accepted": ["ecc256"],
|
||||
+ "expected": True,
|
||||
+ "desc": "ecc algorithm should match ecc256 in accepted list",
|
||||
+ },
|
||||
+ ]
|
||||
+
|
||||
+ for c in test_cases:
|
||||
+ result = is_accepted(c["algorithm"], c["accepted"])
|
||||
+ self.assertEqual(
|
||||
+ result, c["expected"], msg=f"{c['desc']} - algorithm='{c['algorithm']}', accepted={c['accepted']}"
|
||||
+ )
|
||||
--
|
||||
2.47.3
|
||||
|
||||
@ -0,0 +1,87 @@
|
||||
From eecd2f73642f784b19cb1bb9c78c6d0b1e486dda Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Correia <scorreia@redhat.com>
|
||||
Date: Fri, 26 Sep 2025 00:03:49 +0100
|
||||
Subject: [PATCH 14/18] algorithms: add support for specific RSA algorithms
|
||||
|
||||
Similar to the previous change for ECC, now we extend the Encrypt enum
|
||||
to support the following specific RSA algorithms:
|
||||
- RSA1024
|
||||
- RSA2048
|
||||
- RSA3072
|
||||
- RSA4096
|
||||
|
||||
Map also 'rsa' to 'rsa2048' for backwards compatibility.
|
||||
|
||||
Signed-off-by: Sergio Correia <scorreia@redhat.com>
|
||||
---
|
||||
keylime/common/algorithms.py | 8 ++++++++
|
||||
test/test_algorithms.py | 13 ++++++++++++-
|
||||
2 files changed, 20 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/keylime/common/algorithms.py b/keylime/common/algorithms.py
|
||||
index bb22fb6..32a1ec1 100644
|
||||
--- a/keylime/common/algorithms.py
|
||||
+++ b/keylime/common/algorithms.py
|
||||
@@ -84,6 +84,10 @@ class Hash(str, enum.Enum):
|
||||
|
||||
class Encrypt(str, enum.Enum):
|
||||
RSA = "rsa"
|
||||
+ RSA1024 = "rsa1024"
|
||||
+ RSA2048 = "rsa2048"
|
||||
+ RSA3072 = "rsa3072"
|
||||
+ RSA4096 = "rsa4096"
|
||||
ECC = "ecc"
|
||||
ECC192 = "ecc192"
|
||||
ECC224 = "ecc224"
|
||||
@@ -96,6 +100,8 @@ class Encrypt(str, enum.Enum):
|
||||
# Handle aliases to match agent behavior
|
||||
if algorithm == "ecc":
|
||||
algorithm = "ecc256" # Default ECC alias maps to P-256, same as the agent.
|
||||
+ if algorithm == "rsa":
|
||||
+ algorithm = "rsa2048" # Default RSA alias maps to RSA-2048, same as the agent.
|
||||
return algorithm in list(Encrypt)
|
||||
|
||||
@staticmethod
|
||||
@@ -103,6 +109,8 @@ class Encrypt(str, enum.Enum):
|
||||
"""Normalize algorithm string to handle aliases, matching the agent behavior"""
|
||||
if algorithm == "ecc":
|
||||
return "ecc256" # Default ECC alias maps to P-256.
|
||||
+ if algorithm == "rsa":
|
||||
+ return "rsa2048" # Default RSA alias maps to RSA-2048.
|
||||
return algorithm
|
||||
|
||||
|
||||
diff --git a/test/test_algorithms.py b/test/test_algorithms.py
|
||||
index 8a31fa9..5542c0f 100644
|
||||
--- a/test/test_algorithms.py
|
||||
+++ b/test/test_algorithms.py
|
||||
@@ -181,7 +181,7 @@ class TestEncrypt(unittest.TestCase):
|
||||
},
|
||||
{
|
||||
"input": "rsa",
|
||||
- "expected": "rsa",
|
||||
+ "expected": "rsa2048",
|
||||
},
|
||||
]
|
||||
|
||||
@@ -199,6 +199,17 @@ class TestEncrypt(unittest.TestCase):
|
||||
# Test that direct ecc256 works
|
||||
self.assertTrue(Encrypt.is_recognized("ecc256"))
|
||||
|
||||
+ def test_normalize_rsa_alias_behavior(self):
|
||||
+ """Test that RSA alias normalization matches agent behavior"""
|
||||
+ # Test that "rsa" is recognized through alias handling
|
||||
+ self.assertTrue(Encrypt.is_recognized("rsa"))
|
||||
+
|
||||
+ # Test that normalize converts "rsa" to "rsa2048"
|
||||
+ self.assertEqual(Encrypt.normalize("rsa"), "rsa2048")
|
||||
+
|
||||
+ # Test that direct rsa2048 works
|
||||
+ self.assertTrue(Encrypt.is_recognized("rsa2048"))
|
||||
+
|
||||
|
||||
class TestSign(unittest.TestCase):
|
||||
def test_is_recognized(self):
|
||||
--
|
||||
2.47.3
|
||||
|
||||
43
0015-tpm_util-fix-quote-signature-extraction-for-ECDSA.patch
Normal file
43
0015-tpm_util-fix-quote-signature-extraction-for-ECDSA.patch
Normal file
@ -0,0 +1,43 @@
|
||||
From 690a2059be01993f5e7f65a01d994e53b82211e4 Mon Sep 17 00:00:00 2001
|
||||
From: Thore Sommer <mail@thson.de>
|
||||
Date: Mon, 3 Mar 2025 15:44:37 +0100
|
||||
Subject: [PATCH 15/18] tpm_util: fix quote signature extraction for ECDSA
|
||||
|
||||
Signed-off-by: Thore Sommer <mail@thson.de>
|
||||
---
|
||||
keylime/tpm/tpm_util.py | 12 +++++++++---
|
||||
1 file changed, 9 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/keylime/tpm/tpm_util.py b/keylime/tpm/tpm_util.py
|
||||
index cdecd32..25c40e0 100644
|
||||
--- a/keylime/tpm/tpm_util.py
|
||||
+++ b/keylime/tpm/tpm_util.py
|
||||
@@ -223,9 +223,7 @@ def checkquote(
|
||||
pcrblob: The state of the PCRs that were quoted; Intel tpm2-tools specific format
|
||||
exp_hash_alg: The hash that was expected to have been used for quoting
|
||||
"""
|
||||
- sig_alg, hash_alg, sig_size = struct.unpack_from(">HHH", sigblob, 0)
|
||||
-
|
||||
- (signature,) = struct.unpack_from(f"{sig_size}s", sigblob, 6)
|
||||
+ sig_alg, hash_alg = struct.unpack_from(">HH", sigblob, 0)
|
||||
|
||||
pubkey = serialization.load_pem_public_key(aikblob, backend=backends.default_backend())
|
||||
if not isinstance(pubkey, (RSAPublicKey, EllipticCurvePublicKey)):
|
||||
@@ -236,6 +234,14 @@ def checkquote(
|
||||
if isinstance(pubkey, EllipticCurvePublicKey) and sig_alg not in [tpm2_objects.TPM_ALG_ECDSA]:
|
||||
raise ValueError(f"Unsupported quote signature algorithm '{sig_alg:#x}' for EC keys")
|
||||
|
||||
+ if sig_alg in [tpm2_objects.TPM_ALG_RSASSA]:
|
||||
+ (sig_size,) = struct.unpack_from(">H", sigblob, 4)
|
||||
+ (signature,) = struct.unpack_from(f"{sig_size}s", sigblob, 6)
|
||||
+ elif sig_alg in [tpm2_objects.TPM_ALG_ECDSA]:
|
||||
+ signature = ecdsa_der_from_tpm(sigblob)
|
||||
+ else:
|
||||
+ raise ValueError(f"Unsupported quote signature algorithm '{sig_alg:#x}'")
|
||||
+
|
||||
hashfunc = tpm2_objects.HASH_FUNCS.get(hash_alg)
|
||||
if not hashfunc:
|
||||
raise ValueError(f"Unsupported hash with id {hash_alg:#x} in signature blob")
|
||||
--
|
||||
2.47.3
|
||||
|
||||
515
0016-tpm-fix-ECC-P-521-coordinate-validation.patch
Normal file
515
0016-tpm-fix-ECC-P-521-coordinate-validation.patch
Normal file
@ -0,0 +1,515 @@
|
||||
From 7c3d81879dba00dcfe917c73b10ca47e8ca7028a Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Correia <scorreia@redhat.com>
|
||||
Date: Sun, 21 Sep 2025 17:49:56 +0100
|
||||
Subject: [PATCH 16/18] tpm: fix ECC P-521 coordinate validation
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
P-521 coordinates can vary from 65-66 bytes due to TPM implementations
|
||||
padding 521-bit values to byte boundaries or stripping leading zeros.
|
||||
The previous validation was too strict, rejecting valid coordinates.
|
||||
|
||||
Enhanced validation:
|
||||
- Accepts P-521 coordinate range 65-66 bytes (520-528 bits)
|
||||
- Validates against actual NIST prime moduli per SEC1 §2.3.5 and
|
||||
FIPS 186-4 App D (coordinates must be < field prime p)
|
||||
- Strict rejection of unknown curves for security
|
||||
|
||||
The enhanced approach prevents false validation of coordinates that are
|
||||
the correct byte length but exceed the curve's field prime.
|
||||
|
||||
Assisted-by: Claude 4 Sonnet
|
||||
Signed-off-by: Sergio Correia <scorreia@redhat.com>
|
||||
---
|
||||
keylime/tpm/tpm2_objects.py | 42 +++-
|
||||
test/test_tpm2_objects.py | 417 ++++++++++++++++++++++++++++++++++++
|
||||
2 files changed, 455 insertions(+), 4 deletions(-)
|
||||
create mode 100644 test/test_tpm2_objects.py
|
||||
|
||||
diff --git a/keylime/tpm/tpm2_objects.py b/keylime/tpm/tpm2_objects.py
|
||||
index fcc5bb5..9170628 100644
|
||||
--- a/keylime/tpm/tpm2_objects.py
|
||||
+++ b/keylime/tpm/tpm2_objects.py
|
||||
@@ -31,6 +31,16 @@ TPM_ECC_NIST_P256 = 0x0003
|
||||
TPM_ECC_NIST_P384 = 0x0004
|
||||
TPM_ECC_NIST_P521 = 0x0005
|
||||
|
||||
+# ECC curve prime moduli lookup table (coordinates must be < p)
|
||||
+# This structure supports NIST curves and can be extended for other curves.
|
||||
+ECC_CURVE_PRIMES = {
|
||||
+ TPM_ECC_NIST_P192: 2**192 - 2**64 - 1, # P-192 prime
|
||||
+ TPM_ECC_NIST_P224: 2**224 - 2**96 + 1, # P-224 prime
|
||||
+ TPM_ECC_NIST_P256: 2**256 - 2**224 + 2**192 + 2**96 - 1, # P-256 prime
|
||||
+ TPM_ECC_NIST_P384: 2**384 - 2**128 - 2**96 + 2**32 - 1, # P-384 prime
|
||||
+ TPM_ECC_NIST_P521: 2**521 - 1, # P-521 prime
|
||||
+}
|
||||
+
|
||||
TPM_ALG_RSA = 0x0001
|
||||
TPM_ALG_ECC = 0x0023
|
||||
|
||||
@@ -318,10 +328,34 @@ def pubkey_parms_from_tpm2b_public(
|
||||
if len(rest) != 0:
|
||||
raise ValueError("Misparsed: more contents after X and Y")
|
||||
|
||||
- if (len(x) * 8) != curve.key_size:
|
||||
- raise ValueError(f"Misparsed either X or curve: {len(x)}*8 != {curve.key_size}")
|
||||
- if (len(y) * 8) != curve.key_size:
|
||||
- raise ValueError(f"Misparsed either Y or curve curve: {len(y)}*8 != {curve.key_size}")
|
||||
+ # ECC coordinates can vary in byte length due to:
|
||||
+ # 1. Padding to byte boundaries (most common)
|
||||
+ # 2. Leading zero stripping in some encodings
|
||||
+ # Validate both byte length and actual coordinate value.
|
||||
+ max_bytes = (curve.key_size + 7) // 8
|
||||
+ min_bytes = max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes
|
||||
+
|
||||
+ # Get the actual prime modulus for the curve
|
||||
+ prime_p = ECC_CURVE_PRIMES.get(curve_id)
|
||||
+ if prime_p is None:
|
||||
+ raise ValueError(f"Unsupported curve ID {curve_id:#x}: prime modulus not known")
|
||||
+
|
||||
+ for label, coord in (("X", x), ("Y", y)):
|
||||
+ coord_len = len(coord)
|
||||
+ if coord_len < min_bytes or coord_len > max_bytes:
|
||||
+ raise ValueError(
|
||||
+ f"Misparsed {label} coordinate: got {coord_len} bytes, "
|
||||
+ f"expected {min_bytes}-{max_bytes} for {curve.key_size}-bit curve"
|
||||
+ )
|
||||
+
|
||||
+ coord_int = int.from_bytes(coord, "big")
|
||||
+ # Coordinates must be reduced modulo the field prime p
|
||||
+ # (SEC1 §2.3.5, FIPS 186-4 App D). Reject values >= p.
|
||||
+ if coord_int >= prime_p:
|
||||
+ raise ValueError(
|
||||
+ f"{label} coordinate too large: {coord_int.bit_length()} bits, "
|
||||
+ f"must be < {prime_p.bit_length()}-bit prime modulus"
|
||||
+ )
|
||||
|
||||
bx = int.from_bytes(x, byteorder="big")
|
||||
by = int.from_bytes(y, byteorder="big")
|
||||
diff --git a/test/test_tpm2_objects.py b/test/test_tpm2_objects.py
|
||||
new file mode 100644
|
||||
index 0000000..c880770
|
||||
--- /dev/null
|
||||
+++ b/test/test_tpm2_objects.py
|
||||
@@ -0,0 +1,417 @@
|
||||
+import struct
|
||||
+import unittest
|
||||
+
|
||||
+from cryptography.hazmat.primitives.asymmetric import ec
|
||||
+
|
||||
+from keylime.tpm.tpm2_objects import (
|
||||
+ ECC_CURVE_PRIMES,
|
||||
+ TPM_ECC_NIST_P192,
|
||||
+ TPM_ECC_NIST_P224,
|
||||
+ TPM_ECC_NIST_P256,
|
||||
+ TPM_ECC_NIST_P384,
|
||||
+ TPM_ECC_NIST_P521,
|
||||
+ _curve_from_curve_id,
|
||||
+ _pack_in_tpm2b,
|
||||
+ pubkey_parms_from_tpm2b_public,
|
||||
+)
|
||||
+
|
||||
+
|
||||
+class TestTpm2Objects(unittest.TestCase):
|
||||
+ def test_p521_coordinate_validation_logic(self):
|
||||
+ """Test the specific coordinate validation logic for P-521"""
|
||||
+ curve = _curve_from_curve_id(TPM_ECC_NIST_P521)
|
||||
+
|
||||
+ # Test the updated validation logic
|
||||
+ max_bytes = (curve.key_size + 7) // 8 # Should be 66 bytes for P-521
|
||||
+ min_bytes = max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes # Should be 65 bytes for P-521
|
||||
+
|
||||
+ self.assertEqual(max_bytes, 66)
|
||||
+ self.assertEqual(min_bytes, 65) # P-521 is not byte-aligned, so allows 65-66 bytes
|
||||
+
|
||||
+ # Test coordinate sizes that should be accepted (65-66 bytes for P-521)
|
||||
+ valid_sizes = [65, 66]
|
||||
+
|
||||
+ for size in valid_sizes:
|
||||
+ # Check that the validation logic would accept this size
|
||||
+ should_pass = min_bytes <= size <= max_bytes
|
||||
+ self.assertTrue(should_pass, f"Size {size} bytes should be valid for P-521")
|
||||
+
|
||||
+ # Test coordinate sizes that should be rejected
|
||||
+ invalid_sizes = [64, 67, 32, 68]
|
||||
+
|
||||
+ for size in invalid_sizes:
|
||||
+ # This should fail: not in the valid range
|
||||
+ should_fail = size < min_bytes or size > max_bytes
|
||||
+ self.assertTrue(should_fail, f"Size {size} bytes should be invalid for P-521")
|
||||
+
|
||||
+ def test_p256_coordinate_validation_logic(self):
|
||||
+ """Test the coordinate validation logic for P-256 to ensure no regression"""
|
||||
+ curve = _curve_from_curve_id(TPM_ECC_NIST_P256)
|
||||
+
|
||||
+ max_bytes = (curve.key_size + 7) // 8 # Should be 32 bytes for P-256
|
||||
+ min_bytes = (
|
||||
+ max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes
|
||||
+ ) # Should be 32 bytes for P-256 (byte-aligned)
|
||||
+
|
||||
+ self.assertEqual(max_bytes, 32)
|
||||
+ self.assertEqual(min_bytes, 32) # P-256 is byte-aligned, so only accepts 32 bytes
|
||||
+
|
||||
+ # 32 bytes should be accepted
|
||||
+ size = 32
|
||||
+ should_pass = min_bytes <= size <= max_bytes
|
||||
+ self.assertTrue(should_pass, f"P-256 should accept {size} bytes")
|
||||
+
|
||||
+ # Other sizes should be rejected
|
||||
+ invalid_sizes = [31, 33, 64]
|
||||
+ for size in invalid_sizes:
|
||||
+ should_fail = size < min_bytes or size > max_bytes
|
||||
+ self.assertTrue(should_fail, f"P-256 should reject {size} bytes")
|
||||
+
|
||||
+ def test_p384_coordinate_validation_logic(self):
|
||||
+ """Test the coordinate validation logic for P-384 to ensure no regression"""
|
||||
+ curve = _curve_from_curve_id(TPM_ECC_NIST_P384)
|
||||
+
|
||||
+ max_bytes = (curve.key_size + 7) // 8 # Should be 48 bytes for P-384
|
||||
+ min_bytes = (
|
||||
+ max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes
|
||||
+ ) # Should be 48 bytes for P-384 (byte-aligned)
|
||||
+
|
||||
+ self.assertEqual(max_bytes, 48)
|
||||
+ self.assertEqual(min_bytes, 48) # P-384 is byte-aligned, so only accepts 48 bytes
|
||||
+
|
||||
+ # 48 bytes should be accepted
|
||||
+ size = 48
|
||||
+ should_pass = min_bytes <= size <= max_bytes
|
||||
+ self.assertTrue(should_pass, f"P-384 should accept {size} bytes")
|
||||
+
|
||||
+ def test_coordinate_size_calculation(self):
|
||||
+ """Test that coordinate size calculations are correct for different curves"""
|
||||
+ # P-256: 256 bits -> (256 + 7) // 8 = 32 bytes
|
||||
+ curve_p256 = _curve_from_curve_id(TPM_ECC_NIST_P256)
|
||||
+ expected_p256 = (curve_p256.key_size + 7) // 8
|
||||
+ self.assertEqual(expected_p256, 32)
|
||||
+ self.assertEqual(curve_p256.key_size, 256)
|
||||
+
|
||||
+ # P-384: 384 bits -> (384 + 7) // 8 = 48 bytes
|
||||
+ curve_p384 = _curve_from_curve_id(TPM_ECC_NIST_P384)
|
||||
+ expected_p384 = (curve_p384.key_size + 7) // 8
|
||||
+ self.assertEqual(expected_p384, 48)
|
||||
+ self.assertEqual(curve_p384.key_size, 384)
|
||||
+
|
||||
+ # P-521: 521 bits -> (521 + 7) // 8 = 66 bytes
|
||||
+ curve_p521 = _curve_from_curve_id(TPM_ECC_NIST_P521)
|
||||
+ expected_p521 = (curve_p521.key_size + 7) // 8
|
||||
+ self.assertEqual(expected_p521, 66)
|
||||
+ self.assertEqual(curve_p521.key_size, 521)
|
||||
+
|
||||
+ def test_p521_specific_fix(self):
|
||||
+ """Test the specific scenario that was fixed: P-521 with 66-byte coordinates"""
|
||||
+ curve = _curve_from_curve_id(TPM_ECC_NIST_P521)
|
||||
+
|
||||
+ # The key issue: P-521 has 521 bits
|
||||
+ self.assertEqual(curve.key_size, 521)
|
||||
+
|
||||
+ # TPMs pad to 66 bytes (528 bits)
|
||||
+ tpm_padded_size = 66
|
||||
+ tpm_padded_bits = tpm_padded_size * 8
|
||||
+ self.assertEqual(tpm_padded_bits, 528)
|
||||
+
|
||||
+ # The old validation would reject: (66 * 8) != 521
|
||||
+ old_validation_fails = tpm_padded_bits != curve.key_size
|
||||
+ self.assertTrue(old_validation_fails, "Old validation would incorrectly reject 66-byte coordinates")
|
||||
+
|
||||
+ # The new validation should accept: len(x) == expected_bytes OR (len(x) * 8) == curve.key_size
|
||||
+ expected_bytes = (curve.key_size + 7) // 8
|
||||
+ new_validation_passes = (tpm_padded_size == expected_bytes) or (tpm_padded_bits == curve.key_size)
|
||||
+ self.assertTrue(new_validation_passes, "New validation should accept 66-byte coordinates")
|
||||
+
|
||||
+ def test_validation_before_and_after_fix(self):
|
||||
+ """Test that demonstrates the fix by comparing old vs new validation logic"""
|
||||
+ curve = _curve_from_curve_id(TPM_ECC_NIST_P521)
|
||||
+
|
||||
+ # Test multiple coordinate sizes that P-521 can have
|
||||
+ test_sizes = [65, 66] # 65 bytes (leading zero stripped), 66 bytes (padded)
|
||||
+
|
||||
+ max_bytes = (curve.key_size + 7) // 8 # 66 bytes
|
||||
+ min_bytes = max_bytes - 1 if curve.key_size % 8 != 0 else max_bytes # 65 bytes for P-521
|
||||
+
|
||||
+ for coordinate_size in test_sizes:
|
||||
+ # Old validation logic (strict bit size match) - would require exactly 65.125 bytes
|
||||
+ # which is impossible since we can't have fractional bytes
|
||||
+
|
||||
+ # New validation logic (accept range for non-byte-aligned curves)
|
||||
+ new_logic_passes = min_bytes <= coordinate_size <= max_bytes
|
||||
+ self.assertTrue(new_logic_passes, f"New logic should accept {coordinate_size}-byte coordinates for P-521")
|
||||
+
|
||||
+ # Verify the calculations
|
||||
+ self.assertEqual(max_bytes, 66)
|
||||
+ self.assertEqual(min_bytes, 65)
|
||||
+
|
||||
+ def test_p521_coordinate_range_validation(self):
|
||||
+ """Test that P-521 accepts coordinates in the range 65-66 bytes (520-528 bits)"""
|
||||
+ curve = _curve_from_curve_id(TPM_ECC_NIST_P521)
|
||||
+
|
||||
+ # P-521: 521 bits, padded to 66 bytes (528 bits), or 65 bytes with leading zero stripped
|
||||
+ max_bytes = (curve.key_size + 7) // 8 # 66 bytes
|
||||
+ min_bytes = max_bytes - 1 # 65 bytes (since 521 % 8 != 0)
|
||||
+
|
||||
+ # Test all valid sizes
|
||||
+ valid_sizes = [65, 66]
|
||||
+ for size in valid_sizes:
|
||||
+ is_valid = min_bytes <= size <= max_bytes
|
||||
+ self.assertTrue(is_valid, f"P-521 should accept {size} bytes ({size * 8} bits)")
|
||||
+
|
||||
+ # Test invalid sizes
|
||||
+ invalid_sizes = [64, 67, 68, 32]
|
||||
+ for size in invalid_sizes:
|
||||
+ is_invalid = size < min_bytes or size > max_bytes
|
||||
+ self.assertTrue(is_invalid, f"P-521 should reject {size} bytes ({size * 8} bits)")
|
||||
+
|
||||
+ def test_coordinate_value_validation(self):
|
||||
+ """Test that coordinate values are validated against actual prime moduli"""
|
||||
+ # Test P-521 with actual prime
|
||||
+ # curve_p521 = _curve_from_curve_id(TPM_ECC_NIST_P521) # Not needed for this test
|
||||
+ p521_prime = ECC_CURVE_PRIMES[TPM_ECC_NIST_P521]
|
||||
+
|
||||
+ # Test valid coordinate value (within range)
|
||||
+ valid_coord_int = p521_prime - 1 # Largest valid value
|
||||
+ valid_coord_bytes = valid_coord_int.to_bytes(66, "big") # 66 bytes, padded
|
||||
+
|
||||
+ # Test the validation logic
|
||||
+ coord_int = int.from_bytes(valid_coord_bytes, "big")
|
||||
+ is_valid_value = coord_int < p521_prime
|
||||
+ self.assertTrue(is_valid_value, "Coordinate value should be valid for P-521")
|
||||
+
|
||||
+ # Test invalid coordinate value (>= prime)
|
||||
+ invalid_coord_int = p521_prime # Equal to prime (invalid)
|
||||
+ invalid_coord_bytes = invalid_coord_int.to_bytes(66, "big") # 66 bytes, but value too large
|
||||
+
|
||||
+ coord_int = int.from_bytes(invalid_coord_bytes, "big")
|
||||
+ is_invalid_value = coord_int >= p521_prime
|
||||
+ self.assertTrue(is_invalid_value, "Coordinate value >= prime should be invalid for P-521")
|
||||
+
|
||||
+ def test_prime_constants_accuracy(self):
|
||||
+ """Test that our hardcoded prime constants are correct"""
|
||||
+ # Verify the NIST prime values
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P192], 2**192 - 2**64 - 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P224], 2**224 - 2**96 + 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P256], 2**256 - 2**224 + 2**192 + 2**96 - 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P384], 2**384 - 2**128 - 2**96 + 2**32 - 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P521], 2**521 - 1)
|
||||
+
|
||||
+ # Verify they are actually less than 2^m for all curves except P-521
|
||||
+ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P192], 2**192)
|
||||
+ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P224], 2**224)
|
||||
+ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P256], 2**256)
|
||||
+ self.assertLess(ECC_CURVE_PRIMES[TPM_ECC_NIST_P384], 2**384)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P521], 2**521 - 1) # P-521 is special case
|
||||
+
|
||||
+ def test_prime_lookup_table(self):
|
||||
+ """Test that the prime lookup table works correctly"""
|
||||
+ # Test known curves
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P192], 2**192 - 2**64 - 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P224], 2**224 - 2**96 + 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P256], 2**256 - 2**224 + 2**192 + 2**96 - 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P384], 2**384 - 2**128 - 2**96 + 2**32 - 1)
|
||||
+ self.assertEqual(ECC_CURVE_PRIMES[TPM_ECC_NIST_P521], 2**521 - 1)
|
||||
+
|
||||
+ # Test rejection of unknown curve
|
||||
+ unknown_curve_id = 0x9999
|
||||
+ unknown_prime = ECC_CURVE_PRIMES.get(unknown_curve_id)
|
||||
+ self.assertIsNone(unknown_prime, "Unknown curves should not be in ECC_CURVE_PRIMES")
|
||||
+
|
||||
+ def test_error_message_formatting(self):
|
||||
+ """Test that error messages use bit_length() instead of full integers"""
|
||||
+ # Create a large coordinate value
|
||||
+ large_value = ECC_CURVE_PRIMES[TPM_ECC_NIST_P521] # This would be hundreds of digits
|
||||
+
|
||||
+ # Verify bit_length() is much more reasonable than the full number
|
||||
+ bit_length = large_value.bit_length()
|
||||
+ self.assertEqual(bit_length, 521) # Much more readable than 150+ digit number
|
||||
+
|
||||
+ # The error message should use bit lengths, not full integers
|
||||
+ expected_msg_pattern = f"coordinate too large: {bit_length} bits"
|
||||
+ self.assertIn("521 bits", expected_msg_pattern)
|
||||
+
|
||||
+ def test_unknown_curve_rejection(self):
|
||||
+ """Test that unknown curves are strictly rejected"""
|
||||
+ # This tests the design decision to be strict rather than use fallbacks
|
||||
+ unknown_curve_id = 0x9999
|
||||
+
|
||||
+ # The strict approach: unknown curves should not have fallback behavior
|
||||
+ # This ensures we only validate curves we explicitly understand
|
||||
+ result = ECC_CURVE_PRIMES.get(unknown_curve_id)
|
||||
+ self.assertIsNone(result, "Unknown curves should be explicitly rejected, not given fallback primes")
|
||||
+
|
||||
+
|
||||
+class TestEccPublicKeySecurityValidation(unittest.TestCase):
|
||||
+ """Test that ECC public key validation includes all required security checks:
|
||||
+ 1. Point is on the curve
|
||||
+ 2. Point is not zero or infinity
|
||||
+ 3. Point is not in a small subgroup (not applicable to NIST curves with cofactor=1)
|
||||
+ """
|
||||
+
|
||||
+ def create_ecc_tpm2b_public(self, x: int, y: int, curve_id: int = TPM_ECC_NIST_P256) -> bytes:
|
||||
+ """Helper to create a TPM2B_PUBLIC structure for ECC key with given coordinates"""
|
||||
+ # Get coordinate size based on curve
|
||||
+ curve = _curve_from_curve_id(curve_id)
|
||||
+ coord_bytes = (curve.key_size + 7) // 8
|
||||
+
|
||||
+ # Convert coordinates to bytes
|
||||
+ x_bytes = x.to_bytes(coord_bytes, "big")
|
||||
+ y_bytes = y.to_bytes(coord_bytes, "big")
|
||||
+
|
||||
+ # Build TPMT_PUBLIC structure
|
||||
+ # alg_type (TPM_ALG_ECC = 0x0023)
|
||||
+ tpmt = struct.pack(">H", 0x0023)
|
||||
+ # name_alg (TPM_ALG_SHA256 = 0x000B)
|
||||
+ tpmt += struct.pack(">H", 0x000B)
|
||||
+ # object_attributes (4 bytes)
|
||||
+ tpmt += struct.pack(">I", 0x00040072)
|
||||
+ # auth_policy (empty TPM2B)
|
||||
+ tpmt += struct.pack(">H", 0)
|
||||
+ # symmetric (TPM_ALG_NULL)
|
||||
+ tpmt += struct.pack(">H", 0x0010)
|
||||
+ # scheme (TPM_ALG_NULL)
|
||||
+ tpmt += struct.pack(">H", 0x0010)
|
||||
+ # curve_id
|
||||
+ tpmt += struct.pack(">H", curve_id)
|
||||
+ # kdf_scheme (TPM_ALG_NULL)
|
||||
+ tpmt += struct.pack(">H", 0x0010)
|
||||
+ # x coordinate (TPM2B)
|
||||
+ tpmt += _pack_in_tpm2b(x_bytes)
|
||||
+ # y coordinate (TPM2B)
|
||||
+ tpmt += _pack_in_tpm2b(y_bytes)
|
||||
+
|
||||
+ # Wrap in TPM2B_PUBLIC
|
||||
+ return _pack_in_tpm2b(tpmt)
|
||||
+
|
||||
+ def test_point_on_curve_validation(self):
|
||||
+ """Test that points not on the curve are rejected (Security Check #1)"""
|
||||
+ # For P-256, the curve equation is: y² = x³ - 3x + b (mod p)
|
||||
+ # Choose coordinates that don't satisfy this equation
|
||||
+ x = 1
|
||||
+ y = 1 # (1, 1) is not on the P-256 curve
|
||||
+
|
||||
+ tpm2b_public = self.create_ecc_tpm2b_public(x, y, TPM_ECC_NIST_P256)
|
||||
+
|
||||
+ # The cryptography library should reject this point as not being on the curve
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ pubkey_parms_from_tpm2b_public(tpm2b_public)
|
||||
+ self.assertIn("not on the curve", str(cm.exception).lower())
|
||||
+
|
||||
+ def test_point_at_infinity_validation(self):
|
||||
+ """Test that the point at infinity (0, 0) is rejected (Security Check #2)"""
|
||||
+ # The point at infinity should be rejected
|
||||
+ x = 0
|
||||
+ y = 0
|
||||
+
|
||||
+ tpm2b_public = self.create_ecc_tpm2b_public(x, y, TPM_ECC_NIST_P256)
|
||||
+
|
||||
+ # The cryptography library should reject the point at infinity
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ pubkey_parms_from_tpm2b_public(tpm2b_public)
|
||||
+ self.assertIn("not on the curve", str(cm.exception).lower())
|
||||
+
|
||||
+ def test_valid_point_accepted(self):
|
||||
+ """Test that a valid point on the curve is accepted"""
|
||||
+ # Generate a valid key and extract its coordinates
|
||||
+ private_key = ec.generate_private_key(ec.SECP256R1())
|
||||
+ public_key = private_key.public_key()
|
||||
+ numbers = public_key.public_numbers()
|
||||
+
|
||||
+ # Create TPM2B_PUBLIC with valid coordinates
|
||||
+ tpm2b_public = self.create_ecc_tpm2b_public(numbers.x, numbers.y, TPM_ECC_NIST_P256)
|
||||
+
|
||||
+ # Should parse successfully
|
||||
+ parsed_key, _ = pubkey_parms_from_tpm2b_public(tpm2b_public)
|
||||
+ self.assertIsInstance(parsed_key, ec.EllipticCurvePublicKey)
|
||||
+
|
||||
+ # Verify the coordinates match
|
||||
+ assert isinstance(parsed_key, ec.EllipticCurvePublicKey) # Type narrowing for pyright
|
||||
+ parsed_numbers = parsed_key.public_numbers()
|
||||
+ self.assertEqual(parsed_numbers.x, numbers.x)
|
||||
+ self.assertEqual(parsed_numbers.y, numbers.y)
|
||||
+
|
||||
+ def test_small_subgroup_not_applicable_to_nist_curves(self):
|
||||
+ """Test documenting that small subgroup checks are not needed for NIST curves (Security Check #3)
|
||||
+
|
||||
+ NIST P-curves (P-192, P-224, P-256, P-384, P-521) all have cofactor h=1,
|
||||
+ meaning the entire curve has prime order. There are no small subgroups to check.
|
||||
+
|
||||
+ Curves with cofactor > 1 (like Curve25519 with h=8) require additional validation
|
||||
+ to ensure the point is not in a small subgroup, but this is not applicable to
|
||||
+ the NIST curves used by TPMs.
|
||||
+ """
|
||||
+ # This test documents the cofactor=1 property for all supported NIST curves
|
||||
+ # The cryptography library's point validation is sufficient for these curves
|
||||
+
|
||||
+ test_curves = [
|
||||
+ (TPM_ECC_NIST_P192, ec.SECP192R1()),
|
||||
+ (TPM_ECC_NIST_P224, ec.SECP224R1()),
|
||||
+ (TPM_ECC_NIST_P256, ec.SECP256R1()),
|
||||
+ (TPM_ECC_NIST_P384, ec.SECP384R1()),
|
||||
+ (TPM_ECC_NIST_P521, ec.SECP521R1()),
|
||||
+ ]
|
||||
+
|
||||
+ for curve_id, curve_obj in test_curves:
|
||||
+ with self.subTest(curve=curve_obj.name):
|
||||
+ try:
|
||||
+ # Generate a valid key for this curve
|
||||
+ # Note: P-192 may not be supported in newer OpenSSL versions
|
||||
+ private_key = ec.generate_private_key(curve_obj)
|
||||
+ except Exception: # pylint: disable=broad-except
|
||||
+ # Skip this specific curve if not supported by OpenSSL (e.g., P-192)
|
||||
+ self.skipTest(f"Curve {curve_obj.name} not supported by OpenSSL")
|
||||
+
|
||||
+ public_key = private_key.public_key()
|
||||
+ numbers = public_key.public_numbers()
|
||||
+
|
||||
+ # Create TPM2B_PUBLIC and verify it parses successfully
|
||||
+ tpm2b_public = self.create_ecc_tpm2b_public(numbers.x, numbers.y, curve_id)
|
||||
+ parsed_key, _ = pubkey_parms_from_tpm2b_public(tpm2b_public)
|
||||
+
|
||||
+ # All NIST curves have cofactor = 1, so no small subgroup attacks possible
|
||||
+ # The point validation by the cryptography library is sufficient
|
||||
+ self.assertIsInstance(parsed_key, ec.EllipticCurvePublicKey)
|
||||
+
|
||||
+ def test_coordinate_exceeds_field_prime_rejected(self):
|
||||
+ """Test that coordinates >= field prime are rejected"""
|
||||
+ # Use a coordinate value that's >= the field prime for P-256
|
||||
+ p256_prime = ECC_CURVE_PRIMES[TPM_ECC_NIST_P256]
|
||||
+
|
||||
+ # x coordinate exceeds the field prime
|
||||
+ x = p256_prime + 1
|
||||
+ y = 1
|
||||
+
|
||||
+ tpm2b_public = self.create_ecc_tpm2b_public(x, y, TPM_ECC_NIST_P256)
|
||||
+
|
||||
+ # Should be rejected during coordinate validation
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ pubkey_parms_from_tpm2b_public(tpm2b_public)
|
||||
+ # Will fail either at coordinate validation or curve validation
|
||||
+ self.assertTrue(
|
||||
+ "coordinate too large" in str(cm.exception).lower() or "not on the curve" in str(cm.exception).lower()
|
||||
+ )
|
||||
+
|
||||
+ def test_p521_point_validation(self):
|
||||
+ """Test point validation works correctly for P-521 (non-byte-aligned curve)"""
|
||||
+ # Generate a valid P-521 key
|
||||
+ private_key = ec.generate_private_key(ec.SECP521R1())
|
||||
+ public_key = private_key.public_key()
|
||||
+ numbers = public_key.public_numbers()
|
||||
+
|
||||
+ # Valid point should be accepted
|
||||
+ tpm2b_public = self.create_ecc_tpm2b_public(numbers.x, numbers.y, TPM_ECC_NIST_P521)
|
||||
+ parsed_key, _ = pubkey_parms_from_tpm2b_public(tpm2b_public)
|
||||
+ self.assertIsInstance(parsed_key, ec.EllipticCurvePublicKey)
|
||||
+
|
||||
+ # Invalid point should be rejected
|
||||
+ tpm2b_public_invalid = self.create_ecc_tpm2b_public(1, 1, TPM_ECC_NIST_P521)
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ pubkey_parms_from_tpm2b_public(tpm2b_public_invalid)
|
||||
+ self.assertIn("not on the curve", str(cm.exception).lower())
|
||||
+
|
||||
+
|
||||
+if __name__ == "__main__":
|
||||
+ unittest.main()
|
||||
--
|
||||
2.47.3
|
||||
|
||||
221
0017-tpm-fix-ECC-P-521-credential-activation-with-consist.patch
Normal file
221
0017-tpm-fix-ECC-P-521-credential-activation-with-consist.patch
Normal file
@ -0,0 +1,221 @@
|
||||
From 2acb35cb5b203f08aa281c571d341406ff1602c2 Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Correia <scorreia@redhat.com>
|
||||
Date: Thu, 25 Sep 2025 14:25:18 +0100
|
||||
Subject: [PATCH 17/18] tpm: fix ECC P-521 credential activation with
|
||||
consistent marshaling
|
||||
|
||||
The TPM credential activation was failing for P-521 curves due to
|
||||
inconsistent ECC point marshaling in tpms_ecc_point_marshal().
|
||||
|
||||
The function used bit_length() which varies for P-521 coordinates
|
||||
(520-521 bits), producing different blob sizes and causing TPM
|
||||
integrity check failures during ActivateCredential operations.
|
||||
|
||||
Assisted-by: Claude 4 Sonnet
|
||||
Signed-off-by: Sergio Correia <scorreia@redhat.com>
|
||||
---
|
||||
keylime/tpm/tpm2_objects.py | 11 +--
|
||||
keylime/tpm/tpm_util.py | 7 +-
|
||||
test/test_tpm2_objects.py | 130 ++++++++++++++++++++++++++++++++++++
|
||||
3 files changed, 142 insertions(+), 6 deletions(-)
|
||||
|
||||
diff --git a/keylime/tpm/tpm2_objects.py b/keylime/tpm/tpm2_objects.py
|
||||
index 9170628..d33ebaa 100644
|
||||
--- a/keylime/tpm/tpm2_objects.py
|
||||
+++ b/keylime/tpm/tpm2_objects.py
|
||||
@@ -597,9 +597,12 @@ def unmarshal_tpml_pcr_selection(tpml_pcr_selection: bytes) -> Tuple[Dict[int, i
|
||||
|
||||
def tpms_ecc_point_marshal(public_key: EllipticCurvePublicKey) -> bytes:
|
||||
pn = public_key.public_numbers()
|
||||
+ curve = public_key.curve
|
||||
|
||||
- sz = (pn.x.bit_length() + 7) // 8
|
||||
- secret = struct.pack(">H", sz) + pn.x.to_bytes(sz, "big")
|
||||
+ # Use fixed coordinate size based on curve to ensure consistent marshaling
|
||||
+ # This is critical for P-521 where bit_length() can vary (520-521 bits)
|
||||
+ # leading to credential activation failures due to inconsistent blob sizes
|
||||
+ coord_size = (curve.key_size + 7) // 8
|
||||
|
||||
- sz = (pn.y.bit_length() + 7) // 8
|
||||
- return secret + struct.pack(">H", sz) + pn.y.to_bytes(sz, "big")
|
||||
+ secret = struct.pack(">H", coord_size) + pn.x.to_bytes(coord_size, "big")
|
||||
+ return secret + struct.pack(">H", coord_size) + pn.y.to_bytes(coord_size, "big")
|
||||
diff --git a/keylime/tpm/tpm_util.py b/keylime/tpm/tpm_util.py
|
||||
index 25c40e0..fbbe557 100644
|
||||
--- a/keylime/tpm/tpm_util.py
|
||||
+++ b/keylime/tpm/tpm_util.py
|
||||
@@ -318,11 +318,14 @@ def crypt_secret_encrypt_ecc(public_key: EllipticCurvePublicKey, hashfunc: hashe
|
||||
|
||||
digest_size = hashfunc.digest_size
|
||||
|
||||
+ # Use fixed coordinate size for consistent marshaling
|
||||
+ coord_size = (public_key.curve.key_size + 7) // 8
|
||||
+
|
||||
x = my_public_key.public_numbers().x
|
||||
- party_x = x.to_bytes((x.bit_length() + 7) >> 3, "big")
|
||||
+ party_x = x.to_bytes(coord_size, "big")
|
||||
|
||||
x = public_key.public_numbers().x
|
||||
- party_y = x.to_bytes((x.bit_length() + 7) >> 3, "big")
|
||||
+ party_y = x.to_bytes(coord_size, "big")
|
||||
|
||||
data = crypt_kdfe(hashfunc, ecc_secret_x, "IDENTITY", party_x, party_y, digest_size << 3)
|
||||
|
||||
diff --git a/test/test_tpm2_objects.py b/test/test_tpm2_objects.py
|
||||
index c880770..48d6a43 100644
|
||||
--- a/test/test_tpm2_objects.py
|
||||
+++ b/test/test_tpm2_objects.py
|
||||
@@ -1,6 +1,7 @@
|
||||
import struct
|
||||
import unittest
|
||||
|
||||
+from cryptography.hazmat.primitives import hashes
|
||||
from cryptography.hazmat.primitives.asymmetric import ec
|
||||
|
||||
from keylime.tpm.tpm2_objects import (
|
||||
@@ -13,7 +14,9 @@ from keylime.tpm.tpm2_objects import (
|
||||
_curve_from_curve_id,
|
||||
_pack_in_tpm2b,
|
||||
pubkey_parms_from_tpm2b_public,
|
||||
+ tpms_ecc_point_marshal,
|
||||
)
|
||||
+from keylime.tpm.tpm_util import crypt_secret_encrypt_ecc
|
||||
|
||||
|
||||
class TestTpm2Objects(unittest.TestCase):
|
||||
@@ -413,5 +416,132 @@ class TestEccPublicKeySecurityValidation(unittest.TestCase):
|
||||
self.assertIn("not on the curve", str(cm.exception).lower())
|
||||
|
||||
|
||||
+class TestEccMarshaling(unittest.TestCase):
|
||||
+ """Test ECC point marshaling consistency fixes"""
|
||||
+
|
||||
+ def test_p521_marshaling_consistency(self):
|
||||
+ """Test that P-521 marshaling produces consistent blob sizes regardless of coordinate values"""
|
||||
+ # Generate multiple P-521 keys to test with different coordinate values
|
||||
+ keys = []
|
||||
+ for _ in range(10):
|
||||
+ private_key = ec.generate_private_key(ec.SECP521R1())
|
||||
+ keys.append(private_key.public_key())
|
||||
+
|
||||
+ # Marshal all keys and check that blob sizes are consistent
|
||||
+ blob_sizes = []
|
||||
+ for key in keys:
|
||||
+ blob = tpms_ecc_point_marshal(key)
|
||||
+ blob_sizes.append(len(blob))
|
||||
+
|
||||
+ # All blobs should be the same size for P-521
|
||||
+ self.assertEqual(len(set(blob_sizes)), 1, "All P-521 marshaled blobs should have the same size")
|
||||
+
|
||||
+ # Expected size: 2 bytes (x size) + 66 bytes (x coord) + 2 bytes (y size) + 66 bytes (y coord) = 136 bytes
|
||||
+ expected_size = 2 + 66 + 2 + 66
|
||||
+ self.assertEqual(blob_sizes[0], expected_size, f"P-521 marshaled blob should be {expected_size} bytes")
|
||||
+
|
||||
+ def test_marshaling_coordinate_sizes(self):
|
||||
+ """Test that marshaled coordinates use fixed sizes based on curve key size"""
|
||||
+ # Test P-521: 521 bits -> (521 + 7) // 8 = 66 bytes per coordinate
|
||||
+ p521_key = ec.generate_private_key(ec.SECP521R1()).public_key()
|
||||
+ p521_blob = tpms_ecc_point_marshal(p521_key)
|
||||
+
|
||||
+ # Parse the blob to check coordinate sizes
|
||||
+ x_size = struct.unpack(">H", p521_blob[:2])[0]
|
||||
+ y_size = struct.unpack(">H", p521_blob[2 + x_size : 2 + x_size + 2])[0]
|
||||
+
|
||||
+ self.assertEqual(x_size, 66, "P-521 X coordinate should be 66 bytes")
|
||||
+ self.assertEqual(y_size, 66, "P-521 Y coordinate should be 66 bytes")
|
||||
+
|
||||
+ # Test P-256: 256 bits -> (256 + 7) // 8 = 32 bytes per coordinate
|
||||
+ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key()
|
||||
+ p256_blob = tpms_ecc_point_marshal(p256_key)
|
||||
+
|
||||
+ x_size = struct.unpack(">H", p256_blob[:2])[0]
|
||||
+ y_size = struct.unpack(">H", p256_blob[2 + x_size : 2 + x_size + 2])[0]
|
||||
+
|
||||
+ self.assertEqual(x_size, 32, "P-256 X coordinate should be 32 bytes")
|
||||
+ self.assertEqual(y_size, 32, "P-256 Y coordinate should be 32 bytes")
|
||||
+
|
||||
+ def test_p521_credential_activation_consistency(self):
|
||||
+ """Test the specific issue: P-521 credential activation with consistent marshaling"""
|
||||
+ # This test verifies the fix for credential activation failures
|
||||
+ # Generate two P-521 keys with potentially different bit lengths for coordinates
|
||||
+ key1 = ec.generate_private_key(ec.SECP521R1()).public_key()
|
||||
+ key2 = ec.generate_private_key(ec.SECP521R1()).public_key()
|
||||
+
|
||||
+ # Marshal both keys
|
||||
+ blob1 = tpms_ecc_point_marshal(key1)
|
||||
+ blob2 = tpms_ecc_point_marshal(key2)
|
||||
+
|
||||
+ # The critical fix: both blobs should be the same size regardless of coordinate bit lengths
|
||||
+ self.assertEqual(
|
||||
+ len(blob1), len(blob2), "P-521 marshaled blobs must be same size regardless of coordinate bit lengths"
|
||||
+ )
|
||||
+
|
||||
+ # Both should use the fixed coordinate size (66 bytes)
|
||||
+ expected_total_size = 2 + 66 + 2 + 66 # size_x + x + size_y + y
|
||||
+ self.assertEqual(len(blob1), expected_total_size)
|
||||
+ self.assertEqual(len(blob2), expected_total_size)
|
||||
+
|
||||
+ def test_marshaling_format_correctness(self):
|
||||
+ """Test that marshaling follows the correct TPM format: size(2) + coord(n) + size(2) + coord(n)"""
|
||||
+ key = ec.generate_private_key(ec.SECP521R1()).public_key()
|
||||
+ blob = tpms_ecc_point_marshal(key)
|
||||
+
|
||||
+ # Parse the blob structure
|
||||
+ if len(blob) < 4:
|
||||
+ self.fail("Marshaled blob too short")
|
||||
+
|
||||
+ x_size = struct.unpack(">H", blob[:2])[0]
|
||||
+ self.assertEqual(x_size, 66, "X coordinate size should be 66 for P-521")
|
||||
+
|
||||
+ if len(blob) < 2 + x_size + 2:
|
||||
+ self.fail("Marshaled blob missing Y coordinate size")
|
||||
+
|
||||
+ y_size = struct.unpack(">H", blob[2 + x_size : 2 + x_size + 2])[0]
|
||||
+ self.assertEqual(y_size, 66, "Y coordinate size should be 66 for P-521")
|
||||
+
|
||||
+ # Total size should be: 2 + 66 + 2 + 66 = 136
|
||||
+ expected_total = 2 + x_size + 2 + y_size
|
||||
+ self.assertEqual(len(blob), expected_total, "Total marshaled blob size incorrect")
|
||||
+
|
||||
+ def test_crypt_secret_encrypt_ecc_consistency(self):
|
||||
+ """Test that crypt_secret_encrypt_ecc produces consistent results with fixed coordinate sizes"""
|
||||
+ # Generate a P-521 key to test with
|
||||
+ public_key = ec.generate_private_key(ec.SECP521R1()).public_key()
|
||||
+ hashfunc = hashes.SHA256()
|
||||
+
|
||||
+ # Call the function multiple times and check consistency
|
||||
+ results = []
|
||||
+ for _ in range(5):
|
||||
+ data, point = crypt_secret_encrypt_ecc(public_key, hashfunc)
|
||||
+ results.append((data, point))
|
||||
+
|
||||
+ # Check that all returned points have consistent marshaling
|
||||
+ # (the data will be different due to random key generation, but point marshaling should be consistent)
|
||||
+ point_sizes = [len(point) for _, point in results]
|
||||
+ self.assertEqual(len(set(point_sizes)), 1, "All marshaled points should have the same size")
|
||||
+
|
||||
+ # For P-521, the marshaled point should be 136 bytes (2+66+2+66)
|
||||
+ expected_point_size = 2 + 66 + 2 + 66
|
||||
+ self.assertEqual(
|
||||
+ point_sizes[0], expected_point_size, f"P-521 marshaled point should be {expected_point_size} bytes"
|
||||
+ )
|
||||
+
|
||||
+ # All data results should be different (due to random ephemeral keys)
|
||||
+ data_results = [data for data, _ in results]
|
||||
+ self.assertEqual(
|
||||
+ len(set(data_results)),
|
||||
+ len(data_results),
|
||||
+ "All data results should be different due to random ephemeral keys",
|
||||
+ )
|
||||
+
|
||||
+ # All data results should have the same length (SHA256 digest size)
|
||||
+ data_sizes = [len(data) for data, _ in results]
|
||||
+ self.assertEqual(len(set(data_sizes)), 1, "All data results should have the same size")
|
||||
+ self.assertEqual(data_sizes[0], hashfunc.digest_size, "Data size should match hash digest size")
|
||||
+
|
||||
+
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
--
|
||||
2.47.3
|
||||
|
||||
372
0018-tpm-fix-ECC-signature-parsing-to-support-variable-le.patch
Normal file
372
0018-tpm-fix-ECC-signature-parsing-to-support-variable-le.patch
Normal file
@ -0,0 +1,372 @@
|
||||
From ee4192df70384fa6b23f359a287e042103ba4ea9 Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Correia <scorreia@redhat.com>
|
||||
Date: Thu, 25 Sep 2025 14:37:10 +0100
|
||||
Subject: [PATCH 18/18] tpm: fix ECC signature parsing to support
|
||||
variable-length coordinates
|
||||
|
||||
The previous ECC signature validation implementation incorrectly assumed
|
||||
fixed-length coordinate encoding, causing failures with mathematically
|
||||
correct variable-length coordinates (especially P-521 curves where
|
||||
coordinates are typically 65-66 bytes).
|
||||
|
||||
This commit fixes the ecdsa_der_from_tpm() function to properly handle
|
||||
TSS ESAPI signature format where the sig_size field contains only the
|
||||
r component size, followed by the s component with its own size header.
|
||||
|
||||
This enables proper ECC attestation for the supported NIST curves.
|
||||
|
||||
Assisted-by: Claude 4 Sonnet
|
||||
Signed-off-by: Sergio Correia <scorreia@redhat.com>
|
||||
---
|
||||
keylime/tpm/tpm_main.py | 2 +-
|
||||
keylime/tpm/tpm_util.py | 105 +++++++++++++++++++---
|
||||
test/test_tpm2_objects.py | 184 +++++++++++++++++++++++++++++++++++++-
|
||||
3 files changed, 277 insertions(+), 14 deletions(-)
|
||||
|
||||
diff --git a/keylime/tpm/tpm_main.py b/keylime/tpm/tpm_main.py
|
||||
index 6f2e89f..ecbacbe 100644
|
||||
--- a/keylime/tpm/tpm_main.py
|
||||
+++ b/keylime/tpm/tpm_main.py
|
||||
@@ -91,7 +91,7 @@ class Tpm:
|
||||
if isinstance(iak_pub, EllipticCurvePublicKey):
|
||||
if sig_alg in [tpm2_objects.TPM_ALG_ECDSA]:
|
||||
try:
|
||||
- der_sig = tpm_util.ecdsa_der_from_tpm(iak_sign)
|
||||
+ der_sig = tpm_util.ecdsa_der_from_tpm(iak_sign, iak_pub)
|
||||
tpm_util.verify(iak_pub, der_sig, digest, hashfunc)
|
||||
logger.info("Agent %s AIK verified with IAK", uuid)
|
||||
return True
|
||||
diff --git a/keylime/tpm/tpm_util.py b/keylime/tpm/tpm_util.py
|
||||
index fbbe557..f554f94 100644
|
||||
--- a/keylime/tpm/tpm_util.py
|
||||
+++ b/keylime/tpm/tpm_util.py
|
||||
@@ -59,6 +59,43 @@ logger = keylime_logging.init_logging("tpm_util")
|
||||
|
||||
SupportedKeyTypes = Union[RSAPublicKey, EllipticCurvePublicKey]
|
||||
|
||||
+# ECC signature parsing constants.
|
||||
+# Raw signature sizes for different ECC curves (r||s concatenated format).
|
||||
+# r and s are the two mathematical components of an ECDSA signature.
|
||||
+ECC_SECP192R1_SIGNATURE_SIZE = 48 # 24 bytes each for r,s.
|
||||
+ECC_SECP224R1_SIGNATURE_SIZE = 56 # 28 bytes each for r,s.
|
||||
+ECC_SECP256R1_SIGNATURE_SIZE = 64 # 32 bytes each for r,s.
|
||||
+ECC_SECP384R1_SIGNATURE_SIZE = 96 # 48 bytes each for r,s.
|
||||
+ECC_SECP521R1_SIGNATURE_SIZE = 132 # 66 bytes each for r,s.
|
||||
+
|
||||
+# TPM2B_ECDSA_SIGNATURE format constants.
|
||||
+TPM2B_SIZE_FIELD_LENGTH = 2 # 2 bytes for size field.
|
||||
+TPM2B_MIN_HEADER_SIZE = 4 # Minimum: 2 bytes r_size + 2 bytes s_size.
|
||||
+
|
||||
+# DER encoding constants.
|
||||
+DER_SEQUENCE_TAG = 0x30
|
||||
+
|
||||
+# Signature blob header offset (skip alg, hash_alg, sig_size headers).
|
||||
+SIGNATURE_BLOB_HEADER_SIZE = 6
|
||||
+
|
||||
+# ECC curve to signature size mapping for raw r||s format.
|
||||
+ECC_RAW_SIGNATURE_SIZES = {
|
||||
+ "secp192r1": ECC_SECP192R1_SIGNATURE_SIZE,
|
||||
+ "secp224r1": ECC_SECP224R1_SIGNATURE_SIZE,
|
||||
+ "secp256r1": ECC_SECP256R1_SIGNATURE_SIZE,
|
||||
+ "secp384r1": ECC_SECP384R1_SIGNATURE_SIZE,
|
||||
+ "secp521r1": ECC_SECP521R1_SIGNATURE_SIZE,
|
||||
+}
|
||||
+
|
||||
+# ECC curve coordinate size ranges (min, max) for validation.
|
||||
+ECC_COORDINATE_SIZE_RANGES = {
|
||||
+ "secp192r1": (1, 24), # 192 bits = 24 bytes max
|
||||
+ "secp224r1": (1, 28), # 224 bits = 28 bytes max
|
||||
+ "secp256r1": (1, 32), # 256 bits = 32 bytes max
|
||||
+ "secp384r1": (1, 48), # 384 bits = 48 bytes max
|
||||
+ "secp521r1": (1, 66), # 521 bits = 66 bytes max (521 bits, not 512)
|
||||
+}
|
||||
+
|
||||
|
||||
def verify(
|
||||
pubkey: SupportedKeyTypes,
|
||||
@@ -106,17 +143,59 @@ def der_len(encoded_int_len: int) -> bytes:
|
||||
return bytes((0x80 | len(bin_str),)) + bin_str
|
||||
|
||||
|
||||
-def ecdsa_der_from_tpm(sigblob: bytes) -> bytes:
|
||||
- _, _, sig_size_r = struct.unpack_from(">HHH", sigblob, 0)
|
||||
- sig_r = sigblob[6 : 6 + sig_size_r]
|
||||
- encoded_sig_r = der_int(sig_r)
|
||||
- sigblob = sigblob[6 + sig_size_r :]
|
||||
- sig_size_s = struct.unpack_from(">H", sigblob, 0)[0]
|
||||
- sig_s = sigblob[2 : 2 + sig_size_s]
|
||||
- encoded_sig_s = der_int(sig_s)
|
||||
- total_size = len(encoded_sig_r) + len(encoded_sig_s)
|
||||
- der_sig = bytes.fromhex(f"30{total_size:x}") + encoded_sig_r + encoded_sig_s
|
||||
- return der_sig
|
||||
+def ecdsa_der_from_tpm(sigblob: bytes, pubkey: EllipticCurvePublicKey) -> bytes:
|
||||
+ """Convert ECC signature from TPM format to DER format for cryptographic verification.
|
||||
+
|
||||
+ This function handles TSS ESAPI signature format where the signature header's
|
||||
+ sig_size field contains the size of the r component, followed by the s component
|
||||
+ with its own size header.
|
||||
+
|
||||
+ Parameters
|
||||
+ ----------
|
||||
+ sigblob: TPM signature blob containing signature headers and signature data
|
||||
+ pubkey: ECC public key to determine expected signature size
|
||||
+
|
||||
+ Returns
|
||||
+ -------
|
||||
+ DER-encoded ECDSA signature suitable for cryptographic library verification
|
||||
+
|
||||
+ Raises
|
||||
+ ------
|
||||
+ ValueError: If signature format cannot be parsed or is invalid
|
||||
+ """
|
||||
+ # Extract signature header information.
|
||||
+ _sig_alg, _hash_alg, sig_size_r = struct.unpack_from(">HHH", sigblob, 0)
|
||||
+
|
||||
+ # Extract the r component (size is in sig_size_r field).
|
||||
+ sig_r = sigblob[SIGNATURE_BLOB_HEADER_SIZE : SIGNATURE_BLOB_HEADER_SIZE + sig_size_r]
|
||||
+
|
||||
+ # The s component follows immediately after r, with its own size header.
|
||||
+ s_offset = SIGNATURE_BLOB_HEADER_SIZE + sig_size_r
|
||||
+ if s_offset + 2 <= len(sigblob):
|
||||
+ sig_size_s = struct.unpack_from(">H", sigblob, s_offset)[0]
|
||||
+ s_start = s_offset + 2
|
||||
+ if s_start + sig_size_s <= len(sigblob):
|
||||
+ sig_s = sigblob[s_start : s_start + sig_size_s]
|
||||
+
|
||||
+ # Validate coordinate sizes against curve requirements.
|
||||
+ curve_name = pubkey.curve.name
|
||||
+ coordinate_range = ECC_COORDINATE_SIZE_RANGES.get(curve_name)
|
||||
+ if coordinate_range:
|
||||
+ min_size, max_size = coordinate_range
|
||||
+ if not min_size <= len(sig_r) <= max_size:
|
||||
+ raise ValueError(f"Invalid r coordinate size {len(sig_r)} for curve {curve_name}")
|
||||
+ if not min_size <= len(sig_s) <= max_size:
|
||||
+ raise ValueError(f"Invalid s coordinate size {len(sig_s)} for curve {curve_name}")
|
||||
+
|
||||
+ # Convert to DER format.
|
||||
+ encoded_sig_r = der_int(sig_r)
|
||||
+ encoded_sig_s = der_int(sig_s)
|
||||
+ total_size = len(encoded_sig_r) + len(encoded_sig_s)
|
||||
+ der_length = der_len(total_size)
|
||||
+ der_sig = bytes([DER_SEQUENCE_TAG]) + der_length + encoded_sig_r + encoded_sig_s
|
||||
+ return der_sig
|
||||
+
|
||||
+ raise ValueError("Unable to parse ECC signature from TPM format")
|
||||
|
||||
|
||||
def __get_pcrs_from_blob(pcrblob: bytes) -> Tuple[int, Dict[int, int], List[bytes]]:
|
||||
@@ -238,7 +317,9 @@ def checkquote(
|
||||
(sig_size,) = struct.unpack_from(">H", sigblob, 4)
|
||||
(signature,) = struct.unpack_from(f"{sig_size}s", sigblob, 6)
|
||||
elif sig_alg in [tpm2_objects.TPM_ALG_ECDSA]:
|
||||
- signature = ecdsa_der_from_tpm(sigblob)
|
||||
+ if not isinstance(pubkey, EllipticCurvePublicKey):
|
||||
+ raise ValueError(f"ECDSA signature algorithm requires EllipticCurvePublicKey, got {type(pubkey)}")
|
||||
+ signature = ecdsa_der_from_tpm(sigblob, pubkey)
|
||||
else:
|
||||
raise ValueError(f"Unsupported quote signature algorithm '{sig_alg:#x}'")
|
||||
|
||||
diff --git a/test/test_tpm2_objects.py b/test/test_tpm2_objects.py
|
||||
index 48d6a43..c0e4c0a 100644
|
||||
--- a/test/test_tpm2_objects.py
|
||||
+++ b/test/test_tpm2_objects.py
|
||||
@@ -16,7 +16,7 @@ from keylime.tpm.tpm2_objects import (
|
||||
pubkey_parms_from_tpm2b_public,
|
||||
tpms_ecc_point_marshal,
|
||||
)
|
||||
-from keylime.tpm.tpm_util import crypt_secret_encrypt_ecc
|
||||
+from keylime.tpm.tpm_util import crypt_secret_encrypt_ecc, der_int, der_len, ecdsa_der_from_tpm
|
||||
|
||||
|
||||
class TestTpm2Objects(unittest.TestCase):
|
||||
@@ -543,5 +543,187 @@ class TestEccMarshaling(unittest.TestCase):
|
||||
self.assertEqual(data_sizes[0], hashfunc.digest_size, "Data size should match hash digest size")
|
||||
|
||||
|
||||
+class TestEccSignatureParsing(unittest.TestCase):
|
||||
+ """Test ECC signature parsing improvements for variable-length coordinates"""
|
||||
+
|
||||
+ def create_test_signature_blob(self, sig_r: bytes, sig_s: bytes) -> bytes:
|
||||
+ """Create a test TPM signature blob with given r and s components"""
|
||||
+ # TPM signature format: sig_alg(2) + hash_alg(2) + sig_size_r(2) + r_data + sig_size_s(2) + s_data
|
||||
+ sig_alg = 0x0018 # TPM_ALG_ECDSA
|
||||
+ hash_alg = 0x000B # TPM_ALG_SHA256
|
||||
+
|
||||
+ blob = struct.pack(">HHH", sig_alg, hash_alg, len(sig_r))
|
||||
+ blob += sig_r
|
||||
+ blob += struct.pack(">H", len(sig_s))
|
||||
+ blob += sig_s
|
||||
+
|
||||
+ return blob
|
||||
+
|
||||
+ def test_p521_variable_length_coordinates(self):
|
||||
+ """Test that P-521 signatures with variable-length coordinates are parsed correctly"""
|
||||
+ # Generate a P-521 key for testing
|
||||
+ private_key = ec.generate_private_key(ec.SECP521R1())
|
||||
+ public_key = private_key.public_key()
|
||||
+
|
||||
+ # Test with 65-byte coordinates (leading zero stripped)
|
||||
+ sig_r_65 = b"\x00" * 1 + b"\x01" * 64 # 65 bytes
|
||||
+ sig_s_65 = b"\x00" * 1 + b"\x02" * 64 # 65 bytes
|
||||
+
|
||||
+ blob_65 = self.create_test_signature_blob(sig_r_65, sig_s_65)
|
||||
+
|
||||
+ # Should parse successfully
|
||||
+ der_sig_65 = ecdsa_der_from_tpm(blob_65, public_key)
|
||||
+ self.assertIsInstance(der_sig_65, bytes)
|
||||
+ self.assertTrue(len(der_sig_65) > 0)
|
||||
+
|
||||
+ # Test with 66-byte coordinates (full padding)
|
||||
+ sig_r_66 = b"\x00" * 2 + b"\x01" * 64 # 66 bytes
|
||||
+ sig_s_66 = b"\x00" * 2 + b"\x02" * 64 # 66 bytes
|
||||
+
|
||||
+ blob_66 = self.create_test_signature_blob(sig_r_66, sig_s_66)
|
||||
+
|
||||
+ # Should parse successfully
|
||||
+ der_sig_66 = ecdsa_der_from_tpm(blob_66, public_key)
|
||||
+ self.assertIsInstance(der_sig_66, bytes)
|
||||
+ self.assertTrue(len(der_sig_66) > 0)
|
||||
+
|
||||
+ def test_coordinate_size_validation(self):
|
||||
+ """Test that coordinate size validation works for different curves"""
|
||||
+ # Test P-256 with valid coordinates
|
||||
+ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key()
|
||||
+
|
||||
+ # Valid P-256 coordinates (32 bytes each)
|
||||
+ sig_r_32 = b"\x01" * 32
|
||||
+ sig_s_32 = b"\x02" * 32
|
||||
+ blob_p256_valid = self.create_test_signature_blob(sig_r_32, sig_s_32)
|
||||
+
|
||||
+ # Should parse successfully
|
||||
+ der_sig = ecdsa_der_from_tpm(blob_p256_valid, p256_key)
|
||||
+ self.assertIsInstance(der_sig, bytes)
|
||||
+
|
||||
+ # Test P-256 with invalid coordinates (too large)
|
||||
+ sig_r_invalid = b"\x01" * 50 # Too large for P-256
|
||||
+ sig_s_invalid = b"\x02" * 50 # Too large for P-256
|
||||
+ blob_p256_invalid = self.create_test_signature_blob(sig_r_invalid, sig_s_invalid)
|
||||
+
|
||||
+ # Should raise ValueError
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ ecdsa_der_from_tpm(blob_p256_invalid, p256_key)
|
||||
+ self.assertIn("Invalid r coordinate size", str(cm.exception))
|
||||
+
|
||||
+ def test_signature_parsing_edge_cases(self):
|
||||
+ """Test edge cases in signature parsing"""
|
||||
+ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key()
|
||||
+
|
||||
+ # Test with truncated blob (missing s component)
|
||||
+ truncated_blob = struct.pack(">HHH", 0x0018, 0x000B, 32) + b"\x01" * 32
|
||||
+ # Missing s component
|
||||
+
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ ecdsa_der_from_tpm(truncated_blob, p256_key)
|
||||
+ self.assertIn("Unable to parse ECC signature", str(cm.exception))
|
||||
+
|
||||
+ # Test with blob too short for s size header
|
||||
+ short_blob = struct.pack(">HHH", 0x0018, 0x000B, 32) + b"\x01" * 32 + b"\x00" # Only 1 byte for s size
|
||||
+
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ ecdsa_der_from_tpm(short_blob, p256_key)
|
||||
+ self.assertIn("Unable to parse ECC signature", str(cm.exception))
|
||||
+
|
||||
+ def test_der_encoding_correctness(self):
|
||||
+ """Test that DER encoding produces correctly formatted output"""
|
||||
+ p256_key = ec.generate_private_key(ec.SECP256R1()).public_key()
|
||||
+
|
||||
+ # Create test coordinates
|
||||
+ sig_r = b"\x01" * 32
|
||||
+ sig_s = b"\x02" * 32
|
||||
+ blob = self.create_test_signature_blob(sig_r, sig_s)
|
||||
+
|
||||
+ der_sig = ecdsa_der_from_tpm(blob, p256_key)
|
||||
+
|
||||
+ # DER signature should start with SEQUENCE tag (0x30)
|
||||
+ self.assertEqual(der_sig[0], 0x30, "DER signature should start with SEQUENCE tag")
|
||||
+
|
||||
+ # Should be parseable as DER format
|
||||
+ # The structure should be: 0x30 + length + INTEGER(r) + INTEGER(s)
|
||||
+ self.assertTrue(len(der_sig) >= 6, "DER signature should have minimum length")
|
||||
+
|
||||
+ def test_multiple_curve_support(self):
|
||||
+ """Test that signature parsing works for multiple curve types"""
|
||||
+ test_cases = [
|
||||
+ (ec.SECP256R1(), 32),
|
||||
+ (ec.SECP384R1(), 48),
|
||||
+ (ec.SECP521R1(), 66),
|
||||
+ ]
|
||||
+
|
||||
+ for curve, coord_size in test_cases:
|
||||
+ with self.subTest(curve=curve.name):
|
||||
+ private_key = ec.generate_private_key(curve)
|
||||
+ public_key = private_key.public_key()
|
||||
+
|
||||
+ # Create test signature with appropriate coordinate size
|
||||
+ sig_r = b"\x01" * coord_size
|
||||
+ sig_s = b"\x02" * coord_size
|
||||
+ blob = self.create_test_signature_blob(sig_r, sig_s)
|
||||
+
|
||||
+ # Should parse successfully
|
||||
+ der_sig = ecdsa_der_from_tpm(blob, public_key)
|
||||
+ self.assertIsInstance(der_sig, bytes)
|
||||
+ self.assertTrue(len(der_sig) > 0)
|
||||
+ self.assertEqual(der_sig[0], 0x30) # DER SEQUENCE tag
|
||||
+
|
||||
+ def test_der_int_encoding(self):
|
||||
+ """Test DER integer encoding helper function"""
|
||||
+ # Test positive number that doesn't need padding
|
||||
+ test_bytes = b"\x7F" # 127, no padding needed
|
||||
+ der_encoded = der_int(test_bytes)
|
||||
+ expected = b"\x02\x01\x7F" # INTEGER tag + length + value
|
||||
+ self.assertEqual(der_encoded, expected)
|
||||
+
|
||||
+ # Test positive number that needs zero padding (high bit set)
|
||||
+ test_bytes = b"\xFF" # 255, needs zero padding
|
||||
+ der_encoded = der_int(test_bytes)
|
||||
+ expected = b"\x02\x02\x00\xFF" # INTEGER tag + length + zero padding + value
|
||||
+ self.assertEqual(der_encoded, expected)
|
||||
+
|
||||
+ def test_der_len_encoding(self):
|
||||
+ """Test DER length encoding helper function"""
|
||||
+ # Test short form (< 128)
|
||||
+ short_len = der_len(50)
|
||||
+ self.assertEqual(short_len, b"\x32") # 50 in hex
|
||||
+
|
||||
+ # Test long form (>= 128)
|
||||
+ long_len = der_len(300) # 0x012C
|
||||
+ expected = b"\x82\x01\x2C" # Long form: 0x80 | 2 bytes, then 0x012C
|
||||
+ self.assertEqual(long_len, expected)
|
||||
+
|
||||
+ def test_signature_format_validation_comprehensive(self):
|
||||
+ """Comprehensive test of signature format validation"""
|
||||
+ p521_key = ec.generate_private_key(ec.SECP521R1()).public_key()
|
||||
+
|
||||
+ # Test minimum valid coordinate sizes for P-521
|
||||
+ valid_sizes = [65, 66]
|
||||
+ for size in valid_sizes:
|
||||
+ sig_r = b"\x01" * size
|
||||
+ sig_s = b"\x02" * size
|
||||
+ blob = self.create_test_signature_blob(sig_r, sig_s)
|
||||
+
|
||||
+ # Should not raise exception
|
||||
+ der_sig = ecdsa_der_from_tpm(blob, p521_key)
|
||||
+ self.assertIsInstance(der_sig, bytes)
|
||||
+
|
||||
+ # Test invalid coordinate sizes for P-521 (outside the 1-66 range)
|
||||
+ invalid_sizes = [0, 67, 100] # 0 is too small, 67+ is too large
|
||||
+ for size in invalid_sizes:
|
||||
+ with self.subTest(size=size):
|
||||
+ sig_r = b"\x01" * size if size > 0 else b""
|
||||
+ sig_s = b"\x02" * size if size > 0 else b""
|
||||
+ blob = self.create_test_signature_blob(sig_r, sig_s)
|
||||
+
|
||||
+ with self.assertRaises(ValueError) as cm:
|
||||
+ ecdsa_der_from_tpm(blob, p521_key)
|
||||
+ self.assertIn("coordinate size", str(cm.exception))
|
||||
+
|
||||
+
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
--
|
||||
2.47.3
|
||||
|
||||
14
keylime.spec
14
keylime.spec
@ -9,7 +9,7 @@
|
||||
|
||||
Name: keylime
|
||||
Version: 7.12.1
|
||||
Release: 11%{?dist}.2
|
||||
Release: 12%{?dist}
|
||||
Summary: Open source TPM software for Bootstrapping and Maintaining Trust
|
||||
|
||||
URL: https://github.com/keylime/keylime
|
||||
@ -48,6 +48,18 @@ Patch: 0011-fix-malformed-certs-workaround.patch
|
||||
# Backported from https://github.com/keylime/keylime/pull/1795
|
||||
Patch: 0012-keylime-policy-avoid-opening-dev-stdout.patch
|
||||
|
||||
# Backported from:
|
||||
# - https://github.com/keylime/keylime/pull/1746
|
||||
# - https://github.com/keylime/keylime/pull/1803
|
||||
# - https://github.com/keylime/keylime/pull/1808
|
||||
# ECC attestation support.
|
||||
Patch: 0013-algorithms-add-support-for-specific-ECC-curve-algori.patch
|
||||
Patch: 0014-algorithms-add-support-for-specific-RSA-algorithms.patch
|
||||
Patch: 0015-tpm_util-fix-quote-signature-extraction-for-ECDSA.patch
|
||||
Patch: 0016-tpm-fix-ECC-P-521-coordinate-validation.patch
|
||||
Patch: 0017-tpm-fix-ECC-P-521-credential-activation-with-consist.patch
|
||||
Patch: 0018-tpm-fix-ECC-signature-parsing-to-support-variable-le.patch
|
||||
|
||||
# Main program: Apache-2.0
|
||||
# Icons: MIT
|
||||
License: Apache-2.0 AND MIT
|
||||
|
||||
Loading…
Reference in New Issue
Block a user