On Fri, 22 Mar 2024 22:25:47 GMT, rebarbora-mckvak <d...@openjdk.org> wrote:

>> This fixes the defect described at 
>> https://bugs.openjdk.org/browse/JDK-8313367
>> 
>> If the process does not have write permissions, the store is opened as 
>> read-only (instead of failing).
>> 
>> Please note that permissions to use a certificate in a local machine store 
>> must be granted - in a management console, select a certificate, right-click 
>> -> All tasks... -> Manage Private Keys... -> add Full control to user.
>
> rebarbora-mckvak has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8313367: signHash looks for a key in either user or machine store

src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp line 801:

> 799:                 (BYTE *)pbData, &cbData, 0);
> 800: 
> 801:             DWORD keysetType;

This should be initialized explicitly to zero (0) in case the method returns an 
error, we want 0 (through failure or because the CRYPT_KEYMACHINE_KEYSET flag 
was not used when the key was created), or we want this set by the 
CryptGetProvParam to CRYPT_KEYMACHINE_KEYSET

src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp line 803:

> 801:             DWORD keysetType;
> 802:             ::CryptGetProvParam((HCRYPTPROV)hCryptProv, PP_KEYSET_TYPE, 
> //deprecated
> 803:                 (BYTE*)&keysetType, &cbData, 0);

I think this is a bug, cbData starts of initialized to 256 (for the length of 
pbData), however a sideeffect of the previous call can change that (going in to 
the call, informs the method of the number of bytes that can be stored in 
pbData, and coming out indicates the number of bytes actually store)

for this second call to CryptGetProvParam cbData should be the number of bytes 
in the DWORD keysetType.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16687#discussion_r1717581410
PR Review Comment: https://git.openjdk.org/jdk/pull/16687#discussion_r1717577098

Reply via email to