Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for IBMJSSE2 into grpc-java #7422

Merged
merged 8 commits into from Sep 16, 2020
Merged

Add support for IBMJSSE2 into grpc-java #7422

merged 8 commits into from Sep 16, 2020

Conversation

kiwi1969
Copy link
Contributor

@kiwi1969 kiwi1969 commented Sep 14, 2020

I made this very simple change to test for IBMJSSE2 security provider in addition to the others.
IBM JRE does not support the Sun provider, but instead has IBMJSSE2 which supports the same API calls.
I tested this on Z/OS machine as now it works when before it couldn't find a security provider

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 14, 2020

CLA Check
The committers are authorized under a signed CLA.

@@ -186,7 +187,8 @@ public static SslContextBuilder configure(SslContextBuilder builder, SslProvider
@CanIgnoreReturnValue
public static SslContextBuilder configure(SslContextBuilder builder, Provider jdkProvider) {
ApplicationProtocolConfig apc;
if (SUN_PROVIDER_NAME.equals(jdkProvider.getName())) {
if (SUN_PROVIDER_NAME.equals(jdkProvider.getName())
|| IBM_PROVIDER_NAME.equals(jdkProvider.getName())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the IBM provider supports Jetty ALPN/NPN, so this should be its own separate condition checking for isJava9AlpnAvailable. Ditto for findJdkProvider

Copy link
Contributor Author

@kiwi1969 kiwi1969 Sep 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is true JETTY ALPN doesn't work on IBM JDK, and does nothing either on Sun JDK if recent version is being used.
ie Jetty ALPN is only needed for older releases of Java8 which don't contain ALPN.

The IBM JRE has supported ALPN natively since 8.05.15 (build date 20180305).
Since IBM JRE 8.06.11 (June 2020) their ALPN implementation was changed to be the standard back-ported ALPN implementation as OpenJDK9.
This is the same release as I have

java version "1.8.0_251"
Java(TM) SE Runtime Environment (build 8.0.6.11 - pmz6480sr6fp11-20200602_01(SR6 FP11))
IBM J9 VM (build 2.9, JRE 1.8.0 z/OS s390x-64-Bit Compressed References 20200601_448156 (JIT enabled, AOT enabled)
OpenJ9   - 6c940fd
OMR      - 91c4c01
IBM      - 19f2e5e)

Now, what I had eagerly been waiiting for, was GRPC to be rebuilt with the version of Netty, that actually supported the ALPN implementation in the newer Java8 JDKs . This build dependency is mentioned in your 'current issues list'
Last week grpc-netty was actually upgraded to such a version.

I made the change mentioned in my pull request, and now everything works for me.
I did have to use this parm, else get TLSv1 errors "-Dcom.ibm.jsse2.overrideDefaultTLS=true"
On non-IBM JRE, this define with be different, and may not be needed

11:42:23.798 [main] INFO  com.equifax.usis.acro.Main - Running TestPubSub.java...                                                                               
11:42:25.985 [main] INFO  com.equifax.usis.acro.PubSub - Submitted request#1 for Async PubSub, Returning to caller                                              
11:42:25.987 [main] INFO  com.equifax.usis.acro.PubSub - Completed 0/1 Publish requests                                                                         
11:42:25.987 [main] INFO  com.equifax.usis.acro.PubSub - Waiting 3 seconds for outstanding PubSub replies to arrive                                             
11:42:26.209 [grpc-default-executor-0] WARN  i.g.n.shaded.io.netty.util.NetUtil - Failed to find the loopback interface                                         
11:42:26.213 [grpc-default-executor-0] WARN  i.g.n.s.i.n.u.i.MacAddressUtil - Failed to find a usable hardware address from the network interfaces; using random
 bytes: 99:ed:5c:8e:f7:a1:b3:29                                                                                                                                 
11:42:28.987 [main] INFO  com.equifax.usis.acro.PubSub - Number of Pubsub Publish replies outstanding = 0                                                        
11:42:28.987 [main] INFO  com.equifax.usis.acro.PubSub - Completed 1/1 Publish requests                                                                         
11:42:28.988 [main] INFO  com.equifax.usis.acro.PubSub - Requesting Shutdown...                                                                                 
11:42:29.057 [main] INFO  com.equifax.usis.acro.PubSub - Shutdown complete - Returning to caller                                                                
11:42:29.058 [main] INFO  com.equifax.usis.acro.Main - Exiting TestPubSub.java... 

I do not know if "TLSv1" is actually being explicitly requested in the code somewhere, but either way, that parm transforms all TSLv1 requests into the newest supported TLS version (TLSv1.2 in my case)

So all that is needed now is my code to be merged, so we can obtain an official release.

As mentioned above, I had initial failure due to TLSv1 being blocked on your system, which I found in the debug trace.
In the same captured trace I see that IBMSSE2 is definitely being used:

SSLContextImpl:  Using X509TrustManager com.ibm.jsse2.aD
JsseJCE:  Using SecureRandom SHA2DRBG from provider IBMJCE version 1.8
trigger seeding of SecureRandom
done seeding SecureRandom
11:17:49.899 [main] INFO  com.equifax.usis.acro.PubSub - Submitted request#1 for Async PubSub, Returning to caller
11:17:49.900 [main] INFO  com.equifax.usis.acro.PubSub - Completed 0/1 Publish requests
11:17:49.900 [main] INFO  com.equifax.usis.acro.PubSub - Waiting 3 seconds for outstanding PubSub replies to arrive
11:17:50.060 [grpc-default-executor-0] WARN  i.g.n.shaded.io.netty.util.NetUtil - Failed to find the loopback interface
11:17:50.140 [grpc-default-executor-0] WARN  i.g.n.s.i.n.u.i.MacAddressUtil - Failed to find a usable hardware address from the network interfaces; using random bytes: 7b:8f:0e:74:63:a6:1a:5a
Using SSLEngineImpl.
%% Initialized:  [Session-5, SSL_NULL_WITH_NULL_NULL]
JsseJCE:  Using KeyGenerator IbmTlsExtendedMasterSecret from provider TBD via init 
IBMJSSE2 value of useExtendedMasterSecret true
IBMJSSE2 will allow RFC 5746 renegotiation per com.ibm.jsse2.renegotiate set to none or default
IBMJSSE2 will not require renegotiation indicator during initial handshake per com.ibm.jsse2.renegotiation.indicator set to OPTIONAL or default taken
IBMJSSE2 will not perform identity checking against the peer cert check during renegotiation per com.ibm.jsse2.renegotiation.peer.cert.check set to OFF or default
IBMJSSE2 will allow client initiated renegotiation per jdk.tls.rejectClientInitiatedRenegotiation set to FALSE or default
IBMJSSE2 will not allow unsafe server certificate change during renegotiation per jdk.tls.allowUnsafeServerCertChange set to FALSE or default

Is initial handshake: true
Ignoring unsupported cipher suite: SSL_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 for TLSv1
Ignoring unsupported cipher suite: SSL_ECDHE_RSA_WITH_AES_256_GCM_SHA384 for TLSv1
Ignoring unsupported cipher suite: SSL_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 for TLSv1
Ignoring unsupported cipher suite: SSL_ECDHE_RSA_WITH_AES_128_GCM_SHA256 for TLSv1
No available cipher suite for TLSv1
grpc-nio-worker-ELG-1-3, fatal error: 40: Couldn't kickstart handshaking
javax.net.ssl.SSLHandshakeException: No appropriate protocol, may be no appropriate cipher suite specified or protocols are deactivated
%% Invalidated:  [Session-5, SSL_NULL_WITH_NULL_NULL]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be split out into its own condition. At the very least having it together is misleading.

In the future gRPC should check whether the specific provider supports Java 9 ALPN instead of just assuming "if the API is there, it must work." That would reorganize the code and we don't want to check for Jetty NPN/ALPN for the IBMJSSE2 provider. So having it be clear now is important to make sure we understand what is supported when.

gRPC requires HTTP/2 which requires TLSv1.2+. It looks like we're achieving that today by specifying ciphers that are only supported with TLSv1.2+. It looks like we could maybe specify sslContextBuilder.protocols("TLSv1.2") within GrpcSslContexts. We'd need to determine whether we should pass TLSv1.3 as well. Alternatively, it may be better to just say "TLS", but it appears that wouldn't work in Java 7.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, updated it.

Note we are currently checking for Java ALPN availability via JettyUtil API call.
It will return false and then trigger "No ALPN provider" error, if not available.

For the the sslContextBuilder, won't this work? sslContextBuilder.protocols("TLSv1.2","TLSv1.3")
I think you would have to add the new TLSv1.3 ciphers to the cipher list also with TLSv1.3 ones first
If the negotiation works right, the 1st matching cipher should be picked up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the the sslContextBuilder, won't this work? sslContextBuilder.protocols("TLSv1.2","TLSv1.3")

Yes, I expect something like that would. Although I'm not actually 100% certain things work properly with TLSv1.3, especially failures for client certificates. TLSv1.3 ciphers are already in our list. If we start explicitly stating TLSv1.3, it seems we should at least test it.

I'd also need to dig a bit into Netty to see what the list of protocols means; it's unclear to me how that gets translated to the Java security APIs.

IBMJSSE2 enhancement
@ejona86 ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Sep 15, 2020
@grpc-kokoro grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Sep 15, 2020
@ejona86
Copy link
Member

ejona86 commented Sep 15, 2020

As I mentioned in #5374, I'd be happy to have ibmjdk8 added to the .travis.yml. But I trust this change more than that one, since I doubted IBM JDK supported Jetty ALPN.

The main test that would benefit would be TlsTest. It does have a reference to SunJSSE, but that could be enhanced to check for IBMJSSE2 if SunJSSE is unavailable.

@kiwi1969
Copy link
Contributor Author

Note that IBM have own IBMJRE and IBMJSEE available for limited platforms.
The IBMJSSE code only works when running with IBM JRE
So extra work would be needed to set up IBM environment for testing
I don't know if you want to do that or not

Can download JDK from here
https://www.ibm.com/support/pages/java-sdk-downloads

Or use the Official Docker image
https://hub.docker.com/_/ibmjava

@ejona86
Copy link
Member

ejona86 commented Sep 15, 2020

Note that IBM have own IBMJRE and IBMJSEE available for limited platforms.

I linked to https://github.com/michaelklishin/jdk_switcher#jdk-aliases before. It seems to support auto-downloading ibmjdk8. I think that is the script used by travis-ci, so it seemed like it could be as easy as adding another 'jdk' line to our .travis.yml.

@ejona86
Copy link
Member

ejona86 commented Sep 15, 2020

I'm happy to merge this as-is. Tests can be follow-up if you'd like.

@sanjaypujare sanjaypujare changed the title Add support for IBMJSEE2 into grpc-java Add support for IBMJSSE2 into grpc-java Sep 15, 2020
Copy link
Contributor

@sanjaypujare sanjaypujare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ejona86 ejona86 merged commit 5879b53 into grpc:master Sep 16, 2020
wtlucy added a commit to wtlucy/open-liberty that referenced this pull request Sep 22, 2020
wtlucy added a commit to wtlucy/open-liberty that referenced this pull request Sep 24, 2020
dfawley pushed a commit to dfawley/grpc-java that referenced this pull request Jan 15, 2021
This is a very simple change to test for IBMJSSE2 security provider in addition to the others. IBM JRE does not support the Sun provider, but instead has IBMJSSE2 which supports the same API calls.

I tested this on Z/OS machine as now it works when before it couldn't find a security provider
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants