RFR: 8331008: Implement JEP 478: Key Derivation Function API (Preview) [v47]
Weijun Wang
weijun at openjdk.org
Tue Sep 24 23:35:00 UTC 2024
On Tue, 24 Sep 2024 21:39:32 GMT, Kevin Driver <kdriver at openjdk.org> wrote:
>> src/java.base/share/classes/javax/crypto/KDF.java line 546:
>>
>>> 544: if (checkSpiNonNull(theOne)) return;
>>> 545:
>>> 546: synchronized (lock) {
>>
>> Sorry I missed this last time. The `if (checkSpiNonNull(theOne)` check also must be repeated inside the `synchronized` block.
>>
>> Suppose 2 threads, one calling `deriveData` and one calling `getProviderName`, are running at the same time. Both start with `theOne` being null and reach the block. The `deriveData` thread gets the lock, chooses a provider, and assigns `theOne`. When it releases the lock the `getProviderName` thread enters this block and updated `theOne`. This should not happen since `theOne` should only be assigned once.
>>
>> The `KDFDelayedProviderSyncTest.java` hasn't been able to catch this. Since we don't have states inside the HKDF implementation and there is only one candidate, the test is likely to always succeed even if the DPS process is not implemented correctly. I can imagine there is a case that there are 2 HKDF implementations and the 1st one (the `candidate`) always fails at `deriveData`. In my supposed scenario above, the 1st `deriveData` succeeds but if `theOne` was reset to `candidate` later, the next `deriveData` would fail.
>
> Addressed in: https://github.com/openjdk/jdk/pull/20301/commits/52ef5b0095dda55491286ede5ad18e38d700124d.
Have you thought about the test?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1774219844
More information about the security-dev
mailing list