RFR: 8301553: Support Password-Based Cryptography in SunPKCS11 [v3]

Francisco Ferrari Bihurriet duke at openjdk.org
Wed May 31 21:22:23 UTC 2023


On Tue, 23 May 2023 14:00:30 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> We found several more cases of passwords and encoded keys not cleared that were addressed in out Iteration # 2 commit. These cases were both in Java and native code. We still have doubts about the effectiveness and need for these actions, but prefer to have the discussion on a different channel.
>> 
>> We also found that invalid keys (not starting with the name "PBE") passed to PBES2Core::engineInit or P11PBECipher::engineInit were being cleared and this could be unexpected to the caller. This is related to my comment [here](https://github.com/openjdk/jdk/pull/12396#discussion_r1199513839). For these cases, we decided not to invoke ::getEncoded and not to clean. As said, we have concerns about these undocumented mutations to objects passed by argument.
>> 
>> No test regressions observed in jdk/com/sun/crypto/provider or jdk/sun/security/pkcs11.
>
> @martinuy I am adding a CSR requirement for this issue. In this Enhancement, you are adding support for new algorithms in a JCE provider. This type of change requires a CSR as you are adding support for algorithms that can be used by applications and not just inside the JDK, thus it is a type of exported interface of JDK scope. For an example of a similar issue, see [JDK-8274174](https://bugs.openjdk.org/browse/JDK-8274174). The CSR should also include details about any behavioral changes or differences that affect use of the algorithms through the standard JCE APIs.

>> @seanjmullan @valeriepeng , can you please take a look at the proposed [JDK-8308719](https://bugs.openjdk.org/browse/JDK-8308719) CSR?
>
> Under the "Specification" section, the Cipher ones list out the required mech(s). But the rest, e.g. SecretKeyFactory and Mac, have X and Y. Based on the code in SunPKCS11.java, it seems Y is the required one and X is the one which is used.

@valeriepeng: I think there are a couple of things worth clarifying. Let's take one example of each service type.


## Cipher.PBEWithHmacSHA1AndAES_128

https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java#L883-L885

Here, one of `CKM_AES_CBC_PAD` or `CKM_AES_CBC` must be present for the _encryption/decryption_ operation (performed by the [underlying _Cipher_](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11PBECipher.java#L66)). Additionally, we require `CKM_PKCS5_PBKD2` for the _derivation_ operation, and `CKM_SHA_1_HMAC` as way to anticipate the `CKP_PKCS5_PBKD2_HMAC_SHA1` availability:
https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecretKeyFactory.java#L198-L199

A totally explicit entry would be:

|          Java Algorithm          |                         PKCS<span>#</span>11 Mechanisms                         |
|:--------------------------------:|:-------------------------------------------------------------------------------:|
| Cipher.PBEWithHmacSHA1AndAES_128 | (`CKM_AES_CBC_PAD` or `CKM_AES_CBC`) and `CKM_PKCS5_PBKD2` and `CKM_SHA_1_HMAC` |

But we decided to follow the _comma ⟹ "or"_ convention, previously established in the table, so we added the additionally required mechanisms as a parenthesised note:

|          Java Algorithm          |                                PKCS<span>#</span>11 Mechanisms                                 |
|:--------------------------------:|:----------------------------------------------------------------------------------------------:|
| Cipher.PBEWithHmacSHA1AndAES_128 | `CKM_AES_CBC_PAD`, `CKM_AES_CBC` (`CKM_PKCS5_PBKD2` and `CKM_SHA_1_HMAC` required in any case) |

## Mac.HmacPBESHA1

https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java#L620-L621

Here, `CKM_SHA_1_HMAC` must be present for the _MAC generation/verification_ operation (performed by [letting the derived key continue to the _Mac_ implementation](https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Mac.java#L227-L251)). Additionally, we require `CKM_PBA_SHA1_WITH_SHA1_HMAC` for the _derivation_ operation:
https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecretKeyFactory.java#L230-L231

A totally explicit entry would be:

| Java Algorithm  |          PKCS<span>#</span>11 Mechanisms           |
|:---------------:|:--------------------------------------------------:|
| Mac.HmacPBESHA1 | `CKM_SHA_1_HMAC` and `CKM_PBA_SHA1_WITH_SHA1_HMAC` |

This is what we wrote (in the inverse order).

## SecretKeyFactory.PBEWithHmacSHA1AndAES_128

https://github.com/openjdk/jdk/blob/1c596e147dac5102b31a946c905843b9b19b428e/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java#L736-L737

Since the SecretKeyFactory service just derives, we require `CKM_PKCS5_PBKD2` for the _derivation_ operation, and `CKM_SHA_1_HMAC` as way to anticipate the `CKP_PKCS5_PBKD2_HMAC_SHA1` availability.

A totally explicit entry would be:

|               Java Algorithm               |    PKCS<span>#</span>11 Mechanisms     |
|:------------------------------------------:|:--------------------------------------:|
| SecretKeyFactory.PBEWithHmacSHA1AndAES_128 | `CKM_PKCS5_PBKD2` and `CKM_SHA_1_HMAC` |

This is what we wrote.

----
# Proposal

> We should either follow the convention as in Cipher or explain what "X and Y" means. Rest looks good to me.

We are more or less following the same convention as in Cipher, using _comma_ instead of _"and"_ for Mac.HmacPBESHA1 and SecretKeyFactory.PBEWithHmacSHA1AndAES_128 would wrongly imply that any of the mechanisms is enough.

With the above explanations, do you still think that this table needs more clarification? Would it improve if we add a trailing _"required in any case"_ at the end, just like the parenthesised notes? Something like:

|               Java Algorithm               |                                PKCS<span>#</span>11 Mechanisms                                 |
|:------------------------------------------:|:----------------------------------------------------------------------------------------------:|
|                  […]                  |                                            […]                                            |
|      Cipher.PBEWithHmacSHA1AndAES_128      | `CKM_AES_CBC_PAD`, `CKM_AES_CBC` (`CKM_PKCS5_PBKD2` and `CKM_SHA_1_HMAC` required in any case) |
|                  […]                  |                                            […]                                            |
|              Mac.HmacPBESHA1               |            `CKM_SHA_1_HMAC` and `CKM_PBA_SHA1_WITH_SHA1_HMAC` required in any case             |
|                  […]                  |                                            […]                                            |
| SecretKeyFactory.PBEWithHmacSHA1AndAES_128 |                  `CKM_PKCS5_PBKD2` and `CKM_SHA_1_HMAC` required in any case                   |

-------------

PR Comment: https://git.openjdk.org/jdk/pull/12396#issuecomment-1570972521



More information about the security-dev mailing list