I think OptionalSslHandler is very simple. Let's just create our own which applies the same fix that Balazs did for the non-optional case.
On Fri, Aug 9, 2024 at 10:04 AM Andor Molnar <an...@apache.org> wrote: > Thanks Balazs, I've found your fix since then and this is the > OptionalSslHandler case, because I have plaintext mode enabled. > > Looks like there's no way to pass peer host/port to SslEngine, so I > still think that explicitly disabling client hostname verification would > be beneficial in this case. We could emit a warning message in the logs > saying, hey, client hostname verification is disabled, because plaintext > mode is on. > > The other way I'm thinking about is to implement our own version of > OptionalSslHandler, but haven't verified if it works yet. > > Andor > > > > On 8/9/24 1:13 AM, Balazs Meszaros wrote: > > Hi Andor! > > > > SSLEngine does not know anything about the socket that is used for the > > communication. It just encrypt/decrypt byte streams that you pass through > > it. If you check its source code, it just returns that it got in the > > constructor: > > > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/javax/net/ssl/SSLEngine.html#%3Cinit%3E(java.lang.String,int) > > > > SSLEngine is not constructed directly in our code, it is wrapped by > Netty. > > Netty creates it when you call SslContext.newHandler. There are multiple > > newHandler functions which accept the peer information: > > > https://netty.io/4.1/api/io/netty/handler/ssl/SslContext.html#newHandler-io.netty.buffer.ByteBufAllocator-java.lang.String-int- > > > > I modified the handler creation in some places in HBASE-27673. You have > to > > check the other code paths as well to see how it is created. For example > we > > are using OptionalSsl handler but it does not pass peer information when > > creating the handler. We construct it here: > > > https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcServer.java#L390 > > > > Best regards, > > Balazs > > > > On Fri, Aug 9, 2024 at 3:52 AM Andor Molnar <an...@apache.org> wrote: > > > >> Hi, > >> > >> I'm debugging a strange error in our mTLS setup where I had to > >> explicitly disable client hostname verification, because HBase keeps > >> trying to validate 127.0.0.1/localhost as the peer host. > >> > >> ------------------------------------------ > >> 2024-08-09 01:32:21,486 ERROR > >> org.apache.hadoop.hbase.io.crypto.tls.HBaseTrustManager: Failed to > >> verify host address: 127.0.0.1 > >> javax.net.ssl.SSLPeerUnverifiedException: Certificate for <127.0.0.1> > >> doesn't match common name of the certificate subject: my-perfect- > >> hostname > >> ... > >> 2024-08-09 01:32:21,486 ERROR > >> org.apache.hadoop.hbase.io.crypto.tls.HBaseTrustManager: Failed to > >> verify hostname: localhost > >> javax.net.ssl.SSLPeerUnverifiedException: Certificate for <localhost> > >> doesn't match common name of the certificate subject: my-perfect- > >> hostname > >> ------------------------------------------ > >> > >> First it tries 127.0.0.1 and localhost after, because reverse lookup is > >> enabled, but why is that localhost? > >> > >> Check is here: > >> > >> > https://github.com/apache/hbase/blob/master/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/HBaseTrustManager.java#L97 > >> > >> The version of checkClientTrusted with chain, authType and engine. > >> Based on SSLEngine javadoc: > >> ------------------------------------------ > >> /** > >> * Returns the host name of the peer. > >> * <P> > >> * Note that the value is not authenticated, and should not be > >> * relied upon. > >> * > >> * @return the host name of the peer, or null if nothing is > >> * available. > >> */ > >> public String getPeerHost() { > >> return peerHost; > >> } > >> ------------------------------------------ > >> > >> That explains it, because if the peerHost is null, > >> InetAddress.getByName() doesn't fail and it returns the localhost. I > >> have no idea under what circumstances can the peerHost be unknown, but > >> would like to add a null check and skip hostname verification with a > >> warning message in such case. > >> > >> It would be nice to get the peerHost from somewhere else as a fallback. > >> > >> Regards, > >> Andor > >> > >> > >> > >> > > >
