Skip to content

Commit

Permalink
Implement CSR generation from first principles to support commissione…
Browse files Browse the repository at this point in the history
…rs (#18631)

* Implement CSR generation from first principles to support commissioners

- When a commissioner is backing their key with OS or hardware support,
  the built-in P256Keypair::NewCertificateSigningRequest will not be
  usable since it relies on internal P256Keypair base class access to
  key state, as opposed to just using Pubkey() and ECDSA_sign_message
  primitives. This is OK on some embedded usecases that make use
  of P256Keypair backend directly, but not for many other usecases.
- On iOS/Darwin and on native Android, backing the P256Keypair *
  by derived classes is bridgeable to platform APIs, but those
  platform APIs do not offer easy/direct CSR generation, and on
  Darwin, there are not ASN.1 APIs anymore.
- If trying to make use of Darwin APIs introduced in #18519, there
  is no easy way to write code interfacing with an external CA to
  provide a CSR for a natively bridged keypair.

This PR adds a first-principle CSR generator, written and audited
by Google personel, using the ASN1Writer API already used in
CHIPCert.h and used by all Commissioner code making use of SDK
today. This is a straightforward implementation that directly
uses a P256Keypair * (or a derived class thereof!) to generate
a CSR against it, without depending on direct key access like
like the native version P256Keypair::NewCerticateSigningRequest
does.

This PR also fixes constness of operations on P256Keypair.

Issue #18444

Testing done:
- Added unit tests for the new primitive
- Validated generated CSR with OpenSSL
- Validated equivalence to generated CSR from P256Keypair, on
  both mbedTLS and OpenSSL
- Not used by CHIP-tool but usable by Darwin and Android framework
  users.

* Update src/crypto/CHIPCryptoPAL.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Fix CI

* Restyled by clang-format

Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
3 people authored and pull[bot] committed Jan 19, 2024
1 parent 8ef1823 commit 1089578
Show file tree
Hide file tree
Showing 12 changed files with 299 additions and 30 deletions.
1 change: 1 addition & 0 deletions src/crypto/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ static_library("crypto") {

public_deps = [
":crypto_buildconfig",
"${chip_root}/src/lib/asn1",
"${chip_root}/src/lib/core",
"${nlassert_root}:nlassert",
]
Expand Down
185 changes: 185 additions & 0 deletions src/crypto/CHIPCryptoPAL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/

#include "CHIPCryptoPAL.h"
#include <lib/asn1/ASN1.h>
#include <lib/asn1/ASN1Macros.h>
#include <lib/core/CHIPEncoding.h>
#include <lib/support/BufferReader.h>
#include <lib/support/BufferWriter.h>
Expand All @@ -34,6 +36,8 @@ using chip::MutableByteSpan;
using chip::Encoding::BufferWriter;
using chip::Encoding::LittleEndian::Reader;

using namespace chip::ASN1;

namespace {

constexpr uint8_t kIntegerTag = 0x02u;
Expand Down Expand Up @@ -869,5 +873,186 @@ CHIP_ERROR ExtractVIDPIDFromAttributeString(DNAttrType attrType, const ByteSpan
return CHIP_NO_ERROR;
}

// Generates the to-be-signed portion of a PKCS#10 CSR (`CertificationRequestInformation`)
// that contains the
static CHIP_ERROR GenerateCertificationRequestInformation(ASN1Writer & writer, const Crypto::P256PublicKey & pubkey)
{
CHIP_ERROR err = CHIP_NO_ERROR;
/**
*
* CertificationRequestInfo ::=
* SEQUENCE {
* version INTEGER { v1(0) } (v1,...),
* subject Name,
* subjectPKInfo SubjectPublicKeyInfo{{ PKInfoAlgorithms }},
* attributes [0] Attributes{{ CRIAttributes }}
* }
*/
ASN1_START_SEQUENCE
{
ASN1_ENCODE_INTEGER(0); // version INTEGER { v1(0) }

// subject Name
ASN1_START_SEQUENCE
{
ASN1_START_SET
{
ASN1_START_SEQUENCE
{
// Any subject, placeholder is good, since this
// is going to usually be ignored
ASN1_ENCODE_OBJECT_ID(kOID_AttributeType_OrganizationalUnitName);
ASN1_ENCODE_STRING(kASN1UniversalTag_UTF8String, "CSA", static_cast<uint16_t>(strlen("CSA")));
}
ASN1_END_SEQUENCE;
}
ASN1_END_SET;
}
ASN1_END_SEQUENCE;

// subjectPKInfo
ASN1_START_SEQUENCE
{
ASN1_START_SEQUENCE
{
ASN1_ENCODE_OBJECT_ID(kOID_PubKeyAlgo_ECPublicKey);
ASN1_ENCODE_OBJECT_ID(kOID_EllipticCurve_prime256v1);
}
ASN1_END_SEQUENCE;
ReturnErrorOnFailure(writer.PutBitString(0, pubkey, static_cast<uint8_t>(pubkey.Length())));
}
ASN1_END_SEQUENCE;

// attributes [0]
ASN1_START_CONSTRUCTED(kASN1TagClass_ContextSpecific, 0)
{
// Using a plain empty attributes request
ASN1_START_SEQUENCE
{
ASN1_ENCODE_OBJECT_ID(kOID_Extension_CSRRequest);
ASN1_START_SET
{
ASN1_START_SEQUENCE {}
ASN1_END_SEQUENCE;
}
ASN1_END_SET;
}
ASN1_END_SEQUENCE;
}
ASN1_END_CONSTRUCTED;
}
ASN1_END_SEQUENCE;
exit:
return err;
}

CHIP_ERROR GenerateCertificateSigningRequest(const P256Keypair * keypair, MutableByteSpan & csr_span)
{
VerifyOrReturnError(keypair != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(csr_span.size() >= kMAX_CSR_Length, CHIP_ERROR_BUFFER_TOO_SMALL);

// First pass: Generate the CertificatioRequestInformation inner
// encoding one time, to sign it, before re-generating it within the
// full ASN1 writer later, since it's easier than trying to
// figure-out the span we need to sign of the overall object.
P256ECDSASignature signature;

{
// The first pass will just generate a signature, so we can use the
// output buffer as scratch to avoid needing more stack space. There
// are no secrets here and the contents is not reused since all we
// need is the signature which is already separately stored.
ASN1Writer toBeSignedWriter;
toBeSignedWriter.Init(csr_span);
CHIP_ERROR err = GenerateCertificationRequestInformation(toBeSignedWriter, keypair->Pubkey());
ReturnErrorOnFailure(err);

size_t encodedLen = (uint16_t) toBeSignedWriter.GetLengthWritten();
// This should not/will not happen
if (encodedLen > csr_span.size())
{
return CHIP_ERROR_INTERNAL;
}

err = keypair->ECDSA_sign_msg(csr_span.data(), encodedLen, signature);
ReturnErrorOnFailure(err);
}

// Second pass: Generate the entire CSR body, restarting a new write
// of the CertificationRequestInformation (cheap) and adding the
// signature.
//
// See RFC2986 for ASN.1 module, repeated here in snippets
CHIP_ERROR err = CHIP_NO_ERROR;

ASN1Writer writer;
writer.Init(csr_span);

ASN1_START_SEQUENCE
{

/* CertificationRequestInfo ::=
* SEQUENCE {
* version INTEGER { v1(0) } (v1,...),
* subject Name,
* subjectPKInfo SubjectPublicKeyInfo{{ PKInfoAlgorithms }},
* attributes [0] Attributes{{ CRIAttributes }}
* }
*/
GenerateCertificationRequestInformation(writer, keypair->Pubkey());

// algorithm AlgorithmIdentifier
ASN1_START_SEQUENCE
{
// See RFC5480 sec 2.1
ASN1_ENCODE_OBJECT_ID(kOID_SigAlgo_ECDSAWithSHA256);
}
ASN1_END_SEQUENCE;

// signature BIT STRING --> ECDSA-with-SHA256 signature with P256 key with R,S integers format
// (see RFC3279 sec 2.2.3 ECDSA Signature Algorithm)
ASN1_START_BIT_STRING_ENCAPSULATED
{
// Convert raw signature to embedded signature
FixedByteSpan<Crypto::kP256_ECDSA_Signature_Length_Raw> rawSig(signature.Bytes());

uint8_t derInt[kP256_FE_Length + kEmitDerIntegerWithoutTagOverhead];

// Ecdsa-Sig-Value ::= SEQUENCE
ASN1_START_SEQUENCE
{
using P256IntegerSpan = FixedByteSpan<Crypto::kP256_FE_Length>;
// r INTEGER
{
MutableByteSpan derIntSpan(derInt, sizeof(derInt));
ReturnErrorOnFailure(ConvertIntegerRawToDerWithoutTag(P256IntegerSpan(rawSig.data()), derIntSpan));
ReturnErrorOnFailure(writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false,
derIntSpan.data(), static_cast<uint16_t>(derIntSpan.size())));
}

// s INTEGER
{
MutableByteSpan derIntSpan(derInt, sizeof(derInt));
ReturnErrorOnFailure(
ConvertIntegerRawToDerWithoutTag(P256IntegerSpan(rawSig.data() + kP256_FE_Length), derIntSpan));
ReturnErrorOnFailure(writer.PutValue(kASN1TagClass_Universal, kASN1UniversalTag_Integer, false,
derIntSpan.data(), static_cast<uint16_t>(derIntSpan.size())));
}
}
ASN1_END_SEQUENCE;
}
ASN1_END_ENCAPSULATED;
}
ASN1_END_SEQUENCE;

exit:
// Update size of output buffer on success
if (err == CHIP_NO_ERROR)
{
csr_span.reduce_size(writer.GetLengthWritten());
}
return err;
}

} // namespace Crypto
} // namespace chip
35 changes: 28 additions & 7 deletions src/crypto/CHIPCryptoPAL.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ class ECPKeypair
*CSR.
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
virtual CHIP_ERROR NewCertificateSigningRequest(uint8_t * csr, size_t & csr_length) = 0;
virtual CHIP_ERROR NewCertificateSigningRequest(uint8_t * csr, size_t & csr_length) const = 0;

/**
* @brief A function to sign a msg using ECDSA
Expand All @@ -332,7 +332,7 @@ class ECPKeypair
* in raw <r,s> point form (see SEC1).
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
virtual CHIP_ERROR ECDSA_sign_msg(const uint8_t * msg, size_t msg_length, Sig & out_signature) = 0;
virtual CHIP_ERROR ECDSA_sign_msg(const uint8_t * msg, size_t msg_length, Sig & out_signature) const = 0;

/**
* @brief A function to sign a hash using ECDSA
Expand All @@ -342,7 +342,7 @@ class ECPKeypair
* in raw <r,s> point form (see SEC1).
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
virtual CHIP_ERROR ECDSA_sign_hash(const uint8_t * hash, size_t hash_length, Sig & out_signature) = 0;
virtual CHIP_ERROR ECDSA_sign_hash(const uint8_t * hash, size_t hash_length, Sig & out_signature) const = 0;

/** @brief A function to derive a shared secret using ECDH
* @param remote_public_key Public key of remote peer with which we are trying to establish secure channel. remote_public_key is
Expand Down Expand Up @@ -416,7 +416,7 @@ class P256Keypair : public P256KeypairBase
*CSR.
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR NewCertificateSigningRequest(uint8_t * csr, size_t & csr_length) override;
CHIP_ERROR NewCertificateSigningRequest(uint8_t * csr, size_t & csr_length) const override;

/**
* @brief A function to sign a msg using ECDSA
Expand All @@ -426,7 +426,7 @@ class P256Keypair : public P256KeypairBase
* in raw <r,s> point form (see SEC1).
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR ECDSA_sign_msg(const uint8_t * msg, size_t msg_length, P256ECDSASignature & out_signature) override;
CHIP_ERROR ECDSA_sign_msg(const uint8_t * msg, size_t msg_length, P256ECDSASignature & out_signature) const override;

/**
* @brief A function to sign a hash using ECDSA
Expand All @@ -436,7 +436,7 @@ class P256Keypair : public P256KeypairBase
* in raw <r,s> point form (see SEC1).
* @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR ECDSA_sign_hash(const uint8_t * hash, size_t hash_length, P256ECDSASignature & out_signature) override;
CHIP_ERROR ECDSA_sign_hash(const uint8_t * hash, size_t hash_length, P256ECDSASignature & out_signature) const override;

/**
* @brief A function to derive a shared secret using ECDH
Expand All @@ -462,7 +462,7 @@ class P256Keypair : public P256KeypairBase

private:
P256PublicKey mPublicKey;
P256KeypairContext mKeypair;
mutable P256KeypairContext mKeypair;
bool mInitialized = false;
};

Expand Down Expand Up @@ -616,6 +616,27 @@ CHIP_ERROR AES_CCM_decrypt(const uint8_t * ciphertext, size_t ciphertext_length,
const uint8_t * tag, size_t tag_length, const uint8_t * key, size_t key_length, const uint8_t * nonce,
size_t nonce_length, uint8_t * plaintext);

/**
* @brief Generate a PKCS#10 CSR, usable for Matter, from a P256Keypair.
*
* This uses first principles ASN.1 encoding to avoid relying on the CHIPCryptoPAL backend
* itself, other than to provide an implementation of a P256Keypair * that supports
* at least `::Pubkey()` and `::ECDSA_sign_msg`. This allows using it with
* OS/Platform-bridged private key handling, without requiring a specific
* implementation of other bits like ASN.1.
*
* The CSR will have subject OU set to `CSA`. This is needed since omiting
* subject altogether often trips CSR parsing code. The profile at the CA can
* be configured to ignore CSR requested subject.
*
* @param keypair The key pair for which a CSR should be generated. Must not be null.
* @param csr_span Span to hold the resulting CSR. Must be at least kMAX_CSR_Length. Otherwise returns CHIP_ERROR_BUFFER_TOO_SMALL.
* It will get resized to actual size needed on success.
* @return Returns a CHIP_ERROR from P256Keypair or ASN.1 backend on error, CHIP_NO_ERROR otherwise
**/
CHIP_ERROR GenerateCertificateSigningRequest(const P256Keypair * keypair, MutableByteSpan & csr_span);

/**
* @brief Verify the Certificate Signing Request (CSR). If successfully verified, it outputs the public key from the CSR.
* @param csr CSR in DER format
Expand Down
6 changes: 3 additions & 3 deletions src/crypto/CHIPCryptoPALOpenSSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ static inline const EC_KEY * to_const_EC_KEY(const P256KeypairContext * context)
return *SafePointerCast<const EC_KEY * const *>(context);
}

CHIP_ERROR P256Keypair::ECDSA_sign_msg(const uint8_t * msg, const size_t msg_length, P256ECDSASignature & out_signature)
CHIP_ERROR P256Keypair::ECDSA_sign_msg(const uint8_t * msg, const size_t msg_length, P256ECDSASignature & out_signature) const
{
VerifyOrReturnError((msg != nullptr) && (msg_length > 0), CHIP_ERROR_INVALID_ARGUMENT);

Expand All @@ -620,7 +620,7 @@ CHIP_ERROR P256Keypair::ECDSA_sign_msg(const uint8_t * msg, const size_t msg_len
return ECDSA_sign_hash(&digest[0], sizeof(digest), out_signature);
}

CHIP_ERROR P256Keypair::ECDSA_sign_hash(const uint8_t * hash, const size_t hash_length, P256ECDSASignature & out_signature)
CHIP_ERROR P256Keypair::ECDSA_sign_hash(const uint8_t * hash, const size_t hash_length, P256ECDSASignature & out_signature) const
{
ERR_clear_error();

Expand Down Expand Up @@ -1110,7 +1110,7 @@ P256Keypair::~P256Keypair()
Clear();
}

CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & csr_length)
CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & csr_length) const
{
ERR_clear_error();
CHIP_ERROR error = CHIP_NO_ERROR;
Expand Down
6 changes: 3 additions & 3 deletions src/crypto/CHIPCryptoPALmbedTLS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ static inline const mbedtls_ecp_keypair * to_const_keypair(const P256KeypairCont
return SafePointerCast<const mbedtls_ecp_keypair *>(context);
}

CHIP_ERROR P256Keypair::ECDSA_sign_msg(const uint8_t * msg, const size_t msg_length, P256ECDSASignature & out_signature)
CHIP_ERROR P256Keypair::ECDSA_sign_msg(const uint8_t * msg, const size_t msg_length, P256ECDSASignature & out_signature) const
{
#if defined(MBEDTLS_ECDSA_C)
VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE);
Expand All @@ -519,7 +519,7 @@ CHIP_ERROR P256Keypair::ECDSA_sign_msg(const uint8_t * msg, const size_t msg_len
#endif
}

CHIP_ERROR P256Keypair::ECDSA_sign_hash(const uint8_t * hash, const size_t hash_length, P256ECDSASignature & out_signature)
CHIP_ERROR P256Keypair::ECDSA_sign_hash(const uint8_t * hash, const size_t hash_length, P256ECDSASignature & out_signature) const
{
#if defined(MBEDTLS_ECDSA_C)
VerifyOrReturnError(mInitialized, CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -812,7 +812,7 @@ P256Keypair::~P256Keypair()
Clear();
}

CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & csr_length)
CHIP_ERROR P256Keypair::NewCertificateSigningRequest(uint8_t * out_csr, size_t & csr_length) const
{
CHIP_ERROR error = CHIP_NO_ERROR;

Expand Down
6 changes: 3 additions & 3 deletions src/crypto/hsm/nxp/CHIPCryptoPALHsm_SE05X_P256.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ CHIP_ERROR P256KeypairHSM::Initialize()
return CHIP_NO_ERROR;
}

CHIP_ERROR P256KeypairHSM::ECDSA_sign_msg(const uint8_t * msg, size_t msg_length, P256ECDSASignature & out_signature)
CHIP_ERROR P256KeypairHSM::ECDSA_sign_msg(const uint8_t * msg, size_t msg_length, P256ECDSASignature & out_signature) const
{
CHIP_ERROR error = CHIP_ERROR_INTERNAL;
sss_digest_t digest_ctx = { 0 };
Expand Down Expand Up @@ -205,7 +205,7 @@ CHIP_ERROR P256KeypairHSM::ECDSA_sign_msg(const uint8_t * msg, size_t msg_length
return error;
}

CHIP_ERROR P256KeypairHSM::ECDSA_sign_hash(const uint8_t * hash, size_t hash_length, P256ECDSASignature & out_signature)
CHIP_ERROR P256KeypairHSM::ECDSA_sign_hash(const uint8_t * hash, size_t hash_length, P256ECDSASignature & out_signature) const
{
CHIP_ERROR error = CHIP_ERROR_INTERNAL;
sss_asymmetric_t asymm_ctx = { 0 };
Expand Down Expand Up @@ -574,7 +574,7 @@ static void add_tlv(uint8_t * buf, size_t buf_index, uint8_t tag, size_t len, ui
*
*/

CHIP_ERROR P256KeypairHSM::NewCertificateSigningRequest(uint8_t * csr, size_t & csr_length)
CHIP_ERROR P256KeypairHSM::NewCertificateSigningRequest(uint8_t * csr, size_t & csr_length) const
{
CHIP_ERROR error = CHIP_ERROR_INTERNAL;
sss_status_t status = kStatus_SSS_Success;
Expand Down

0 comments on commit 1089578

Please sign in to comment.