Re: 8245686: Ed25519 and Ed448 present in handshake messages

2020-06-09 Thread Anthony Scarpino
Thanks for the comment.. I moved the code up toward the top. Tony On 6/9/20 4:04 PM, Xuelei Fan wrote: A simple fix like this looks good to me.  I may check this first, before the EC available and signature checking. Xuelei On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code

Re: 8245686: Ed25519 and Ed448 present in handshake messages

2020-06-09 Thread Anthony Scarpino
Thanks for catching that. Tony On 6/9/20 5:23 PM, Bradford Wetmore wrote: Update the year, but otherwise looks good. Brad On 6/9/2020 4:04 PM, Xuelei Fan wrote: A simple fix like this looks good to me.  I may check this first, before the EC available and signature checking. Xuelei On

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

2020-06-09 Thread Weijun Wang
Good to see both -keystore and -trustcacerts mentioned. Some comments: 1. I think there is no need to say "from -file cert_file". Or, do you mean the new function does not apply to those from -sslserver and -jarfile? If so, that might be a problem. 2. While you said "attempts to construct a

Re: 8245686: Ed25519 and Ed448 present in handshake messages

2020-06-09 Thread Bradford Wetmore
Update the year, but otherwise looks good. Brad On 6/9/2020 4:04 PM, Xuelei Fan wrote: A simple fix like this looks good to me.  I may check this first, before the EC available and signature checking. Xuelei On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this

Re: [15] RFR JDK-8246613: Choose the default SecureRandom algo based on registration ordering

2020-06-09 Thread Valerie Peng
Hi, Sean, On 6/9/2020 12:21 PM, Sean Mullan wrote: Looks good, just a couple of minor comments on the test: * test/jdk/java/security/SecureRandom/DefaultAlgo.java 75 Objects.requireNonNull(p); Not sure why you need this line, since the test never passes null. True, the test never

Re: 8245686: Ed25519 and Ed448 present in handshake messages

2020-06-09 Thread Xuelei Fan
A simple fix like this looks good to me. I may check this first, before the EC available and signature checking. Xuelei On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The

Re: [RFR] 8241680: KeyPairGen & Signature microbenchmarks need updating for disabled EC curves

2020-06-09 Thread Jamil Nimeh
+1 --Jamil On 6/8/2020 12:22 PM, Sergey Kuksenko wrote: Looks fine to me. On 6/8/20 11:15 AM, Anthony Scarpino wrote: Hi, I need a quick code review of updates to the microbenchmarks tests for EC.  These tests used curves that are now disabled by default in 15.

Re: Thread leak by LdapLoginModule

2020-06-09 Thread Sean Mullan
Adding core-libs-dev ... --Sean On 6/9/20 5:15 PM, Mkrtchyan, Tigran wrote: Hi all, with Java-11 we have notice a thread leak with ldap module. We use LDAP to authenticate users with username+pasword by directly calling LdapLoginModule. This was ok with java 7 and java 8. With java 11 we see

Re: 8245686: Ed25519 and Ed448 present in handshake messages

2020-06-09 Thread Jamil Nimeh
Looks fine to me. --Jamil On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there

8245686: Ed25519 and Ed448 present in handshake messages

2020-06-09 Thread Anthony Scarpino
Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there was an EdDSA JCE implementation or JSSE support. Without an

Thread leak by LdapLoginModule

2020-06-09 Thread Mkrtchyan, Tigran
Hi all, with Java-11 we have notice a thread leak with ldap module. We use LDAP to authenticate users with username+pasword by directly calling LdapLoginModule. This was ok with java 7 and java 8. With java 11 we see threads getting accumulated. here is a test case that demonstrates it: ```

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-06-09 Thread Alexey Bakhtin
Hello Aleks, Thank you very much for review. I’ve fixed missed spaces and removed casting from LdapSasl.java Failure of the SaslMutual test was caused by prop.remove() in the GssKrb5Client This operation is not required any more. GssKrb5Client receives temporary copy of the properties. Fixed

Re: [15] RFR JDK-8246613: Choose the default SecureRandom algo based on registration ordering

2020-06-09 Thread Sean Mullan
Looks good, just a couple of minor comments on the test: * test/jdk/java/security/SecureRandom/DefaultAlgo.java 75 Objects.requireNonNull(p); Not sure why you need this line, since the test never passes null. 90 validate(new SecureRandom(), pName, algos[0]); Is there

Re: [15] RFR JDK-8246613: Choose the default SecureRandom algo based on registration ordering

2020-06-09 Thread Valerie Peng
Thanks for review~ As for the isProviderInfo() name, since I reverted the code for its impl to pre-7092821, I changed it back to the old name as you noticed. Sean mentioned that he also wants to take a look at this updated webrev, so I will wait for him to do that... Valerie On 6/8/2020

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-06-09 Thread Sean Mullan
On 6/9/20 12:40 PM, Xuelei Fan wrote: About the prefix, it may follow RFC 5056 (See page 7, section 2.1).    o  Specifications of channel bindings for any secure channels MUST   provide for a single, canonical octet string encoding of the   channel bindings.  Under this framework,

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-06-09 Thread Xuelei Fan
About the prefix, it may follow RFC 5056 (See page 7, section 2.1). o Specifications of channel bindings for any secure channels MUST provide for a single, canonical octet string encoding of the channel bindings. Under this framework, channel bindings MUST start with the

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-06-09 Thread Alexey Bakhtin
Hello Sean, Thank you for the link. I’ll follow it to create CSR I could not find any clear document or specification for this Channel Binding format. The only document I found that describes this format is the following:

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-06-09 Thread Sean Mullan
On 6/8/20 5:33 PM, Alexey Bakhtin wrote: Hello Sean, Yes, I think we'll need CSR and release notes as soon as this patch adds new property. I do not know exact process for it, so I will be grateful if you could explain me exact steps. The CSR process is documented at

Re: RFR 8244148: keytool -printcert and -printcrl should support the -trustcacerts and -keystore options

2020-06-09 Thread Hai-May Chao
> On Jun 7, 2020, at 6:08 PM, Weijun Wang wrote: > > Looks fine to me. > > For CSR, since there is already a "Note" there for these 2 options, you can > add a few words about what -keystore and -trustcacerts can do. Updated CSR as suggested. Thanks, Hai-May > > Thanks, > Max > >> On

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-06-09 Thread Aleks Efimov
Hi Alexey, Thank you for incorporating LdapCtx and LdapSasl changes. I've reviewed both classes and they look good to me, with few minor comments in LdapSasl.java:   missing spaces in the following lines: 78, 152   With your last changes we can remove explicit cast of 'envProps' on