commit ec45e104a608ba556be73a0776cfb495c6c8ae44
author David Benjamin <davidben@google.com> Sun Mar 17 16:15:48 2024 +1000
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Sun Mar 17 07:42:18 2024 +0000
tree 5615330e4d930e276763e849fb25a81b955668be
parent 440c51317bcbc15aec372bc78cf6fbf59d7eb435

X509_ALGOR_set_md is a mess, document it

tb noticed that our X509_ALGOR_set_md differs from OpenSSL because we
never set EVP_MD_FLAG_DIGALGID_ABSENT. That is, we include an explicit
NULL parameter, while OpenSSL omits it.

RFC 4055, section 2.1 says:

   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 RSASSA-PSS and RSAES-OAEP were
   defined, it was done using the NULL parameters rather than absent
   parameters.

   ...

   To be clear, the following algorithm identifiers are used when a NULL
   parameter MUST be present:

   ...

My read of this text is:

1. The correct encoding of, say, SHA-256 as an AlgorithmIdentifer *was*
   to omit the parameter. So if you're using it in, I dunno, CMS, you
   should omit it.

2. Due to a mishap, RSASSA-PSS originally said otherwise and included
   it. Additionally, there are some implementations that only work if
   you include it.

3. Once the mistake was discovered, PSS chose to preserve the mistake,
   rather than undo it.

This means that the correct encoding of SHA-256 as an AlgorithmIdentifer
is *different* depending on whether you're doing PSS or CMS.
Fortunately, there are only two users of this function, one inside the
library and one in Android. Both are trying to encode PSS, so the
current behavior is correct. Nonetheless, we should document this.

Also, because this is a huge mess, we should also add an API for
specifically encoding RSA-PSS. From there, we can update Android to call
that function and remove X509_ALGOR_set_md.

Amusingly, RSASSA-PKCS1-v1_5 *also* differs from the "correct" encoding.
RFC 8017, Appendix B.1 says:

   The parameters field associated with id-sha1, id-sha224, id-sha256,
   id-sha384, id-sha512, id-sha512/224, and id-sha512/256 should
   generally be omitted, but if present, it shall have a value of type
   NULL.

   This is to align with the definitions originally promulgated by NIST.
   For the SHA algorithms, implementations MUST accept
   AlgorithmIdentifier values both without parameters and with NULL
   parameters.

   Exception: When formatting the DigestInfoValue in EMSA-PKCS1-v1_5
   (see Section 9.2), the parameters field associated with id-sha1,
   id-sha224, id-sha256, id-sha384, id-sha512, id-sha512/224, and
   id-sha512/256 shall have a value of type NULL.  This is to maintain
   compatibility with existing implementations and with the numeric
   information values already published for EMSA-PKCS1-v1_5, which are
   also reflected in IEEE 1363a [IEEE1363A].

Finally, there's EVP_marshal_digest_algorithm, used in PKCS#8 and OCSP.
I suspect we're doing that one wrong. I've left a TODO there to dig into
that one.

Bug: 710
Change-Id: I46b11f8c56442a9badd186c7f04bb366147ed98f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67088
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>