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.

Sign up for GitHub

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

Jump to bottom

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

Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
44ef7bb
Update GrpcSslContexts.java
kiwi1969 Sep 13, 2020
80c1bef
Add files via upload
kiwi1969 Sep 13, 2020
e9f5bc5
Delete grpc-netty-shaded-1.33.0-SNAPSHOT.jar
kiwi1969 Sep 13, 2020
d47497b
Update GrpcSslContexts.java
kiwi1969 Sep 13, 2020
9a7eca4
Update GrpcSslContexts.java
kiwi1969 Sep 13, 2020
143ccbd
Prebuilt jar for IBMJSEE testing
kiwi1969 Sep 13, 2020
10b9089
Delete grpc-netty-shaded-1.33.0-SNAPSHOT.jar
kiwi1969 Sep 14, 2020
dc4b905
Update GrpcSslContexts.java
kiwi1969 Sep 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions netty/src/main/java/io/grpc/netty/GrpcSslContexts.java
View file
Open in desktop
Expand Up @@ -82,6 +82,7 @@ private GrpcSslContexts() {}
NEXT_PROTOCOL_VERSIONS);

private static final String SUN_PROVIDER_NAME = "SunJSSE";
private static final String IBM_PROVIDER_NAME = "IBMJSSE2";

/**
* Creates an SslContextBuilder with ciphers and APN appropriate for gRPC.
Expand Down Expand Up @@ -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 •
edited by sanjaypujare

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.

// Jetty ALPN/NPN only supports one of NPN or ALPN
if (JettyTlsUtil.isJettyAlpnConfigured()) {
apc = ALPN;
Expand All @@ -196,7 +198,7 @@ public static SslContextBuilder configure(SslContextBuilder builder, Provider jd
apc = ALPN;
} else {
throw new IllegalArgumentException(
SUN_PROVIDER_NAME + " selected, but Java 9+ and Jetty NPN/ALPN unavailable");
jdkProvider.getName() + " selected, but Java 9+ and Jetty NPN/ALPN unavailable");
}
} else if (ConscryptLoader.isConscrypt(jdkProvider)) {
apc = ALPN;
Expand Down Expand Up @@ -237,7 +239,8 @@ private static SslProvider defaultSslProvider() {

private static Provider findJdkProvider() {
for (Provider provider : Security.getProviders("SSLContext.TLS")) {
if (SUN_PROVIDER_NAME.equals(provider.getName())) {
if (SUN_PROVIDER_NAME.equals(provider.getName())
|| IBM_PROVIDER_NAME.equals(provider.getName())) {
if (JettyTlsUtil.isJettyAlpnConfigured()
|| JettyTlsUtil.isJettyNpnConfigured()
|| JettyTlsUtil.isJava9AlpnAvailable()) {
Expand Down