Add Compressed Fabric ID crypto primitive (#8776)

* Add Compressed Fabric ID crypto primitive - Add compressed fabric ID computation crypto primitive, used by operational discovery in spec. The value output by this function should be used rather than raw fabric ID, as it is unambiguous since it takes root public key scoping into account. - Primitive is not integrated in operational discovery yet, but ready to do so. - Also fixed `ClearSecretData` primitive to use the correct impl in openssl/mbedtls - Added `Bytes()`/`ConstBytes()` accessors to `ECPkey` and `P256PublicKey` to avoid grimy implicit uint8_t conversion usage in new code. - Added necessary unit tests including all error paths. Testing done: `ninja -C out/host check` with both OpenSSL and mbedTLS, after adding necessary unit tests. Employed unit tests to discover implementation issue (initially forgot to drop 0x04 start byte specifying raw uncompressed EC public key) Issue #8227 Issue #4984 Issue #3121 * Restyled by clang-format * Add Compressed Fabric ID crypto primitive - Add compressed fabric ID computation crypto primitive, used by operational discovery in spec. The value output by this function should be used rather than raw fabric ID, as it is unambiguous since it takes root public key scoping into account. - Primitive is not integrated in operational discovery yet, but ready to do so. - Also fixed `ClearSecretData` primitive to use the correct impl in openssl/mbedtls - Added `Bytes()`/`ConstBytes()` accessors to `ECPkey` and `P256PublicKey` to avoid grimy implicit uint8_t conversion usage in new code. - Added necessary unit tests including all error paths. Testing done: `ninja -C out/host check` with both OpenSSL and mbedTLS, after adding necessary unit tests. Employed unit tests to discover implementation issue (initially forgot to drop 0x04 start byte specifying raw uncompressed EC public key) Issue #8227 Issue #4984 Issue #3121 * Restyled by clang-format * Addressed review comments * Restyled by clang-format * Address review comments * Restyled by clang-format Co-authored-by: Restyled.io <[email protected]>
project-chip · Aug 11, 2021 · a398eec · a398eec
1 parent b488700
commit a398eec
Show file tree

Hide file tree

Showing 5 changed files with 164 additions and 25 deletions.
diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp
@@ -608,5 +608,45 @@ CHIP_ERROR EcdsaAsn1SignatureToRaw(size_t fe_length_bytes, const ByteSpan & asn1
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR GenerateCompressedFabricId(const Crypto::P256PublicKey & root_public_key, uint64_t fabric_id,
+                                      MutableByteSpan & out_compressed_fabric_id)
+{
+    VerifyOrReturnError(root_public_key.IsUncompressed(), CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrReturnError(out_compressed_fabric_id.size() >= kCompressedFabricIdentifierSize, CHIP_ERROR_BUFFER_TOO_SMALL);
+
+    // Ensure proper endianness for Fabric ID (i.e. big-endian as it appears in certificates)
+    uint8_t fabric_id_as_big_endian_salt[kCompressedFabricIdentifierSize];
+    chip::Encoding::BigEndian::Put64(&fabric_id_as_big_endian_salt[0], fabric_id);
+
+    // Compute Compressed fabric reference per spec pseudocode
+    //   CompressedFabricIdentifier =
+    //     CHIP_Crypto_KDF(
+    //       inputKey := TargetOperationalRootPublicKey,
+    //       salt:= TargetOperationalFabricID,
+    //       info := CompressedFabricInfo,
+    //       len := 64)
+    //
+    // NOTE: len=64 bits is implied by output buffer size when calling HKDF_sha::HKDF_SHA256.
+
+    constexpr uint8_t kCompressedFabricInfo[16] = /* "CompressedFabric" */
+        { 0x43, 0x6f, 0x6d, 0x70, 0x72, 0x65, 0x73, 0x73, 0x65, 0x64, 0x46, 0x61, 0x62, 0x72, 0x69, 0x63 };
+    HKDF_sha hkdf;
+
+    // Must drop uncompressed point form format specifier (first byte), per spec method
+    ByteSpan input_key_span(root_public_key.ConstBytes() + 1, root_public_key.Length() - 1);
+
+    CHIP_ERROR status = hkdf.HKDF_SHA256(
+        input_key_span.data(), input_key_span.size(), &fabric_id_as_big_endian_salt[0], sizeof(fabric_id_as_big_endian_salt),
+        &kCompressedFabricInfo[0], sizeof(kCompressedFabricInfo), out_compressed_fabric_id.data(), kCompressedFabricIdentifierSize);
+
+    // Resize output to final bounds on success
+    if (status == CHIP_NO_ERROR)
+    {
+        out_compressed_fabric_id = out_compressed_fabric_id.SubSpan(0, kCompressedFabricIdentifierSize);
+    }
+
+    return status;
+}
+
 } // namespace Crypto
 } // namespace chip
diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h
@@ -108,6 +108,8 @@ static_assert(kMax_ECDH_Secret_Length >= kP256_FE_Length, "ECDH shared secret is
 static_assert(kMax_ECDSA_Signature_Length >= kP256_ECDSA_Signature_Length_Raw,
               "ECDSA signature buffer length is too short for crypto suite");
 
+constexpr size_t kCompressedFabricIdentifierSize = 8;
+
 /**
  * Spake2+ parameters for P256
  * Defined in https://www.ietf.org/id/draft-bar-cfrg-spake2plus-01.html#name-ciphersuites
@@ -157,19 +159,17 @@ class ECPKey
 {
 public:
     virtual ~ECPKey() {}
-    virtual SupportedECPKeyTypes Type() const = 0;
-    virtual size_t Length() const             = 0;
-    virtual operator const uint8_t *() const  = 0;
-    virtual operator uint8_t *()              = 0;
-
-    virtual CHIP_ERROR ECDSA_validate_msg_signature(const uint8_t * msg, const size_t msg_length, const Sig & signature) const
-    {
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
-    virtual CHIP_ERROR ECDSA_validate_hash_signature(const uint8_t * hash, const size_t hash_length, const Sig & signature) const
-    {
-        return CHIP_ERROR_NOT_IMPLEMENTED;
-    }
+    virtual SupportedECPKeyTypes Type() const  = 0;
+    virtual size_t Length() const              = 0;
+    virtual bool IsUncompressed() const        = 0;
+    virtual operator const uint8_t *() const   = 0;
+    virtual operator uint8_t *()               = 0;
+    virtual const uint8_t * ConstBytes() const = 0;
+    virtual uint8_t * Bytes()                  = 0;
+
+    virtual CHIP_ERROR ECDSA_validate_msg_signature(const uint8_t * msg, const size_t msg_length, const Sig & signature) const = 0;
+    virtual CHIP_ERROR ECDSA_validate_hash_signature(const uint8_t * hash, const size_t hash_length,
+                                                     const Sig & signature) const                                              = 0;
 };
 
 template <size_t Cap>
@@ -220,10 +220,28 @@ typedef CapacityBoundBuffer<kMax_ECDH_Secret_Length> P256ECDHDerivedSecret;
 class P256PublicKey : public ECPKey<P256ECDSASignature>
 {
 public:
+    P256PublicKey() {}
+
+    template <size_t N>
+    constexpr P256PublicKey(const uint8_t (&raw_value)[N])
+    {
+        static_assert(N == kP256_PublicKey_Length, "Can only array-initialize from proper bounds");
+        memcpy(&bytes[0], &raw_value[0], N);
+    }
+
     SupportedECPKeyTypes Type() const override { return SupportedECPKeyTypes::ECP256R1; }
     size_t Length() const override { return kP256_PublicKey_Length; }
     operator uint8_t *() override { return bytes; }
     operator const uint8_t *() const override { return bytes; }
+    const uint8_t * ConstBytes() const override { return &bytes[0]; }
+    uint8_t * Bytes() override { return &bytes[0]; }
+    bool IsUncompressed() const override
+    {
+        constexpr uint8_t kUncompressedPointMarker = 0x04;
+        // SEC1 definition of an uncompressed point is (0x04 || X || Y) where X and Y are
+        // raw zero-padded big-endian large integers of the group size.
+        return (Length() == ((kP256_FE_Length * 2) + 1)) && (ConstBytes()[0] == kUncompressedPointMarker);
+    }
 
     CHIP_ERROR ECDSA_validate_msg_signature(const uint8_t * msg, size_t msg_length,
                                             const P256ECDSASignature & signature) const override;
@@ -1110,11 +1128,29 @@ class Spake2p_P256_SHA256_HKDF_HMAC : public Spake2p
     Spake2pOpaqueContext mSpake2pContext;
 };
 
-/** @brief Clears the first `len` bytes of memory area `buf`.
- * @param buf Pointer to a memory buffer holding secret data that should be cleared.
+/**
+ * @brief Compute the compressed fabric identifier used for operational discovery service
+ *        records from a Node's root public key and Fabric ID. On success, out_compressed_fabric_id
+ *        will have a size of exactly kCompressedFabricIdentifierSize.
+ *
+ * Errors are:
+ *   - CHIP_ERROR_INVALID_ARGUMENT if root_public_key is invalid
+ *   - CHIP_ERROR_BUFFER_TOO_SMALL if out_compressed_fabric_id is too small for serialization
+ *   - CHIP_ERROR_INTERNAL on any unexpected crypto or data conversion errors.
+ *
+ * @param[in] root_public_key The root public key associated with the node's fabric
+ * @param[in] fabric_id The fabric ID associated with the node's fabric
+ * @param[out] out_compressed_fabric_id Span where output will be written. Its size must be >= kCompressedFabricIdentifierSize.
+ * @returns a CHIP_ERROR (see above) on failure or CHIP_NO_ERROR otherwise.
+ */
+CHIP_ERROR GenerateCompressedFabricId(const Crypto::P256PublicKey & root_public_key, uint64_t fabric_id,
+                                      MutableByteSpan & out_compressed_fabric_id);
+
+/** @brief Safely clears the first `len` bytes of memory area `buf`.
+ * @param buf Pointer to a memory buffer holding secret data that must be cleared.
  * @param len Specifies secret data size in bytes.
  **/
-void ClearSecretData(uint8_t * buf, uint32_t len);
+void ClearSecretData(uint8_t * buf, size_t len);
 
 typedef CapacityBoundBuffer<kMax_x509_Certificate_Length> X509DerCertificate;
 

diff --git a/src/crypto/CHIPCryptoPALOpenSSL.cpp b/src/crypto/CHIPCryptoPALOpenSSL.cpp
@@ -837,9 +837,9 @@ CHIP_ERROR P256Keypair::ECDH_derive_secret(const P256PublicKey & remote_public_k
     return error;
 }
 
-void ClearSecretData(uint8_t * buf, uint32_t len)
+void ClearSecretData(uint8_t * buf, size_t len)
 {
-    memset(buf, 0, len);
+    OPENSSL_cleanse(buf, len);
 }
 
 static CHIP_ERROR P256PublicKeyFromECKey(EC_KEY * ec_key, P256PublicKey & pubkey)

diff --git a/src/crypto/CHIPCryptoPALmbedTLS.cpp b/src/crypto/CHIPCryptoPALmbedTLS.cpp
@@ -630,9 +630,9 @@ CHIP_ERROR P256Keypair::ECDH_derive_secret(const P256PublicKey & remote_public_k
 #endif
 }
 
-void ClearSecretData(uint8_t * buf, uint32_t len)
+void ClearSecretData(uint8_t * buf, size_t len)
 {
-    memset(buf, 0, len);
+    mbedtls_platform_zeroize(buf, len);
 }
 
 CHIP_ERROR P256Keypair::Initialize()

diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp
@@ -1246,21 +1246,21 @@ static void TestP256_Keygen(nlTestSuite * inSuite, void * inContext)
 
 static void TestCSR_Gen(nlTestSuite * inSuite, void * inContext)
 {
-    static uint8_t csr[kMAX_CSR_Length];
+    uint8_t csr[kMAX_CSR_Length];
     size_t length = sizeof(csr);
 
-    static Test_P256Keypair keypair;
+    Test_P256Keypair keypair;
     NL_TEST_ASSERT(inSuite, keypair.Initialize() == CHIP_NO_ERROR);
     NL_TEST_ASSERT(inSuite, keypair.NewCertificateSigningRequest(csr, length) == CHIP_NO_ERROR);
     NL_TEST_ASSERT(inSuite, length > 0);
 
-    static P256PublicKey pubkey;
+    P256PublicKey pubkey;
     CHIP_ERROR err = VerifyCertificateSigningRequest(csr, length, pubkey);
     if (err != CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE)
     {
         NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
         NL_TEST_ASSERT(inSuite, pubkey.Length() == kP256_PublicKey_Length);
-        NL_TEST_ASSERT(inSuite, memcmp(pubkey, keypair.Pubkey(), pubkey.Length()) == 0);
+        NL_TEST_ASSERT(inSuite, memcmp(pubkey.ConstBytes(), keypair.Pubkey().ConstBytes(), pubkey.Length()) == 0);
 
         // Let's corrupt the CSR buffer and make sure it fails to verify
         csr[length - 2] = (uint8_t)(csr[length - 2] + 1);
@@ -1687,6 +1687,68 @@ static void TestSPAKE2P_RFC(nlTestSuite * inSuite, void * inContext)
     NL_TEST_ASSERT(inSuite, numOfTestsRan == numOfTestVectors);
 }
 
+static void TestCompressedFabricIdentifier(nlTestSuite * inSuite, void * inContext)
+{
+    // Data from spec test vector (see Operational Discovery section)
+    const uint8_t kRootPublicKey[65] = {
+        0x04, 0x4a, 0x9f, 0x42, 0xb1, 0xca, 0x48, 0x40, 0xd3, 0x72, 0x92, 0xbb, 0xc7, 0xf6, 0xa7, 0xe1, 0x1e,
+        0x22, 0x20, 0x0c, 0x97, 0x6f, 0xc9, 0x00, 0xdb, 0xc9, 0x8a, 0x7a, 0x38, 0x3a, 0x64, 0x1c, 0xb8, 0x25,
+        0x4a, 0x2e, 0x56, 0xd4, 0xe2, 0x95, 0xa8, 0x47, 0x94, 0x3b, 0x4e, 0x38, 0x97, 0xc4, 0xa7, 0x73, 0xe9,
+        0x30, 0x27, 0x7b, 0x4d, 0x9f, 0xbe, 0xde, 0x8a, 0x05, 0x26, 0x86, 0xbf, 0xac, 0xfa,
+    };
+    P256PublicKey root_public_key(kRootPublicKey);
+
+    constexpr uint64_t kFabricId = 0x2906C908D115D362;
+
+    const uint8_t kExpectedCompressedFabricIdentifier[8] = {
+        0x87, 0xe1, 0xb0, 0x04, 0xe2, 0x35, 0xa1, 0x30,
+    };
+    static_assert(sizeof(kExpectedCompressedFabricIdentifier) == kCompressedFabricIdentifierSize,
+                  "Expected compressed fabric identifier must the correct size");
+
+    uint8_t compressed_fabric_id[kCompressedFabricIdentifierSize];
+    MutableByteSpan compressed_fabric_id_span(compressed_fabric_id);
+    ClearSecretData(compressed_fabric_id, sizeof(compressed_fabric_id));
+
+    CHIP_ERROR error = GenerateCompressedFabricId(root_public_key, kFabricId, compressed_fabric_id_span);
+    NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, compressed_fabric_id_span.size() == kCompressedFabricIdentifierSize);
+    NL_TEST_ASSERT(inSuite,
+                   0 ==
+                       memcmp(compressed_fabric_id_span.data(), kExpectedCompressedFabricIdentifier,
+                              sizeof(kExpectedCompressedFabricIdentifier)));
+
+    // Test bigger input buffer than needed
+    uint8_t compressed_fabric_id_large[3 * kCompressedFabricIdentifierSize];
+    MutableByteSpan compressed_fabric_id_large_span(compressed_fabric_id_large);
+    ClearSecretData(compressed_fabric_id_large, sizeof(compressed_fabric_id_large));
+
+    error = GenerateCompressedFabricId(root_public_key, kFabricId, compressed_fabric_id_large_span);
+    NL_TEST_ASSERT(inSuite, error == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite, compressed_fabric_id_large_span.size() == kCompressedFabricIdentifierSize);
+    NL_TEST_ASSERT(inSuite,
+                   0 ==
+                       memcmp(compressed_fabric_id_large_span.data(), kExpectedCompressedFabricIdentifier,
+                              sizeof(kExpectedCompressedFabricIdentifier)));
+
+    // Test smaller buffer than needed
+    MutableByteSpan compressed_fabric_id_small_span(compressed_fabric_id, kCompressedFabricIdentifierSize - 1);
+    error = GenerateCompressedFabricId(root_public_key, kFabricId, compressed_fabric_id_small_span);
+    NL_TEST_ASSERT(inSuite, error == CHIP_ERROR_BUFFER_TOO_SMALL);
+
+    // Test invalid public key
+    const uint8_t kInvalidRootPublicKey[65] = {
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    };
+    P256PublicKey invalid_root_public_key(kInvalidRootPublicKey);
+
+    error = GenerateCompressedFabricId(invalid_root_public_key, kFabricId, compressed_fabric_id_span);
+    NL_TEST_ASSERT(inSuite, error == CHIP_ERROR_INVALID_ARGUMENT);
+}
+
 #if CHIP_CRYPTO_OPENSSL
 static void TestX509_PKCS7Extraction(nlTestSuite * inSuite, void * inContext)
 {
@@ -1730,7 +1792,7 @@ static void TestPubkey_x509Extraction(nlTestSuite * inSuite, void * inContext)
 
         err = ExtractPubkeyFromX509Cert(cert, publicKey);
         NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
-        NL_TEST_ASSERT(inSuite, memcmp(publicKey, certPubkey, certPubkeyLen) == 0);
+        NL_TEST_ASSERT(inSuite, memcmp(publicKey.ConstBytes(), certPubkey, certPubkeyLen) == 0);
     }
 }
 #endif // CHIP_CRYPTO_OPENSSL
@@ -1793,6 +1855,7 @@ static const nlTest sTests[] = {
     NL_TEST_DEF("Test Spake2p_spake2p PointLoad/PointWrite", TestSPAKE2P_spake2p_PointLoadWrite),
     NL_TEST_DEF("Test Spake2p_spake2p PointIsValid", TestSPAKE2P_spake2p_PointIsValid),
     NL_TEST_DEF("Test Spake2+ against RFC test vectors", TestSPAKE2P_RFC),
+    NL_TEST_DEF("Test compressed fabric identifier", TestCompressedFabricIdentifier),
 #if CHIP_CRYPTO_OPENSSL
     NL_TEST_DEF("Test x509 Certificate Extraction from PKCS7", TestX509_PKCS7Extraction),
     NL_TEST_DEF("Test Pubkey Extraction from x509 Certificate", TestPubkey_x509Extraction),