On Sat, 20 Mar 2021 17:41:45 GMT, SalusaSecondus <github.com+829871+salusasecon...@openjdk.org> wrote:
>> I think that this code will work, but since there already is >> [P11RSAPrivateKey and >> P11RSAPrivateNonCRTKey](https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Key.java#L375-L406) >> (both of which implement the standard `RSAPrivateKey` and >> `RSAPrivateCrtKey` interfaces respectively), can we just depend on and >> simplify our code? (My concern is that there is some unknown reason for >> calling `C_GetAttributeValue` directly here.) >> >> For example (based on the non-P11 code above) >> } else if (key instanceof RSAPrivateKey) { >> if (keySpec.isAssignableFrom(PKCS8_KEYSPEC_CLS)) { >> return keySpec.cast(new >> PKCS8EncodedKeySpec(key.getEncoded())); >> } else if (keySpec.isAssignableFrom(RSA_PRIVCRT_KEYSPEC_CLS)) { >> if (key instanceof RSAPrivateCrtKey) { >> RSAPrivateCrtKey crtKey = (RSAPrivateCrtKey)key; >> return keySpec.cast(new RSAPrivateCrtKeySpec( >> crtKey.getModulus(), >> crtKey.getPublicExponent(), >> crtKey.getPrivateExponent(), >> crtKey.getPrimeP(), >> crtKey.getPrimeQ(), >> crtKey.getPrimeExponentP(), >> crtKey.getPrimeExponentQ(), >> crtKey.getCrtCoefficient(), >> crtKey.getParams() >> )); >> } else if (key instanceof RSAPrivateKey) { >> if (!keySpec.isAssignableFrom(RSA_PRIV_KEYSPEC_CLS)) { >> throw new InvalidKeySpecException >> ("RSAPrivateCrtKeySpec can only be used with CRT >> keys"); >> } >> >> // fall through to RSAPrivateKey (non-CRT) >> RSAPrivateKey rsaKey = (RSAPrivateKey) key; >> return keySpec.cast(new RSAPrivateKeySpec( >> rsaKey.getModulus(), >> rsaKey.getPrivateExponent(), >> rsaKey.getParams() >> )); >> } else { // New P11 code starts here and handles the >> P11PrivateKey case >> throw new InvalidKeySpecException("RSA Private key is >> not extractable"); >> } // End new P11 code >> } else { >> throw new InvalidKeySpecException >> ("KeySpec must be RSAPrivate(Crt)KeySpec or " >> + "PKCS8EncodedKeySpec for RSA private keys"); >> } > > I've used your code for both because it looks like a good solution. > > One thing which puzzles me is that the following implementation for the P11 > code passes all tests and avoids any interaction with P11 library at all. I > didn't want to use it because I'm concerned that there is an untested > edge-case we'd bump into. > > <T extends KeySpec> T implGetPrivateKeySpec(P11Key key, Class<T> keySpec, > Session[] session) throws PKCS11Exception, > InvalidKeySpecException { > if (key instanceof RSAPrivateKey) { > try { > return implGetSoftwareFactory().getKeySpec(key, keySpec); > } catch (final InvalidKeySpecException e) { > throw e; > } catch (final GeneralSecurityException e) { > throw new InvalidKeySpecException("Could not generate > KeySpec", e); > } > } else { > throw new InvalidKeySpecException("Key is not extractable"); > } > } Well, when the key object has all the attribute values available, things will often work regardless which path you took. ------------- PR: https://git.openjdk.java.net/jdk/pull/2949
