Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v5]
On Wed, 2 Dec 2020 17:14:11 GMT, Jamil Nimeh wrote: >> Hello all, >> This change brings in support for certificates with EdDSA keys (both Ed25519 >> and Ed448) allowing those signature algorithms to be used both on the >> certificates themselves and used during the handshaking process for messages >> like CertificateVerify, ServerKeyExchange and so forth. > > Jamil Nimeh has updated the pull request incrementally with one additional > commit since the last revision: > > Applied code review comments There are still a few minor concerns, but nothing significant. We could have them addressed in separate bugs. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v5]
> Hello all, > This change brings in support for certificates with EdDSA keys (both Ed25519 > and Ed448) allowing those signature algorithms to be used both on the > certificates themselves and used during the handshaking process for messages > like CertificateVerify, ServerKeyExchange and so forth. Jamil Nimeh has updated the pull request incrementally with one additional commit since the last revision: Applied code review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/1197/files - new: https://git.openjdk.java.net/jdk/pull/1197/files/58651fc5..9f9164a5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1197=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1197=03-04 Stats: 31 lines in 2 files changed: 10 ins; 12 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/1197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1197/head:pull/1197 PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Wed, 2 Dec 2020 15:33:20 GMT, Jamil Nimeh wrote: >> SunEC's algorithm name for keys are always "EdDSA", but I know BC returns >> "Ed25519" or "Ed448". > > Filed and took ownership of JDK-8257607 to address BC JCE provider issues for > both XDH and EdDSA when used with SunJSSE. Also, specific to this particular algorithm the signature type can be EdDSA for both BC and SunJCE, regardless of if the key type is EdDSA (SunJCE) or Ed25519 or Ed448 (BC). Creating a signature with algorithm EdDSA and the use of the key from either provider will perform a signature of the proper kind. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 20:05:09 GMT, Weijun Wang wrote: >> src/java.base/share/classes/sun/security/ssl/JsseJce.java line 97: >> >>> 95: */ >>> 96: static final String SIGNATURE_EDDSA = "EdDSA"; >>> 97: >> >> Please update the copyright year. >> >> Is it possible that "ed25519" or "ed448" is used as the signature algorithm, >> especially in the X.509 certificate implementation? > > SunEC's algorithm name for keys are always "EdDSA", but I know BC returns > "Ed25519" or "Ed448". Filed and took ownership of JDK-8257607 to address BC JCE provider issues for both XDH and EdDSA when used with SunJSSE. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v4]
> Hello all, > This change brings in support for certificates with EdDSA keys (both Ed25519 > and Ed448) allowing those signature algorithms to be used both on the > certificates themselves and used during the handshaking process for messages > like CertificateVerify, ServerKeyExchange and so forth. Jamil Nimeh has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision: - Merge - Update test to account for JDK-8202343 fix - Merge - Merge - Applied code review comments to tests - Fix cut/paste error with ECDH-RSA key exchange - Merge - Initial EdDSA/TLS solution - Changes: - all: https://git.openjdk.java.net/jdk/pull/1197/files - new: https://git.openjdk.java.net/jdk/pull/1197/files/dee70944..58651fc5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1197=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1197=02-03 Stats: 192313 lines in 1163 files changed: 125674 ins; 47707 del; 18932 mod Patch: https://git.openjdk.java.net/jdk/pull/1197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1197/head:pull/1197 PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 20:22:33 GMT, Jamil Nimeh wrote:
>> src/java.base/share/classes/sun/security/ssl/CertificateRequest.java line
>> 139:
>>
>>> 137: if (cct.isAvailable) {
>>> 138: cct.keyAlgorithm.forEach(key -> {
>>> 139: if (!keyTypes.contains(key)) {
>>
>> Can this ever happen? Why not just `addAll`?
>
> I wanted to make sure we didn't end up with duplicates if two different
> certificate types had the same underlying key type and also to deal with both
> well-formed certificate request messages or ones that erroneously reiterate
> the type. It is definitely an edge case.
OK.
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 20:39:46 GMT, Xue-Lei Andrew Fan wrote: >> JsseJce.isEcAvailable doesn't check for EdDSA availability so I'm not sure >> we want that second clause. I don't think the EdDSA code is implemented in >> the same module as the other EC code is so I don't know if we'd want to >> extend isEcAvailable to include EdDSA. I'll need to go look to see where >> EdDSA is located relative to the EC code. > > Hm, maybe a new isEdDsaAvailable(). The reasons for having isEcAvailable for EC keys do not apply for EdDSA. I don't think we should make any changes to this part of the code right now. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 20:12:47 GMT, Jamil Nimeh wrote:
>> src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 73:
>>
>>> 71: ED448 (0x0808, "ed448", "Ed448",
>>> 72: "EdDSA",
>>> 73: ProtocolVersion.PROTOCOLS_12_13),
>>
>> You may also want to check if EdDSA is available in the following block:
>> - 282 if ("EC".equals(keyAlgorithm)) {
>> + 282 if ("EC".equals(keyAlgorithm) || "EdDSA"... ) {
>> 283 mediator = JsseJce.isEcAvailable();
>> 284 }
>
> JsseJce.isEcAvailable doesn't check for EdDSA availability so I'm not sure we
> want that second clause. I don't think the EdDSA code is implemented in the
> same module as the other EC code is so I don't know if we'd want to extend
> isEcAvailable to include EdDSA. I'll need to go look to see where EdDSA is
> located relative to the EC code.
Hm, maybe a new isEdDsaAvailable().
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 19:58:23 GMT, Weijun Wang wrote:
>> Jamil Nimeh has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - Update test to account for JDK-8202343 fix
>> - Merge
>> - Merge
>> - Applied code review comments to tests
>> - Fix cut/paste error with ECDH-RSA key exchange
>> - Merge
>> - Initial EdDSA/TLS solution
>
> src/java.base/share/classes/sun/security/ssl/CertificateRequest.java line 139:
>
>> 137: if (cct.isAvailable) {
>> 138: cct.keyAlgorithm.forEach(key -> {
>> 139: if (!keyTypes.contains(key)) {
>
> Can this ever happen? Why not just `addAll`?
I wanted to make sure we didn't end up with duplicates if two different
certificate types had the same underlying key type and also to deal with both
well-formed certificate request messages or ones that erroneously reiterate the
type. It is definitely an edge case.
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 18:37:36 GMT, Xue-Lei Andrew Fan wrote:
>> Jamil Nimeh has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - Update test to account for JDK-8202343 fix
>> - Merge
>> - Merge
>> - Applied code review comments to tests
>> - Fix cut/paste error with ECDH-RSA key exchange
>> - Merge
>> - Initial EdDSA/TLS solution
>
> src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 73:
>
>> 71: ED448 (0x0808, "ed448", "Ed448",
>> 72: "EdDSA",
>> 73: ProtocolVersion.PROTOCOLS_12_13),
>
> You may also want to check if EdDSA is available in the following block:
> - 282 if ("EC".equals(keyAlgorithm)) {
> + 282 if ("EC".equals(keyAlgorithm) || "EdDSA"... ) {
> 283 mediator = JsseJce.isEcAvailable();
> 284 }
JsseJce.isEcAvailable doesn't check for EdDSA availability so I'm not sure we
want that second clause. I don't think the EdDSA code is implemented in the
same module as the other EC code is so I don't know if we'd want to extend
isEcAvailable to include EdDSA. I'll need to go look to see where EdDSA is
located relative to the EC code.
> src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 48:
>
>> 46: if (authentication != null) {
>> 47: this.authentication = new ArrayList<>();
>> 48: this.authentication.addAll(authentication);
>
> I may use an immutable list, for example List.copyOf(authentication).
That's probably a good idea. It doesn't look like we're making changes to the
List outside of the constructor.
> src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 134:
>
>> 132: SSLAuthentication authType = li.next();
>> 133: auHandshakes =
>> authType.getRelatedHandshakers(handshakeContext);
>> 134: }
>
> I guess for-each loop could be a little bit more lightweight.
Yeah, I thought about that. I just like having all the loop termination
clauses in one place rather than testing in the loop body and breaking. I can
change it to for-each though, no strong feelings one way or the other.
> src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 163:
>
>> 161: SSLAuthentication authType = li.next();
>> 162: auProducers =
>> authType.getHandshakeProducers(handshakeContext);
>> 163: }
>
> Use for-each?
Will change to for-each.
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 17:31:20 GMT, Xue-Lei Andrew Fan wrote:
>> Jamil Nimeh has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - Update test to account for JDK-8202343 fix
>> - Merge
>> - Merge
>> - Applied code review comments to tests
>> - Fix cut/paste error with ECDH-RSA key exchange
>> - Merge
>> - Initial EdDSA/TLS solution
>
> src/java.base/share/classes/sun/security/ssl/CertificateRequest.java line 83:
>
>> 81: ECDSA_SIGN ((byte)0x40, "ecdsa_sign",
>> 82: List.of("EC", "EdDSA"),
>> 83: JsseJce.isEcAvailable()),
>
> Would you like to add RFC 8422 to the comment line as well?
I think that's a good idea.
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Fri, 20 Nov 2020 18:09:26 GMT, Xue-Lei Andrew Fan wrote: >> Jamil Nimeh has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains seven additional >> commits since the last revision: >> >> - Update test to account for JDK-8202343 fix >> - Merge >> - Merge >> - Applied code review comments to tests >> - Fix cut/paste error with ECDH-RSA key exchange >> - Merge >> - Initial EdDSA/TLS solution > > src/java.base/share/classes/sun/security/ssl/JsseJce.java line 97: > >> 95: */ >> 96: static final String SIGNATURE_EDDSA = "EdDSA"; >> 97: > > Please update the copyright year. > > Is it possible that "ed25519" or "ed448" is used as the signature algorithm, > especially in the X.509 certificate implementation? SunEC's algorithm name for keys are always "EdDSA", but I know BC returns "Ed25519" or "Ed448". - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Thu, 19 Nov 2020 17:48:34 GMT, Jamil Nimeh wrote:
>> Hello all,
>> This change brings in support for certificates with EdDSA keys (both Ed25519
>> and Ed448) allowing those signature algorithms to be used both on the
>> certificates themselves and used during the handshaking process for messages
>> like CertificateVerify, ServerKeyExchange and so forth.
>
> Jamil Nimeh has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains seven additional
> commits since the last revision:
>
> - Update test to account for JDK-8202343 fix
> - Merge
> - Merge
> - Applied code review comments to tests
> - Fix cut/paste error with ECDH-RSA key exchange
> - Merge
> - Initial EdDSA/TLS solution
src/java.base/share/classes/sun/security/ssl/CertificateRequest.java line 139:
> 137: if (cct.isAvailable) {
> 138: cct.keyAlgorithm.forEach(key -> {
> 139: if (!keyTypes.contains(key)) {
Can this ever happen? Why not just `addAll`?
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
On Thu, 19 Nov 2020 17:48:34 GMT, Jamil Nimeh wrote:
>> Hello all,
>> This change brings in support for certificates with EdDSA keys (both Ed25519
>> and Ed448) allowing those signature algorithms to be used both on the
>> certificates themselves and used during the handshaking process for messages
>> like CertificateVerify, ServerKeyExchange and so forth.
>
> Jamil Nimeh has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains seven additional
> commits since the last revision:
>
> - Update test to account for JDK-8202343 fix
> - Merge
> - Merge
> - Applied code review comments to tests
> - Fix cut/paste error with ECDH-RSA key exchange
> - Merge
> - Initial EdDSA/TLS solution
src/java.base/share/classes/sun/security/ssl/CertificateRequest.java line 83:
> 81: ECDSA_SIGN ((byte)0x40, "ecdsa_sign",
> 82: List.of("EC", "EdDSA"),
> 83: JsseJce.isEcAvailable()),
Would you like to add RFC 8422 to the comment line as well?
src/java.base/share/classes/sun/security/ssl/JsseJce.java line 97:
> 95: */
> 96: static final String SIGNATURE_EDDSA = "EdDSA";
> 97:
Please update the copyright year.
Is it possible that "ed25519" or "ed448" is used as the signature algorithm,
especially in the X.509 certificate implementation?
src/java.base/share/classes/sun/security/ssl/SignatureScheme.java line 73:
> 71: ED448 (0x0808, "ed448", "Ed448",
> 72: "EdDSA",
> 73: ProtocolVersion.PROTOCOLS_12_13),
You may also want to check if EdDSA is available in the following block:
- 282 if ("EC".equals(keyAlgorithm)) {
+ 282 if ("EC".equals(keyAlgorithm) || "EdDSA"... ) {
283 mediator = JsseJce.isEcAvailable();
284 }
src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 48:
> 46: if (authentication != null) {
> 47: this.authentication = new ArrayList<>();
> 48: this.authentication.addAll(authentication);
I may use an immutable list, for example List.copyOf(authentication).
src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 134:
> 132: SSLAuthentication authType = li.next();
> 133: auHandshakes =
> authType.getRelatedHandshakers(handshakeContext);
> 134: }
I guess for-each loop could be a little bit more lightweight.
src/java.base/share/classes/sun/security/ssl/SSLKeyExchange.java line 163:
> 161: SSLAuthentication authType = li.next();
> 162: auProducers =
> authType.getHandshakeProducers(handshakeContext);
> 163: }
Use for-each?
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v3]
> Hello all, > This change brings in support for certificates with EdDSA keys (both Ed25519 > and Ed448) allowing those signature algorithms to be used both on the > certificates themselves and used during the handshaking process for messages > like CertificateVerify, ServerKeyExchange and so forth. Jamil Nimeh has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Update test to account for JDK-8202343 fix - Merge - Merge - Applied code review comments to tests - Fix cut/paste error with ECDH-RSA key exchange - Merge - Initial EdDSA/TLS solution - Changes: - all: https://git.openjdk.java.net/jdk/pull/1197/files - new: https://git.openjdk.java.net/jdk/pull/1197/files/59aced35..dee70944 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1197=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1197=01-02 Stats: 6780 lines in 253 files changed: 3650 ins; 1976 del; 1154 mod Patch: https://git.openjdk.java.net/jdk/pull/1197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1197/head:pull/1197 PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm [v2]
> Hello all, > This change brings in support for certificates with EdDSA keys (both Ed25519 > and Ed448) allowing those signature algorithms to be used both on the > certificates themselves and used during the handshaking process for messages > like CertificateVerify, ServerKeyExchange and so forth. Jamil Nimeh has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - Merge - Applied code review comments to tests - Fix cut/paste error with ECDH-RSA key exchange - Merge - Initial EdDSA/TLS solution - Changes: - all: https://git.openjdk.java.net/jdk/pull/1197/files - new: https://git.openjdk.java.net/jdk/pull/1197/files/3396e6c2..59aced35 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1197=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1197=00-01 Stats: 19865 lines in 433 files changed: 12225 ins; 5055 del; 2585 mod Patch: https://git.openjdk.java.net/jdk/pull/1197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1197/head:pull/1197 PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Tue, 17 Nov 2020 23:31:13 GMT, Jamil Nimeh wrote: >> I don't think there's any reason why we could use a >> Collection for these. I'll try switching to that. > > Xuelei, I went back and looked at my rationale for using Strings here. The > reason I went with this approach was so I could have client and server > parameter maps of . I had a common form for parameters > that I'd want to set/reset/change between each type of test run. If I were > to go with something like a Collection then my client > and server parameter maps would end up needing to be and > then I'd have to cast based on the param type. Not sure if changing from the > Strings ends up being more clear int the long run. No problem, using String is ok to me. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Tue, 17 Nov 2020 19:43:25 GMT, Jamil Nimeh wrote: >> test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 81: >> >>> 79: static final String DEF_ALL_EE = >>> "EE_ECDSA_SECP256R1:EE_ECDSA_SECP384R1:" + >>> 80: "EE_ECDSA_SECP521R1:EE_RSA_2048:EE_EC_RSA_SECP256R1:" + >>> 81: "EE_DSA_2048:EE_DSA_1024:EE_ED25519:EE_ED448"; >> >> Why not use enum, array or collection directly? Which is easy to read, I >> think. > > I don't think there's any reason why we could use a > Collection for these. I'll try switching to that. Xuelei, I went back and looked at my rationale for using Strings here. The reason I went with this approach was so I could have client and server parameter maps of . I had a common form for parameters that I'd want to set/reset/change between each type of test run. If I were to go with something like a Collection then my client and server parameter maps would end up needing to be and then I'd have to cast based on the param type. Not sure if changing from the Strings ends up being more clear int the long run. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Tue, 17 Nov 2020 18:32:34 GMT, Xue-Lei Andrew Fan wrote: >> Hello all, >> This change brings in support for certificates with EdDSA keys (both Ed25519 >> and Ed448) allowing those signature algorithms to be used both on the >> certificates themselves and used during the handshaking process for messages >> like CertificateVerify, ServerKeyExchange and so forth. > > test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 92: > >> 90: final SessionChecker serverChecker; >> 91: final Class clientException; >> 92: final Class serverException; > > If no problem, I may declare the class fields and inner classes as private > for safe. OK. Will do. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Tue, 17 Nov 2020 19:47:37 GMT, Jamil Nimeh wrote: >> test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 583: >> >>> 581: serverParameters.put(ParamType.CERTALIAS, "EE_ED25519"); >>> 582: runtest(testFormat, isPeerEd25519, null, null, null); >>> 583: serverParameters.remove(ParamType.CERTALIAS); >> >> I did not get the idea here. Is there a special case in practice that use a >> similar key manger like the AliasKeyManager? > > Right now, for TLS 1.0/1.1 EC certificates will be favored over EdDSA > certificates in keystores that have valid certificates with both kinds of > keys. There's nothing we can do about that because 1.0/1.1 has no signaling > mechanism to indicate signature preference like 1.2+ has. Given that, I was > thinking of ways to get around that restriction and one case I thought of was > the Tomcat connector, which has options to specify a certificate for use by > alias. I wanted to make sure that we could still do that for 1.0/1.1 and it > wouldn't break so I cooked up this simple KeyManager and ran a basic > connection, expecting to see the cert specified by the alias. Got it. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Tue, 17 Nov 2020 19:07:33 GMT, Xue-Lei Andrew Fan wrote: >> Hello all, >> This change brings in support for certificates with EdDSA keys (both Ed25519 >> and Ed448) allowing those signature algorithms to be used both on the >> certificates themselves and used during the handshaking process for messages >> like CertificateVerify, ServerKeyExchange and so forth. > > test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 583: > >> 581: serverParameters.put(ParamType.CERTALIAS, "EE_ED25519"); >> 582: runtest(testFormat, isPeerEd25519, null, null, null); >> 583: serverParameters.remove(ParamType.CERTALIAS); > > I did not get the idea here. Is there a special case in practice that use a > similar key manger like the AliasKeyManager? Right now, for TLS 1.0/1.1 EC certificates will be favored over EdDSA certificates in keystores that have valid certificates with both kinds of keys. There's nothing we can do about that because 1.0/1.1 has no signaling mechanism to indicate signature preference like 1.2+ has. Given that, I was thinking of ways to get around that restriction and one case I thought of was the Tomcat connector, which has options to specify a certificate for use by alias. I wanted to make sure that we could still do that for 1.0/1.1 and it wouldn't break so I cooked up this simple KeyManager and ran a basic connection, expecting to see the cert specified by the alias. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Tue, 17 Nov 2020 18:29:13 GMT, Xue-Lei Andrew Fan wrote: >> Hello all, >> This change brings in support for certificates with EdDSA keys (both Ed25519 >> and Ed448) allowing those signature algorithms to be used both on the >> certificates themselves and used during the handshaking process for messages >> like CertificateVerify, ServerKeyExchange and so forth. > > test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 81: > >> 79: static final String DEF_ALL_EE = >> "EE_ECDSA_SECP256R1:EE_ECDSA_SECP384R1:" + >> 80: "EE_ECDSA_SECP521R1:EE_RSA_2048:EE_EC_RSA_SECP256R1:" + >> 81: "EE_DSA_2048:EE_DSA_1024:EE_ED25519:EE_ED448"; > > Why not use enum, array or collection directly? Which is easy to read, I > think. I don't think there's any reason why we could use a Collection for these. I'll try switching to that. > test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 592: > >> 590: } >> 591: >> 592: private static void keyManagerTests(String keyStoreSpec, String >> keyType, > > Java method name is normally an action. What do you think if update to > testKeyManager()? Sure, easy enough to do. - PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Tue, 17 Nov 2020 18:24:35 GMT, Xue-Lei Andrew Fan wrote:
>> Hello all,
>> This change brings in support for certificates with EdDSA keys (both Ed25519
>> and Ed448) allowing those signature algorithms to be used both on the
>> certificates themselves and used during the handshaking process for messages
>> like CertificateVerify, ServerKeyExchange and so forth.
>
> test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 35:
>
>> 33: * SunJSSE does not support dynamic system properties, no way to
>> re-use
>> 34: * system properties in samevm/agentvm mode.
>> 35: */
>
> Leading white spaces could be removed. I may put this comment before the
> "@test" block to be consistent with other test cases.
No problem, I'll take care of that.
> test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 74:
>
>> 72:
>> 73: public class TLSWithEdDSA extends SSLSocketTemplate {
>> 74: static final boolean DEBUG = false;
>
> I may not use this filed. The debug property could be specified in the
> "@run" tag if needed.
No problem. Will change.
-
PR: https://git.openjdk.java.net/jdk/pull/1197
Re: RFR: JDK-8166596: TLS support for the EdDSA signature algorithm
On Fri, 13 Nov 2020 04:57:12 GMT, Jamil Nimeh wrote:
> Hello all,
> This change brings in support for certificates with EdDSA keys (both Ed25519
> and Ed448) allowing those signature algorithms to be used both on the
> certificates themselves and used during the handshaking process for messages
> like CertificateVerify, ServerKeyExchange and so forth.
test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 35:
> 33: * SunJSSE does not support dynamic system properties, no way to
> re-use
> 34: * system properties in samevm/agentvm mode.
> 35: */
Leading white spaces could be removed. I may put this comment before the
"@test" block to be consistent with other test cases.
test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 74:
> 72:
> 73: public class TLSWithEdDSA extends SSLSocketTemplate {
> 74: static final boolean DEBUG = false;
I may not use this filed. The debug property could be specified in the "@run"
tag if needed.
test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 81:
> 79: static final String DEF_ALL_EE =
> "EE_ECDSA_SECP256R1:EE_ECDSA_SECP384R1:" +
> 80: "EE_ECDSA_SECP521R1:EE_RSA_2048:EE_EC_RSA_SECP256R1:" +
> 81: "EE_DSA_2048:EE_DSA_1024:EE_ED25519:EE_ED448";
Why not use enum, array or collection directly? Which is easy to read, I think.
test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 92:
> 90: final SessionChecker serverChecker;
> 91: final Class clientException;
> 92: final Class serverException;
If no problem, I may declare the class fields and inner classes as private for
safe.
test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 592:
> 590: }
> 591:
> 592: private static void keyManagerTests(String keyStoreSpec, String
> keyType,
Java method name is normally an action. What do you think if update to
testKeyManager()?
test/jdk/javax/net/ssl/TLSCommon/TLSWithEdDSA.java line 583:
> 581: serverParameters.put(ParamType.CERTALIAS, "EE_ED25519");
> 582: runtest(testFormat, isPeerEd25519, null, null, null);
> 583: serverParameters.remove(ParamType.CERTALIAS);
I did not get the idea here. Is there a special case in practice that use a
similar key manger like the AliasKeyManager?
-
PR: https://git.openjdk.java.net/jdk/pull/1197
