On Tue, Jan 18, 2022 at 04:54:33PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
>
> An X.509 certificate has two, potentially different public key
> algorithms: the one used by the certificate's key, and the one that was
> used to sign the certificate. Some of the naming made it unclear which
> algorithm was meant. Rename things appropriately:
>
> - x509_note_pkey_algo() => x509_note_sig_algo()
> - algo_oid => sig_algo
>
> Acked-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> ---
> crypto/asymmetric_keys/x509.asn1 | 2 +-
> crypto/asymmetric_keys/x509_cert_parser.c | 32 +++++++++++++----------
> 2 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/x509.asn1 b/crypto/asymmetric_keys/x509.asn1
> index 5c9f4e4a52310..92d59c32f96a8 100644
> --- a/crypto/asymmetric_keys/x509.asn1
> +++ b/crypto/asymmetric_keys/x509.asn1
> @@ -7,7 +7,7 @@ Certificate ::= SEQUENCE {
> TBSCertificate ::= SEQUENCE {
> version [ 0 ] Version DEFAULT,
> serialNumber CertificateSerialNumber ({ x509_note_serial }),
> - signature AlgorithmIdentifier ({ x509_note_pkey_algo }),
> + signature AlgorithmIdentifier ({ x509_note_sig_algo }),
> issuer Name ({ x509_note_issuer }),
> validity Validity,
> subject Name ({ x509_note_subject }),
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index 083405eb80c32..aec2396a7f7e1 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -24,9 +24,9 @@ struct x509_parse_context {
> size_t key_size; /* Size of key data */
> const void *params; /* Key parameters */
> size_t params_size; /* Size of key parameters */
> - enum OID key_algo; /* Public key algorithm */
> + enum OID key_algo; /* Algorithm used by the cert's key */
> enum OID last_oid; /* Last OID encountered */
> - enum OID algo_oid; /* Algorithm OID */
> + enum OID sig_algo; /* Algorithm used to sign the cert */
> unsigned char nr_mpi; /* Number of MPIs stored */
> u8 o_size; /* Size of organizationName (O) */
> u8 cn_size; /* Size of commonName (CN) */
> @@ -187,11 +187,10 @@ int x509_note_tbs_certificate(void *context, size_t hdrlen,
> }
>
> /*
> - * Record the public key algorithm
> + * Record the algorithm that was used to sign this certificate.
> */
> -int x509_note_pkey_algo(void *context, size_t hdrlen,
> - unsigned char tag,
> - const void *value, size_t vlen)
> +int x509_note_sig_algo(void *context, size_t hdrlen, unsigned char tag,
> + const void *value, size_t vlen)
> {
> struct x509_parse_context *ctx = context;
>
> @@ -263,22 +262,22 @@ int x509_note_pkey_algo(void *context, size_t hdrlen,
> rsa_pkcs1:
> ctx->cert->sig->pkey_algo = "rsa";
> ctx->cert->sig->encoding = "pkcs1";
> - ctx->algo_oid = ctx->last_oid;
> + ctx->sig_algo = ctx->last_oid;
> return 0;
> ecrdsa:
> ctx->cert->sig->pkey_algo = "ecrdsa";
> ctx->cert->sig->encoding = "raw";
> - ctx->algo_oid = ctx->last_oid;
> + ctx->sig_algo = ctx->last_oid;
> return 0;
> sm2:
> ctx->cert->sig->pkey_algo = "sm2";
> ctx->cert->sig->encoding = "raw";
> - ctx->algo_oid = ctx->last_oid;
> + ctx->sig_algo = ctx->last_oid;
> return 0;
> ecdsa:
> ctx->cert->sig->pkey_algo = "ecdsa";
> ctx->cert->sig->encoding = "x962";
> - ctx->algo_oid = ctx->last_oid;
> + ctx->sig_algo = ctx->last_oid;
> return 0;
> }
>
> @@ -291,11 +290,16 @@ int x509_note_signature(void *context, size_t hdrlen,
> {
> struct x509_parse_context *ctx = context;
>
> - pr_debug("Signature type: %u size %zu\n", ctx->last_oid, vlen);
> + pr_debug("Signature: alg=%u, size=%zu\n", ctx->last_oid, vlen);
>
> - if (ctx->last_oid != ctx->algo_oid) {
> - pr_warn("Got cert with pkey (%u) and sig (%u) algorithm OIDs\n",
> - ctx->algo_oid, ctx->last_oid);
> + /*
> + * In X.509 certificates, the signature's algorithm is stored in two
> + * places: inside the TBSCertificate (the data that is signed), and
> + * alongside the signature. These *must* match.
> + */
> + if (ctx->last_oid != ctx->sig_algo) {
> + pr_warn("signatureAlgorithm (%u) differs from tbsCertificate.signature (%u)\n",
> + ctx->last_oid, ctx->sig_algo);
> return -EINVAL;
> }
>
> --
> 2.34.1
>
Reviewed-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
/Jarkko
