Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkey: Use openssl generated pkcs8 key instead #830

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

samuel40791765
Copy link

In the test_private_encoding_encrypted test, we have a rsa private key pem file that was generated with certtool. I'm not entirely sure if the command that was used to generated the file was correct though. Dumping out the ASN.1 contents of this file indicates that this file was generated with an additional --pkcs-cipher=aes-128. When I try running the same command as the documentation indicates, the pem file would be encrypted with the default 3des-pkcs12 instead.

  • Note: ASN.1 contents were dumped out with der-ascii and running der2ascii -pem -i rsa.pem > rsa.txt.

The commentary above is fairly minor. The larger issue is that certtool's generated pkcs8 files aren't entirely consistent with RFC8018 Appendix A.2. certtool's PKCS8 private keys have a missing NULL field that should be paired with the Algorithm ID when using PBKDF2-PRF. OpenSSL doesn't seem to notice this, although this does fail with stricter parsers that explicitly check the field. I've cut an issue to GnuTLS regarding this, but it would be awesome if we could use a key file that's consistent with RFC guidelines for now instead.
OpenSSL's generated PKCS8 keys don't have the same issue described, so I've replaced the key file for an OpenSSL generated command along with the command I used to generate.

@rhenium
Copy link
Member

rhenium commented Dec 21, 2024

This is interesting. The parameters field in AlgorithmIdentifier is marked as OPTIONAL in the ASN.1 module. RFC 8018 specifies the type is NULL, but nowhere in RFC 8018 does it say whether the field should be present or not.

I've looked at a few RFCs that use AlgorithmIdentifier and they're not consistent. I'm under the impression that the field may or may not be present in PBKDF2, too.

  • RFC 4231 (HMAC test vectors) says "the parameter component SHOULD be present but have type NULL" in AlgorithmIdentifier with the same HMAC OIDs as in RFC 8018.
    https://datatracker.ietf.org/doc/html/rfc4231#section-3.1
  • RFC 8017 (PKCS#1, published 2 months earlier than RFC 8018) says in Appendix B.1, parameters "should generally be omitted, but if present, it shall have a value of type NULL." This is about hash functions and not HMAC, however.
    https://datatracker.ietf.org/doc/html/rfc8017#appendix-B.1
  • RFC 5754 (CMS) says "[CMS] Implementations MUST accept SHA2 AlgorithmIdentifiers with absent parameters. Implementations MUST accept SHA2 AlgorithmIdentifiers with NULL parameters. Implementations MUST generate SHA2 AlgorithmIdentifiers with absent parameters." This is also about hash functions.
    https://datatracker.ietf.org/doc/html/rfc5754#section-2

I didn't know there was such a difference between OpenSSL and GnuTLS, but I wonder if this might have accidentally revealed an edge case not considered in AWS-LC/BoringSSL if they don't parse it.

When I try running the same command as the documentation indicates, the pem file would be encrypted with the default 3des-pkcs12 instead.

I think it's the command @junaruga used to generate the current PEM block in #790. I tried with certtool from GnuTLS 3.8.8 and it used the same PBES2 parameters with AES. I imagine it could depend on the GnuTLS's version and other variables.

@samuel40791765
Copy link
Author

I think it’s the command @junaruga used to generate the current PEM block in #790. I tried with certtool from GnuTLS 3.8.8 and it used the same PBES2 parameters with AES. I imagine it could depend on the GnuTLS’s version and other variables.

Yeah you’re absolutely right, I was misusing an older version of GnuTLS and looking at an older revision of it as well. Later versions past 3.7.3 changed the defaults to be AES-128 with HMAC-SHA256 instead. Sorry for the confusion there.

@samuel40791765
Copy link
Author

Thanks for sharing the multiple RFCs that use AlgorithmIdentifier! It does help shed more light on the context of it’s definition. These RFCs are referring to the same AlgorithmIdentifier defined below.

      AlgorithmIdentifier  ::=  SEQUENCE  {
        algorithm   OBJECT IDENTIFIER,
        parameters  ANY DEFINED BY algorithm OPTIONAL  }

This was taken from RFC5754, Section 1, but a similar definition can be found in RFC8017's and RFC8018’s Appendix C.

One thing I'd like to add is that I think OPTIONAL only means that the field could be defined or ignored depending on the context where AlgorithmIdentifier is used. There are certain cases where the parameters field is required if specified. A good example is PBKDF2, which is also defined using AlgorithmIdentifier from the same RFC, where the following is stated: “The parameters field associated with this OID in an AlgorithmIdentifier shall have type PBKDF2-params:…“. This means that parameters must be of the type PBKDF2-params to provide more context on how PBKDF2 was used. There could be instances where parameters is omitted for other algorithms like you’re implying, but I’ll need to look around for an example.

For hmacWithSHA1, there’s this sentence in RFC8018 B.1.1 that specifies that NULL shall be specified.

   The object identifier id-hmacWithSHA1 identifies the HMAC-SHA-1
   pseudorandom function:

   id-hmacWithSHA1 OBJECT IDENTIFIER ::= {digestAlgorithm 7}

   The parameters field associated with this OID in an
   AlgorithmIdentifier shall have type NULL.  This object identifier is
   employed in the object set PBKDF2-PRFs (Appendix A.2).

Unfortunately the RFC does not specify the same for other hmacWithSHA*s... which does mean leave the door open for interpretation. OpenSSL’s ASN.1 parsers are known to be very general and relatively lax. I generated a file with GnuTLS 3.6.13 where NULL for parameters isn't paired with hmacWithSHA1. This file which I believe is non-consistent with the RFC8018 B.1.1 specification above, successfully passed OpenSSL’s pkcs8 parsers since OpenSSL doesn’t specifically check for the field.

Sidenote: I did find this interesting newer RFC9579 that was meant to be an amendment upon RFC8018. Appendix B in this RFC does have examples where other hmacWithSHA*s follow the same pattern as hmacWithSHA1's specification. Unfortunately, there’s no concrete wording on the specification like RFC8018 has for hmacwithSHA1 however and this RFC does seem to be more directed towards PKCS12 😞.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants