On Thu, 29 May 2025 00:54:37 GMT, Bradford Wetmore <wetm...@openjdk.org> wrote:
>> Adds the RFC 5705/8446 TLS Key Exporters API/implementation to JSSE/SunJSSE >> respectively. >> >> CSR is complete/approved. >> >> Tests include new unit tests for TLSv1-1.3. Have run tier1-2, plus the JCK >> API (jck:api/java_security jck:api/javax_crypto jck:api/javax_net >> jck:api/javax_security jck:api/org_ietf jck:api/javax_xml/crypto) > > Bradford Wetmore has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 37 commits: > > - Merge branch 'master' into JDK-8341346 > - Remove TlsExporterKeyingMaterial for now. Can add later if needed. > - Merge branch 'master' into JDK-8341346 > - Merge branch 'master' into JDK-8341346 > - Added PKCS11 testing > - Minor bug > - Missed one change > - Merge branch 'master' into JDK-8341346 > - Private Codereview comment: Don't allow use of null keyAlgs, plus some > minor cleanups > - get*() no longer needed, backout error (oops!) > - ... and 27 more: https://git.openjdk.org/jdk/compare/2ec6ab34...858362c8 src/java.base/share/classes/javax/net/ssl/ExtendedSSLSession.java line 176: > 174: * RFC 8446 (TLSv1.3) treats a null context as non-null/empty. > 175: * <P> > 176: * The {@code label} {@code String} will be converted to bytes using Two consecutive `{@code}` look a little strange to me, visually. How about just `{@code label} will be...`? You already use that in `@param label`. Same as in the other method. src/java.base/share/classes/javax/net/ssl/ExtendedSSLSession.java line 198: > 196: * {@code label} will be converted to a {@code byte[]} > 197: * before the operation begins. > 198: * @param context the context bytes used in the EKM calculation, or > null `null` needs to be in `{@code}`. Same as in `@throws NullPointerException`. Same as in the other method. src/java.base/share/classes/sun/security/internal/spec/TlsPrfParameterSpec.java line 85: > 83: * @param secret the secret to use in the calculation (or null) > 84: * @param keyAlg the algorithm name that the generated SecretKey > should > 85: * have, or null if the default should be used You no longer allow it to be `null`. src/java.base/share/classes/sun/security/ssl/SSLSessionImpl.java line 1494: > 1492: if (exporterMasterSecret == null) { > 1493: throw new RuntimeException( > 1494: "Exporter master secret not captured"); Do you want to choose another exception type? Like `ProviderException`. Actually, how unlikely this is? If you believe this would never happen (unless there is a programming error), you can even throw an `AssertionError`. Same question in `useTLS10PlusSpec()` for the two randoms. test/jdk/javax/net/ssl/ExtendedSSLSession/ExportKeyingMaterialTests.java line 1: > 1: /* Do you want to test about null/empty context difference in TLS 1.2/1.3? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24976#discussion_r2114057734 PR Review Comment: https://git.openjdk.org/jdk/pull/24976#discussion_r2114060429 PR Review Comment: https://git.openjdk.org/jdk/pull/24976#discussion_r2114065976 PR Review Comment: https://git.openjdk.org/jdk/pull/24976#discussion_r2114120216 PR Review Comment: https://git.openjdk.org/jdk/pull/24976#discussion_r2114083657
