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

Set SHA-2 as supportedCMSTypes instead of 3DES #1254

Merged
merged 1 commit into from Jul 7, 2022

Conversation

jrisc
Copy link
Contributor

@jrisc jrisc commented Jun 1, 2022

The PKINIT client uses AuthPack.supportedCMSTypes to let the KDC know
about the algorithms it supports for verification of CMS data signature.

This commit replaces 3DES, which is currently the only algorithm
provided in this list and could be considered outdated, by SHA-256 and
SHA-512. But more algorithms could be added. As a comparison, the list used by Heimdal includes more algorithms.

This change will not affect the way the MIT KDC handles PKINIT requests because the CMS data digest (SHA-256) is hard coded. However, this is important for integration with Heimdal because this KDC selects the digest algorithm from request's supportedCMSTypes list.

@greghudson
Copy link
Member

greghudson commented Jun 11, 2022

Has this been tested against Heimdal?

sha256_oid and sha512_oid from pkinit_constants.c are the OIDs defined in RFC 8636 (PKINIT Algorithm Agility): 1.3.6.1.5.2.3.6.2 and 1.3.6.1.5.2.3.6.3. I don't think they will match the sha256_alg and sha512_alg digest entries in Heimdal's sig_algs array, which are 2.16.840.1.101.3.4.2.1 and 2.16.840.1.101.3.4.2.3 from RFC 3560 (a CMS RFC).

@greghudson
Copy link
Member

greghudson commented Jun 14, 2022

Some notes from further investigation:

  • supportedCMSTypes is an optional field in the RFC 4556 AuthPack, which is sent from the client to the KDC in a PKINIT-authenticated AS-REQ. According to RFC 4556 it can contain "key transport algorithms, or content encryption algorithms, or signature algorithms". Key transport and content encryption algorithms only apply to the public key encryption mode of PKINIT, which is rarely used (but is present in all three major Kerberos implementations). For the more commonly-used Diffie-Hellman mode we only need to consider signature algorithms.

  • As noted in the PR, the MIT KDC ignores this field. Our client currently sends one algorithm 1.2.840.113549.3.7, which is the content encryption algorithm des-ede3-cbc from RFC 3370.

  • Heimdal stores this field in a "peer info" structure which is passed to hx509_cms_create_signed_1(). If the field is omitted entirely, the client is treated as having sent 1.2.840.113549.3.7, 1.2.840.113549.1.1.5, and 1.3.14.3.2.26. The first of these is des-ede3-cbc again. The second is the signature algorithm sha1WithRSAEncryption from RFC 3370. The third is the message digest algorithm sha-1 from RFC 3370--note that RFC 4556 does not say anything about message digest algorithms or including them in supportedCMSTypes.

  • When creating a SignedData container for Diffie-Hellman PKINIT, Heimdal looks for a message digest algorithm it can support in the client's supportedCMSTypes, defaulting to SHA-256 if none is found. The selected digest is computed over the container contents and included as a signed attribute (of type MessageDigest, 1.2.840.113549.1.9.4). [ETA: before defaulting to SHA-256, Heimdal selects the first algorithm in its list that will work with the certificate private key, which is typically SHA-512.]

  • Heimdal then looks for a signature algorithm it can support in supportedCMSTypes, defaulting to sha256WithRSAEncryption from RFC 4055 (1.2.840.113549.1.1.11). [ETA: before defaulting to sha256WithRSAEncryption, Heimdal selects the first algorithm in its list that will work with the certificate private key, which is typically sha512WithRSAEncryption.] This algorithm is used to compute the public key signature over the signed attributes.

  • [MS-PKCA] specifies that a few signature algorithms should be supported, but doesn't go into detail on how Windows handles supportedCMSTypes.

When interoperating with Heimdal during Diffie-Hellman PKINIT, the current value of supportedCMSTypes should result in Heimdal using its default digest and signature algorithm. The current PR code should not change that, as it uses the wrong OIDs to communicate SHA-256 and SHA-512 as CMS digest algorithms. If we sent the correct OIDs, Heimdal would switch to using a SHA-512 digest; I don't think that's preferred as it would make the KDC reply longer for an unnecessary benefit in security bit strength. Sending no supportedCMSTypes field would cause Heimdal to switch to less secure algorithms (SHA-1 instead of SHA-256).

Can I have more context as to how this PR arose? Is it about getting rid of the reference to triple-DES in the code, or was there an actual interop issue with Heimdal?

@jrisc
Copy link
Contributor Author

jrisc commented Jun 15, 2022

You are right about OIDs. The ones I used are meant for KDF and should not be used in supportedCMSTYpes:

The following ones are probably more relevant indeed:

According to RFC4556 section 3.2.1 step 5, supportedCMSTypes contains the list of CMS digest/signature algorithms in decreasing order of preference. Hence if SHA-256 is placed before SHA-512, the former will be selected in priority until the day the KDC stops supporting it.

However, Heimdal does not seem to comply with this RFC, because it follows the ordering of its own algorithms list, instead of supportedCMSTypes's one.

I got the wrong impression that this PR was working because SHA-256 was used. But knowing that Heimdal does not take the order of preference into account, and that the OID does not match, I realize it is actually just falling back to the default digest aglorithm, which is indeed SHA-256.

The same is happening with the current supportedCMSTypes MIT krb5 is providing: it's falling back to SHA-256 on Heimdal's side because 3DES is not part this KDC's supported digest list.

About the reason that motivated this PR: We ran into this issue because the supportedCMSTypes attribute was removed from AS-REQ in CentOS/RHEL release versions since we removed 3DES support. This caused Heimdal KDC to start using SHA-1 instead of SHA-256 for CMS data digest (as one of the 3 default digest algorithms it loads in this situation as you explained). But this usage of SHA-1 is not longer allowed on CentOS 9 Stream and RHEL9.

We could revert this change to go back to the upstream supportedCMSTypes with 3DES to take advantage of Heimdal's fallback sequence. But updating this attribute to reflect the actual list of supported algorithms sounds like a more proper approach.

I can get in touch with the Heimdal dev team to discuss the order of preference issue.

@greghudson
Copy link
Member

Thanks for the additional context. I have two small points of disagreement. First, RFC 4556 section 3.2.1 step 5 doesn't say anything about digest algorithms, only signature/encryption/key-transport algorithms. Second, I'm not sure it's really a conformance issue that Heimdal uses its own preference instead of the client's preference; although the RFC is clear that the order of supportedCMSTypes indicates a preference order, there is no statement that the KDC must obey that preference. I think it's more of a concern that Heimdal applies older defaults if supportedCMSTypes is absent than it does if the field is present but there's no agreement. Because of that foible we must send something, even if it's garbage.

From a protocol cleanliness perspective, I would prefer not to send digest algorithms, as it isn't supported by the RFC. There is no point in sending encryption and key-transport algorithms when we aren't using the public key encryption mode. That leaves signature algorithms. I can see a few options and I'm not completely fond of any of them. Procuring a full list of OpenSSL-supported CMS algorithms and ordering them by preference might be difficult. We could send a list of algorithms we know about from the RFCs that we know are supported in OpenSSL 1.0 and later. We could send a single algorithm that we prefer the most (probably sha256WithRSAEncryption). Or we could deliberately send a garbage OID, forcing the other side to pick something itself (basically what we do at the moment by sending an encryption algorithm).

@jrisc
Copy link
Contributor Author

jrisc commented Jun 16, 2022

Okay, I see your point. So if I summarize the situation the 3 options are:

  • Send the full list of supported signature algorithms
    • Ideally generated by OpenSSL
    • Might be difficult to order based on preference
    • Would cause Heindal KDC to pick the strongest compatible one, which is not optimal from performance perspective
  • Send one signature algorithm only
    • sha256WithRSAEncryption most likely
    • Will have to be updated when SHA-256 becomes deprecated
  • Send irrelevant OID to let Heimdal fallback to its own default algorithm
    • Current approach
    • Might break if Heimdal's default algorithm selection process is modified

Before taking a decision, I would like to test the behavior of AD KDC for each approach. MS-PKCA (section 2.2) only lists these algorithms as supported:

  • md5WithRSAEncryption (since Windows 2000)
  • sha1WithRSAEncryption (newer than Windows Server 2003)
  • ecdsa-with-sha1/256/384/512 (newer than Windows Server 2008)

@greghudson
Copy link
Member

There's a fourth option to send several signature algorithms that we pick from the RFCs ourselves (similar to Heimdal's approach).

Heimdal picking an unnecessarily bulky algorithm is only a minor negative. That's what it does when interoperating with itself, so it's more of an issue on their side as long as the exchange still succeeds.

@jrisc
Copy link
Contributor Author

jrisc commented Jun 20, 2022

I made some tests against Windows Sever 2019 for these 3 scenarios:

  • No supportedCMSTypes attribute
  • sha256WithRSAEncryption only
  • ecdsa-with-sha256/384/512 and sha256WithRSAEncryption

Each time, the CMS digest in the resulting AS-REP was SHA-1. It seems AD does not take supportedCMSTypes into account for algorithm selection here.

So it looks like Heimdal KDC is the only major implementation the MIT client has influence on the choice of the digest/signature algorithm.

What do you think a list of selected signature algorithms would look like? On our side, as long as it results in either SHA-256, SHA-384, or SHA-512 being picked as digest algorithm, we will be happy with it.

@greghudson
Copy link
Member

I think my preference has settled on sending just sha256WithRSAEncryption, since we are currently always using that type for the client authpack signature anyway.

From your Windows testing it sounds like if we want to interoperate without allowing SHA-1 we would need to implement RFC 5349 (PKINIT ECC). That would be nice anyway for performance and reduced message size, but I imagine it would be a fair amount of work.

@jrisc
Copy link
Contributor Author

jrisc commented Jun 23, 2022

I updated the pull request, but there is still something wrong in Heimdal's response:

PKINIT OpenSSL error: Failed to verify CMS message
PKINIT OpenSSL error: error:1700006B:CMS routines::content type not enveloped data
PKINIT OpenSSL error: error:02000068:rsa routines::bad signature
PKINIT OpenSSL error: error:1C880004:Provider routines::RSA lib
PKINIT OpenSSL error: error:1700009E:CMS routines::verification failure
PKINIT client could not verify DH reply

I am trying to figure out what the issue is.

@greghudson
Copy link
Member

I'm a bit puzzled by the "content type not enveloped data" message in the error queue, since this is clearly a call to cms_signeddata_verify() which does not traffic in enveloped data. But I'm guessing that is a red herring, and "rsa routines::bad signature" is what we're looking for the cause of.

@jrisc
Copy link
Contributor Author

jrisc commented Jun 28, 2022

My assumptions regarding the selection of the digest and signature algorithms by Heimdal were wrong. It is actually taking supportedCMSTypes's preference order into account. The code I was looking at in the first place was only selecting the default algorithm based on the type of the private key.

The PR code as it is right now (i.e. supportedCMSTypes contains sha256WithRSAEncryption only) results in the following algorithms selection:

  • Digest: Heimdal looks for a digest algorithm in supportedCMSTypes. None is found, so it falls back to the result of alg_for_privatekey(), which in case of a RSA key, is SHA-512 because the comparison applies to the key type only (i.e. ASN1_OID_ID_PKCS1_RSAENCRYPTION).
  • Signature: Matching with sha256WithRSAEncryption from suppportedCMSTypes, as expected.

I also noticed that, when requesting PKINIT pre-auth against Heimdal KDC either with sha256WIthRSAEncryption+sha256 or sha512WithRSAEncryption+sha512 as supportedCMSTypes, both succeed. The CMS routines::verification failure seems to occur only when the digest and signature algorithms are not based on the same hashing algorithm. Hence there is probably a confusion between those somewhere...

The error is caused by a failing verification of SignerInfo. The digest and signature algorithm identifiers are valid though.

@greghudson
Copy link
Member

Thanks for the continued investigation. When we send only irrelevant OIDs (as in current upstream or the original PR), I guess Heimdal selects sha512 and sha512WithRSAEncryption (both via alg_for_privatekey()), not sha256 and sha256WithRSAEncryption as I had previously guessed.

I'm not immediately sure why CMS_SignerInfo_verify() would get tripped up by the signature algorithm using a different (internal) digest.

@greghudson
Copy link
Member

I set up a Heimdal environment and reproduced the interop failure with the current PR code.

The SignerInfo object being verified has digestAlgorithm sha512 and signatureAlgorithm sha256WithRSAEncryption. RFC 5652 section 5.3 says "digestAlgorithm identifies the message digest algorithm, and any associated parameters, used by the signer." To me this is ambiguous; is it the digest algorithm used to compute the signature field, or was it used when computing the message-digest signed attribute? (And if the former, why is it supplied separately from signatureAlgorithm when the signature algorithm contains an internal digest?)

OpenSSL's CMS_SignerInfo_verify() appears to use the digestAlgorithm when computing the signature, making no reference to signatureAlgorithm. It appears to be used again in CMS_SignerInfo_verify_content() to verify the message-digest attribute, although I'm not certain what ossl_cms_DigestAlgorithm_find_ctx() is doing.

So I'm not sure which of Heimdal or OpenSSL is at fault, but since we can't make assumptions about bugfixes in either, we need to send something that results in Heimdal using a matching digest and signature algorithm. That narrows the options to:

  1. Send an irrelevant OID.
  2. Send both a signature and a digest algorithm, with matching digest and signature algorithms coming first.
  3. Send only signature algorithms, putting sha512WithRSAEncryption first to match the digest algorithm Heimdal will select by default for an RSA key.

Option 3 seems like the most standards-conformant, so let's try sending {sha512WithRSAEncryption, sha256WithRSAEncryption}.

@jrisc
Copy link
Contributor Author

jrisc commented Jun 30, 2022

Thank you for sorting this out. I added sha512WithRSAEncryption as first element of the list, and tested this configuration against Heimdal KDC. As expected, it works.

Feel free to merge the PR if you are okay with its current content. On my side, I will notify OpenSSL developers about this issue. Hopefully we can have both ends agreeing on a common behavior eventually.

@greghudson
Copy link
Member

I filed heimdal/heimdal#1000 about the Heimdal peculiarities related to this. I expect to merge the current PR soon (probably with revisions).

The PKINIT client uses AuthPack.supportedCMSTypes to let the KDC know
the algorithms it supports for verification of the CMS data signature.
(The MIT krb5 KDC currently ignores this list, but other
implementations use it.)

Replace 3DES with sha512WithRSAEncryption and sha256WithRSAEncryption.

[ghudson@mit.edu: simplified code and used appropriate helpers; edited
commit message]

ticket: 9066 (new)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants