Convert PKCS7_SIGNER_INFO_set() to X509_ALGOR_set0_by_nid()
authortb <tb@openbsd.org>
Thu, 9 Nov 2023 19:00:53 +0000 (19:00 +0000)
committertb <tb@openbsd.org>
Thu, 9 Nov 2023 19:00:53 +0000 (19:00 +0000)
This is a straightforward conversion because I'm not going to start a
cleanup here. Explain why this is not using X509_ALGOR_set_md(). See
below.

ok jca

Let me include a beautiful note from RFC 5754 in its entirety:

   NOTE: There are two possible encodings for the AlgorithmIdentifier
   parameters field associated with these object identifiers.  The two
   alternatives arise from the loss of the OPTIONAL associated with the
   algorithm identifier parameters when the 1988 syntax for
   AlgorithmIdentifier was translated into the 1997 syntax.  Later, the
   OPTIONAL was recovered via a defect report, but by then many people
   thought that algorithm parameters were mandatory.  Because of this
   history, some implementations encode parameters as a NULL element
   while others omit them entirely.  The correct encoding is to omit the
   parameters field; however, when some uses of these algorithms were
   defined, it was done using the NULL parameters rather than absent
   parameters.  For example, PKCS#1 [RFC3447] requires that the padding
   used for RSA signatures (EMSA-PKCS1-v1_5) MUST use SHA2
   AlgorithmIdentifiers with NULL parameters (to clarify, the
   requirement "MUST generate SHA2 AlgorithmIdentifiers with absent
   parameters" in the previous paragraph does not apply to this
   padding).

lib/libcrypto/pkcs7/pk7_lib.c

index 6eda698..c3501c2 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: pk7_lib.c,v 1.26 2023/02/16 08:38:17 tb Exp $ */
+/* $OpenBSD: pk7_lib.c,v 1.27 2023/11/09 19:00:53 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -370,6 +370,7 @@ int
 PKCS7_SIGNER_INFO_set(PKCS7_SIGNER_INFO *p7i, X509 *x509, EVP_PKEY *pkey,
     const EVP_MD *dgst)
 {
+       int nid;
        int ret;
 
        /* We now need to add another PKCS7_SIGNER_INFO entry */
@@ -390,10 +391,15 @@ PKCS7_SIGNER_INFO_set(PKCS7_SIGNER_INFO *p7i, X509 *x509, EVP_PKEY *pkey,
        CRYPTO_add(&pkey->references, 1, CRYPTO_LOCK_EVP_PKEY);
        p7i->pkey = pkey;
 
-       /* Set the algorithms */
-
-       X509_ALGOR_set0(p7i->digest_alg, OBJ_nid2obj(EVP_MD_type(dgst)),
-           V_ASN1_NULL, NULL);
+       /*
+        * Do not use X509_ALGOR_set_evp_md() to match historical behavior.
+        * A mistranslation of the ASN.1 from 1988 to 1997 syntax lost the
+        * OPTIONAL field, cf. the NOTE above RFC 5254, 2.1.
+        * Using X509_ALGOR_set_evp_md() would change encoding of the SHAs.
+        */
+       nid = EVP_MD_type(dgst);
+       if (!X509_ALGOR_set0_by_nid(p7i->digest_alg, nid, V_ASN1_NULL, NULL))
+               return 0;
 
        if (pkey->ameth && pkey->ameth->pkey_ctrl) {
                ret = pkey->ameth->pkey_ctrl(pkey, ASN1_PKEY_CTRL_PKCS7_SIGN,