On Thu, 23 Jun 2022 22:07:42 GMT, zzambers <d...@openjdk.org> wrote:
>> TLS `*_CHACHA20_POLY1305_*` cipher suites are currently broken when
>> configuration with SunPKCS11 provider is used. I discovered this by my
>> ssl-tests testsuite [1].
>>
>>
>> make TEST_PKCS11_FIPS=1
>> SSLTESTS_SSL_CONFIG_FILTER=SunJSSE,Default,TLSv1.2,TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256
>> SSLTESTS_CUSTOM_JAVA_PARAMS=-Djdk.tls.ephemeralDHKeySize=2048 ssl-tests
>> ...
>> javax.net.ssl.SSLException: Unknown algorithm: ChaCha20-Poly1305
>> at java.base/sun.security.ssl.Alert.createSSLException(Alert.java:132)
>> at
>> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:371)
>> at
>> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:314)
>> at
>> java.base/sun.security.ssl.TransportContext.fatal(TransportContext.java:309)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.handleException(SSLSocketImpl.java:1712)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:470)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:426)
>> at SSLSocketClient.test(SSLSocketClient.java:72)
>> at SSLSocketTester.testConfiguration(SSLSocketTester.java:392)
>> at SSLSocketTester.testConfigurations(SSLSocketTester.java:322)
>> at SSLSocketTester.testProvider(SSLSocketTester.java:234)
>> at SSLSocketTester.testProviders(SSLSocketTester.java:190)
>> at Main.main(Main.java:30)
>> Caused by: java.security.ProviderException: Unknown algorithm:
>> ChaCha20-Poly1305
>> at
>> jdk.crypto.cryptoki/sun.security.pkcs11.P11TlsKeyMaterialGenerator.engineGenerateKey(P11TlsKeyMaterialGenerator.java:168)
>> at
>> java.base/javax.crypto.KeyGenerator.generateKey(KeyGenerator.java:564)
>> at
>> java.base/sun.security.ssl.SSLTrafficKeyDerivation$LegacyTrafficKeyDerivation.<init>(SSLTrafficKeyDerivation.java:282)
>> at
>> java.base/sun.security.ssl.SSLTrafficKeyDerivation$T12TrafficKeyDerivationGenerator.createKeyDerivation(SSLTrafficKeyDerivation.java:117)
>> at
>> java.base/sun.security.ssl.SSLTrafficKeyDerivation.createKeyDerivation(SSLTrafficKeyDerivation.java:79)
>> at
>> java.base/sun.security.ssl.DHClientKeyExchange$DHClientKeyExchangeProducer.produce(DHClientKeyExchange.java:221)
>> at
>> java.base/sun.security.ssl.ClientKeyExchange$ClientKeyExchangeProducer.produce(ClientKeyExchange.java:65)
>> at
>> java.base/sun.security.ssl.SSLHandshake.produce(SSLHandshake.java:440)
>> at
>> java.base/sun.security.ssl.ServerHelloDone$ServerHelloDoneConsumer.consume(ServerHelloDone.java:182)
>> at
>> java.base/sun.security.ssl.SSLHandshake.consume(SSLHandshake.java:396)
>> at
>> java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:480)
>> at
>> java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:458)
>> at
>> java.base/sun.security.ssl.TransportContext.dispatch(TransportContext.java:201)
>> at java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:172)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.decode(SSLSocketImpl.java:1510)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.readHandshakeRecord(SSLSocketImpl.java:1425)
>> at
>> java.base/sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:455)
>> ... 7 more
>>
>> FAILED: SunJSSE/Default: TLSv1.2 + TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256
>>
>>
>> Problem:
>> Exception is thrown by P11TlsKeyMaterialGenerator.engineGenerateKey method
>> [2], based on result of P11SecretKeyFactory.getKeyType method [3], which
>> only "knows" "ChaCha20" key algorithm, but does not accept
>> "ChaCha20-Poly1305" as algorithm. Algorithm value is passed from
>> SSLTrafficKeyDerivation.LegacyTrafficKeyDerivation class [4], which leads to
>> algorithm field in SSLCipher class [5]. Value of that field comes from
>> cipher name in JsseJce class [6] (ending at first slash, if any).
>>
>> Fix:
>> This fix basically modifies P11SecretKeyFactory.getKeyType method to accept
>> "ChaCha20-Poly1305" as alias for "ChaCha20".
>>
>> Testing:
>> I ran jdk_security tests locally and they passed. Also failure in ssl-tests
>> gets fixed.
>>
>> [1]
>> https://urldefense.com/v3/__https://github.com/zzambers/ssl-tests__;!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXhcORrrb$
>>
>> [2]
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/b7a34f728d0653d55ef01da045c9aad4c0471143/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11TlsKeyMaterialGenerator.java*L168__;Iw!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXroZ9zf5$
>>
>> [3]
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/b7a34f728d0653d55ef01da045c9aad4c0471143/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11SecretKeyFactory.java*L101__;Iw!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXr9N27hf$
>>
>> [4]
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/b7a34f728d0653d55ef01da045c9aad4c0471143/src/java.base/share/classes/sun/security/ssl/SSLTrafficKeyDerivation.java*L270__;Iw!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXt6Noj0J$
>>
>> [5]
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/b7a34f728d0653d55ef01da045c9aad4c0471143/src/java.base/share/classes/sun/security/ssl/SSLCipher.java*L496__;Iw!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXlpeoiLU$
>>
>> [6]
>> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/b7a34f728d0653d55ef01da045c9aad4c0471143/src/java.base/share/classes/sun/security/ssl/JsseJce.java*L81__;Iw!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXvb4GCuu$
>>
>
> zzambers has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Link in comment made pernament
> - ChaCha20-Poly1305 added as key type
test/jdk/sun/security/pkcs11/tls/TestKeyMaterialChaCha20.java line 57:
> 55: System.out.println("Skipping, ChaCha20 not supported");
> 56: return;
> 57: }
This test does not use CKM_CHACHA20_KEY_GEN, and the test passes when I run it
against the version of NSS which does not support CKM_CHACHA20_KEY_GEN. Since
the test requires the provider to support "SunTlsRsaPremasterSecret",
"SunTls12MasterSecret", and "SunTls12KeyMaterial" key generators, probably
should base the check on them instead?
test/jdk/sun/security/pkcs11/tls/TestKeyMaterialChaCha20.java line 74:
> 72:
> 73: //
> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/ccec5d1e8529c8211cc678d8acc8d37fe461cb51/src/java.base/share/classes/sun/security/ssl/SSLTrafficKeyDerivation.java*L270__;Iw!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXqs5Fs1V$
>
> 74: //
> https://urldefense.com/v3/__https://github.com/openjdk/jdk/blob/ccec5d1e8529c8211cc678d8acc8d37fe461cb51/src/java.base/share/classes/sun/security/ssl/CipherSuite.java*L93__;Iw!!ACWV5N9M2RV99hQ!NkKDKTC1HJP1zRwYPBG_S8tcH8-fjaZQqUESU0pGPJNMl-_nUg9yiQiJ7eZm35pRa22MQKfBPiChpF2RXuIwam32$
>
nit: do these two links really needed? If yes, add some more text for their
purpose?
-------------
PR: https://git.openjdk.org/jdk/pull/9072
