Re: RFR [16] [JDK-8248745] Add jarsigner and keytool tests for restricted algorithms
Thanks Sean for the review. Addressed your comments on new webrev - http://cr.openjdk.java.net/~akolarkunnu/8248745/webrev.02/ -Muneer On 06/08/20 8:00 pm, Sean Mullan wrote: You should also add a test for MD2 and MD5 for the jarsigner -digestalg option. 125 private static void testJarsignerSiginig(String sigAlg, String alias) typo: s/Siginig/Signing/ All else looks fine. --Sean On 8/4/20 11:13 PM, Hai-May Chao wrote: Hi Muneer, Updated webrev looks good. Thanks, Hai-May On Aug 4, 2020, at 7:26 PM, abdul.kolarku...@oracle.com wrote: Thanks Hai-May for review. Updated the webrev with your comment -http://cr.openjdk.java.net/~akolarkunnu/8248745/webrev.01/ -Muneer On 04/08/20 11:58 pm, Hai-May Chao wrote: Hi Muneer, Looks good with one minor comment. #58: suggest that the SECURITY_WARNING will also include “and is disabled” at the end to make it clear. Thanks, Hai-May On Jul 27, 2020, at 9:15 AM, abdul.kolarku...@oracle.com wrote: Hi All, This is a new test int the area of jarsigner and keytool for the restricted/disabled algorithms. Bug Id - https://bugs.openjdk.java.net/browse/JDK-8248745 Webrev - http://cr.openjdk.java.net/~akolarkunnu/8248745/webrev.00/ Description: Adding a test for key generation, jar signing and verification with all disabled algorithms and key sizes which are in the property jdk.jar.disabledAlgorithms. Covered the scenario of with and without these disabled entries in jdk.jar.disabledAlgorithms. Whenever the entries are in the property jdk.jar.disabledAlgorithms, corresponding warning or error message should shown, otherwise everything should work fine without any related error or warning. This test covers all entries listed in "jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024, include jdk.disabled.namedCurves". In case of disabled curves, this test covers only one curve secp112r1. Tested in Linux, Windows and Mac Osx platforms and all are working fine. -Muneer
"Blocking operation" during SSLEngineImpl.unwrap()
Hi there, In netty we support using BlockHound[1] to detect if people do blocking operations within the EventLoop and so notify them that this should not be done. While running our integration tests with TLS1.3 we noticed that unwrap(…) may trigger an FileInputStream.read(…) which in theory could block for a long time. I was assuming that such an operation should only be done after SSLEngine.* returns NEED_TASK and so be delegated to another ThreadPool via getTask(). Now the question(s): * Is my assumption incorrect ? * If my assumption is correct should we fix this ? Here is the stack trace when such a blocking call is detected: reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes at java.base/java.io.FileInputStream.readBytes(FileInputStream.java) at java.base/java.io.FileInputStream.read(FileInputStream.java:273) at java.base/java.io.FilterInputStream.read(FilterInputStream.java:133) at java.base/sun.security.provider.NativePRNG$RandomIO.readFully(NativePRNG.java:424) at java.base/sun.security.provider.NativePRNG$RandomIO.ensureBufferValid(NativePRNG.java:526) at java.base/sun.security.provider.NativePRNG$RandomIO.implNextBytes(NativePRNG.java:545) at java.base/sun.security.provider.NativePRNG$NonBlocking.engineNextBytes(NativePRNG.java:318) at java.base/java.security.SecureRandom.nextBytes(SecureRandom.java:741) at java.base/sun.security.ssl.RandomCookie.(RandomCookie.java:67) at java.base/sun.security.ssl.SessionId.(SessionId.java:45) at java.base/sun.security.ssl.NewSessionTicket$NewSessionTicketKickstartProducer.produce(NewSessionTicket.java:225) at java.base/sun.security.ssl.Finished$T13FinishedConsumer.onConsumeFinished(Finished.java:1100) at java.base/sun.security.ssl.Finished$T13FinishedConsumer.consume(Finished.java:867) at java.base/sun.security.ssl.SSLHandshake.consume(SSLHandshake.java:392) at java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:443) at java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:418) at java.base/sun.security.ssl.TransportContext.dispatch(TransportContext.java:177) at java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:164) at java.base/sun.security.ssl.SSLEngineImpl.decode(SSLEngineImpl.java:681) at java.base/sun.security.ssl.SSLEngineImpl.readRecord(SSLEngineImpl.java:636) at java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:454) at java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:433) at java.base/javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:634) at io.netty.handler.ssl.SslHandler$SslEngineType$3.unwrap(SslHandler.java:282) at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1380) at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1275) at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1322) [1] https://github.com/reactor/BlockHound
RFR (16): 8241003: Deprecate "denigrated" java.security.cert APIs that represent DNs as Principal or String objects
Please review this change to deprecate the following APIs: java.security.cert.X509Certificate.getIssuerDN() java.security.cert.X509Certificate.getSubjectDN() java.security.cert.X509CRL.getIssuerDN() java.security.cert.X509CertSelector.setIssuer(String) java.security.cert.X509CertSelector.setSubject(String) java.security.cert.X509CertSelector.getIssuerAsString() java.security.cert.X509CertSelector.getSubjectAsString() java.security.cert.X509CRLSelector.addIssuerName(String) These APIs either take or return Distinguished Names as Principal or String objects which can cause issues due to loss of encoding information or differences when comparing names across different Principal implementations. All of them have alternative APIs which use X500Principal objects instead. They have long had warnings in the javadoc and have been discouraged from being used. There are no plans to remove the APIs at this time, as they have been in the platform for a long time and removing them would be a much higher compatibility risk. webrev: https://cr.openjdk.java.net/~mullan/webrevs/8241003/webrev.00/ CSR: https://bugs.openjdk.java.net/browse/JDK-8250970 bug: https://bugs.openjdk.java.net/browse/JDK-8241003 --Sean
Re: "Blocking operation" during SSLEngineImpl.unwrap()
Hm, it's an interesting bug. I filed the issue on the Java Bug System. https://bugs.openjdk.java.net/browse/JDK-8251304 Thanks, Xuelei On 8/7/2020 5:00 AM, Norman Maurer wrote: Hi there, In netty we support using BlockHound[1] to detect if people do blocking operations within the EventLoop and so notify them that this should not be done. While running our integration tests with TLS1.3 we noticed that unwrap(…) may trigger an FileInputStream.read(…) which in theory could block for a long time. I was assuming that such an operation should only be done after SSLEngine.* returns NEED_TASK and so be delegated to another ThreadPool via getTask(). Now the question(s): * Is my assumption incorrect ? * If my assumption is correct should we fix this ? Here is the stack trace when such a blocking call is detected: reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes at java.base/java.io.FileInputStream.readBytes(FileInputStream.java) at java.base/java.io.FileInputStream.read(FileInputStream.java:273) at java.base/java.io.FilterInputStream.read(FilterInputStream.java:133) at java.base/sun.security.provider.NativePRNG$RandomIO.readFully(NativePRNG.java:424) at java.base/sun.security.provider.NativePRNG$RandomIO.ensureBufferValid(NativePRNG.java:526) at java.base/sun.security.provider.NativePRNG$RandomIO.implNextBytes(NativePRNG.java:545) at java.base/sun.security.provider.NativePRNG$NonBlocking.engineNextBytes(NativePRNG.java:318) at java.base/java.security.SecureRandom.nextBytes(SecureRandom.java:741) at java.base/sun.security.ssl.RandomCookie.(RandomCookie.java:67) at java.base/sun.security.ssl.SessionId.(SessionId.java:45) at java.base/sun.security.ssl.NewSessionTicket$NewSessionTicketKickstartProducer.produce(NewSessionTicket.java:225) at java.base/sun.security.ssl.Finished$T13FinishedConsumer.onConsumeFinished(Finished.java:1100) at java.base/sun.security.ssl.Finished$T13FinishedConsumer.consume(Finished.java:867) at java.base/sun.security.ssl.SSLHandshake.consume(SSLHandshake.java:392) at java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:443) at java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:418) at java.base/sun.security.ssl.TransportContext.dispatch(TransportContext.java:177) at java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:164) at java.base/sun.security.ssl.SSLEngineImpl.decode(SSLEngineImpl.java:681) at java.base/sun.security.ssl.SSLEngineImpl.readRecord(SSLEngineImpl.java:636) at java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:454) at java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:433) at java.base/javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:634) at io.netty.handler.ssl.SslHandler$SslEngineType$3.unwrap(SslHandler.java:282) at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1380) at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1275) at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1322) [1] https://github.com/reactor/BlockHound
Re: "Blocking operation" during SSLEngineImpl.unwrap()
Thanks a lot… So seems like my assumption is correct then :) Bye Norman > On 7. Aug 2020, at 17:00, Xuelei Fan wrote: > > Hm, it's an interesting bug. I filed the issue on the Java Bug System. > https://bugs.openjdk.java.net/browse/JDK-8251304 > > Thanks, > Xuelei > > On 8/7/2020 5:00 AM, Norman Maurer wrote: >> Hi there, >> In netty we support using BlockHound[1] to detect if people do blocking >> operations within the EventLoop and so notify them that this should not be >> done. While running our integration tests with TLS1.3 we noticed that >> unwrap(…) may trigger an FileInputStream.read(…) which in theory could block >> for a long time. I was assuming that such an operation should only be done >> after SSLEngine.* returns NEED_TASK and so be delegated to another >> ThreadPool via getTask(). >> Now the question(s): >> * Is my assumption incorrect ? >> * If my assumption is correct should we fix this ? >> Here is the stack trace when such a blocking call is detected: >> reactor.blockhound.BlockingOperationError: Blocking call! >> java.io.FileInputStream#readBytes >> at java.base/java.io.FileInputStream.readBytes(FileInputStream.java) >> at java.base/java.io.FileInputStream.read(FileInputStream.java:273) >> at java.base/java.io.FilterInputStream.read(FilterInputStream.java:133) >> at >> java.base/sun.security.provider.NativePRNG$RandomIO.readFully(NativePRNG.java:424) >> at >> java.base/sun.security.provider.NativePRNG$RandomIO.ensureBufferValid(NativePRNG.java:526) >> at >> java.base/sun.security.provider.NativePRNG$RandomIO.implNextBytes(NativePRNG.java:545) >> at >> java.base/sun.security.provider.NativePRNG$NonBlocking.engineNextBytes(NativePRNG.java:318) >> at java.base/java.security.SecureRandom.nextBytes(SecureRandom.java:741) >> at java.base/sun.security.ssl.RandomCookie.(RandomCookie.java:67) >> at java.base/sun.security.ssl.SessionId.(SessionId.java:45) >> at >> java.base/sun.security.ssl.NewSessionTicket$NewSessionTicketKickstartProducer.produce(NewSessionTicket.java:225) >> at >> java.base/sun.security.ssl.Finished$T13FinishedConsumer.onConsumeFinished(Finished.java:1100) >> at >> java.base/sun.security.ssl.Finished$T13FinishedConsumer.consume(Finished.java:867) >> at java.base/sun.security.ssl.SSLHandshake.consume(SSLHandshake.java:392) >> at >> java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:443) >> at >> java.base/sun.security.ssl.HandshakeContext.dispatch(HandshakeContext.java:418) >> at >> java.base/sun.security.ssl.TransportContext.dispatch(TransportContext.java:177) >> at java.base/sun.security.ssl.SSLTransport.decode(SSLTransport.java:164) >> at java.base/sun.security.ssl.SSLEngineImpl.decode(SSLEngineImpl.java:681) >> at >> java.base/sun.security.ssl.SSLEngineImpl.readRecord(SSLEngineImpl.java:636) >> at java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:454) >> at java.base/sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:433) >> at java.base/javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:634) >> at >> io.netty.handler.ssl.SslHandler$SslEngineType$3.unwrap(SslHandler.java:282) >> at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1380) >> at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1275) >> at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1322) >> [1] https://github.com/reactor/BlockHound
Re: "Blocking operation" during SSLEngineImpl.unwrap()
On 07/08/2020 16:00, Xuelei Fan wrote: Hm, it's an interesting bug. I filed the issue on the Java Bug System. https://bugs.openjdk.java.net/browse/JDK-8251304 It is a bug that a new random cookie is needed or it has from read from /dev/urandom? I don't think the stack trace is enough to know if read is really blocked. -Alan
Re: "Blocking operation" during SSLEngineImpl.unwrap()
I think the possibility that it may block should be enough to signal and so offload to a task. If it never blocks then it’s not a bug... that’s why I asked the question in the first place . Bye Norman > Am 07.08.2020 um 18:13 schrieb Alan Bateman : > > On 07/08/2020 16:00, Xuelei Fan wrote: >> Hm, it's an interesting bug. I filed the issue on the Java Bug System. >>https://bugs.openjdk.java.net/browse/JDK-8251304 > It is a bug that a new random cookie is needed or it has from read from > /dev/urandom? I don't think the stack trace is enough to know if read is > really blocked. > > -Alan
Re: RFR [16] [JDK-8248745] Add jarsigner and keytool tests for restricted algorithms
Looks good. --Sean On 8/7/20 5:04 AM, abdul.kolarku...@oracle.com wrote: Thanks Sean for the review. Addressed your comments on new webrev - http://cr.openjdk.java.net/~akolarkunnu/8248745/webrev.02/ -Muneer On 06/08/20 8:00 pm, Sean Mullan wrote: You should also add a test for MD2 and MD5 for the jarsigner -digestalg option. 125 private static void testJarsignerSiginig(String sigAlg, String alias) typo: s/Siginig/Signing/ All else looks fine. --Sean On 8/4/20 11:13 PM, Hai-May Chao wrote: Hi Muneer, Updated webrev looks good. Thanks, Hai-May On Aug 4, 2020, at 7:26 PM, abdul.kolarku...@oracle.com wrote: Thanks Hai-May for review. Updated the webrev with your comment -http://cr.openjdk.java.net/~akolarkunnu/8248745/webrev.01/ -Muneer On 04/08/20 11:58 pm, Hai-May Chao wrote: Hi Muneer, Looks good with one minor comment. #58: suggest that the SECURITY_WARNING will also include “and is disabled” at the end to make it clear. Thanks, Hai-May On Jul 27, 2020, at 9:15 AM, abdul.kolarku...@oracle.com wrote: Hi All, This is a new test int the area of jarsigner and keytool for the restricted/disabled algorithms. Bug Id - https://bugs.openjdk.java.net/browse/JDK-8248745 Webrev - http://cr.openjdk.java.net/~akolarkunnu/8248745/webrev.00/ Description: Adding a test for key generation, jar signing and verification with all disabled algorithms and key sizes which are in the property jdk.jar.disabledAlgorithms. Covered the scenario of with and without these disabled entries in jdk.jar.disabledAlgorithms. Whenever the entries are in the property jdk.jar.disabledAlgorithms, corresponding warning or error message should shown, otherwise everything should work fine without any related error or warning. This test covers all entries listed in "jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024, include jdk.disabled.namedCurves". In case of disabled curves, this test covers only one curve secp112r1. Tested in Linux, Windows and Mac Osx platforms and all are working fine. -Muneer
Re: "Blocking operation" during SSLEngineImpl.unwrap()
Well if there were a bug it's with NativePRNG as the operation is suppose to be non-blocking. Even so, /dev/urandom is nonblocking. The only reason this looks to have been detected by the tool is because it's a blocking read op. This all seems like an extremely unlikely situation. I don't see this as something SSLEngine should be compensating for. Tony On 8/7/20 9:16 AM, Norman Maurer wrote: I think the possibility that it may block should be enough to signal and so offload to a task. If it never blocks then it’s not a bug... that’s why I asked the question in the first place . Bye Norman Am 07.08.2020 um 18:13 schrieb Alan Bateman : On 07/08/2020 16:00, Xuelei Fan wrote: Hm, it's an interesting bug. I filed the issue on the Java Bug System. https://bugs.openjdk.java.net/browse/JDK-8251304 It is a bug that a new random cookie is needed or it has from read from /dev/urandom? I don't think the stack trace is enough to know if read is really blocked. -Alan
Re: "Blocking operation" during SSLEngineImpl.unwrap()
On 07/08/2020 18:27, Anthony Scarpino wrote: Well if there were a bug it's with NativePRNG as the operation is suppose to be non-blocking. Even so, /dev/urandom is nonblocking. The only reason this looks to have been detected by the tool is because it's a blocking read op. This all seems like an extremely unlikely situation. I don't see this as something SSLEngine should be compensating for. Right, /dev/random is blocking, /dev/urandom is non-blocking. I just checked BlockHound and it seems to have the names of private methods in the java.io and java.net classes and I think instruments these methods on the assumption that they are blocking calls. The list seems to have been generated from an older JDK too, not really in sync with release JDK releases. So not clear to me that there is really an issue here. -Alan
Re: RFR 8251117: Cannot check P11Key size in P11Cipher and P11AEADCipher
Sure, looks fine to me as well. Thanks, Valerie On 8/4/2020 2:03 PM, Martin Balao wrote: Hi, I'd like to propose a fix for 8251117 [1], on behalf of Zdenek Zambersky (Red Hat employee - OCA signed). Webrev.00: * http://cr.openjdk.java.net/~mbalao/webrevs/8251117/8251117.webrev.00/ As noted in the ticket [1], the fix is about using P11Key::length method for retrieving P11Key sizes when initializing P11Cipher and P11AEADCipher instances. By doing that, we avoid NullPointerExceptions that happens when the P11Key is CKA_SENSITIVE and cannot be extracted in plain (this is the case for NSS software token keys configured in FIPS mode). I found no regressions in sun/security/pkcs11 tests. I've also done manual testing in my NSS-FIPS environment. Thanks, Martin.- -- [1] - https://bugs.openjdk.java.net/browse/JDK-8251117
