RFR: 8313367: SunMSCAPI cannot read Local Computer certs w/o Windows elevation [v4]

[Mat Carter]

On Fri, 22 Mar 2024 22:25:47 GMT, rebarbora-mckvak <duke at 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