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

Using underscore character('_') as part of SNI leads JVM crash #8144

Closed
r9liucc opened this issue Jul 23, 2018 · 4 comments
Closed

Using underscore character('_') as part of SNI leads JVM crash #8144

r9liucc opened this issue Jul 23, 2018 · 4 comments
Assignees
Labels
Milestone

Comments

@r9liucc
Copy link

r9liucc commented Jul 23, 2018

Hello guys, we use Netty as our network implementation of microservices framework. We plan to support https and thus enable authentication on both client side and server side. But we encounter a problem when we want to use underscore character('_') in TLS SNI.

Steps to reproduce

1. Netty Server implementation:

// define ssl provider
private static final SslProvider SSL_PROVIDER = SslProvider.OPENSSL;

// factory method for creating ssl context
public SslContext serverCtx(InputStream certificate, InputStream privateKey) {
        try {
            return SslContextBuilder.forServer(certificate, privateKey)
                                    .sslProvider(SSL_PROVIDER)
                                    .trustManager(TwoWayTrustManagerFactory.INSTANCE)
                                    .clientAuth(ClientAuth.REQUIRE)
                                    .build();
        } catch (Exception e) {
            throw new RuntimeException("failed to create ssl context", e);
        }
}

// SNI Matcher implementation
@Slf4j
public class MySNIMatcher extends SNIMatcher {

    private final byte[] hostIdentifier;

    public MySNIMatcher(Identifier hostIdentifier) {
        super(0);
        this.hostIdentifier = hostIdentifier.idDistKey().getBytes(StandardCharsets.US_ASCII);
    }

    @Override
    public boolean matches(SNIServerName sniServerName) {
        if (log.isDebugEnabled()) {
            log.debug("match sni {}", new String(sniServerName.getEncoded(), StandardCharsets.US_ASCII));
        }
        return Arrays.equals(sniServerName.getEncoded(), hostIdentifier);
    }
}

// Netty bootstrap
bootstrap.childHandler(new ChannelInitializer<Channel>() {
            @Override
            protected void initChannel(Channel ch) throws Exception {
                ChannelPipeline pipeline = ch.pipeline();

                if (conf.logEnable())
                    pipeline.addLast("logging", new LoggingHandler());

                if (conf.sslEnable()) {
                    // conf.sslCtx() will actually use serverCtx() to create context
                    SSLEngine engine = conf.sslCtx().newEngine(ch.alloc());
                    SSLParameters s = engine.getSSLParameters();
                    s.setSNIMatchers(new MySNIMatcher(...));
                    s.setEndpointIdentificationAlgorithm("HTTPS");
                    engine.setSSLParameters(s);
                    pipeline.addLast("ssl", new SslHandler(engine, true));
                }

                codec(pipeline);

                int idleTimeout = (int) TimeUnit.MILLISECONDS.toSeconds(conf.idleTimeout());
                if (0 < idleTimeout)
                    pipeline.addLast("idle", new IdleStateHandler(0, 0, idleTimeout));

                pipeline.addLast("handler", handler);
            }
        })

2. Openssl Client test:

openssl s_client -cert service.crt -key service.key -CAfile root_ca.pem -connect 127.0.0.1:8080 -servername rb8hx3pww30y3tvw0mwy.v1_1
CONNECTED(00000003)
write:errno=0
---
no peer certificate available
---
No client certificate CA names sent
---
SSL handshake has read 0 bytes and written 210 bytes
Verification: OK
---
New, (NONE), Cipher is (NONE)
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : 0000
    Session-ID:
    Session-ID-ctx:
    Master-Key:
    PSK identity: None
    PSK identity hint: None
    SRP username: None
    Start Time: 1532336403
    Timeout   : 7200 (sec)
    Verify return code: 0 (ok)
    Extended master secret: no
---

As we can see from the command, we used rb8hx3pww30y3tvw0mwy.v1_1 as sni

stdout of JVM process when crash:

java: ../ssl/handshake_server.c:541: negotiate_version: Assertion `!ssl->s3->have_version' failed.

Please note that if we use sni which excluded underscore character(e.g. rb8hx3pww30y3tvw0mwy.v1), everything works fine, I can see sni logs in MySNIMatcher.

Netty version

Netty: 4.1.14.Final
Boringssl: netty-tcnative-boringssl-static:2.0.5.Final

Openssl Version

1.0.2o

JVM version (e.g. java -version)

java version "1.8.0_112"
Java(TM) SE Runtime Environment (build 1.8.0_112-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.112-b16, mixed mode)

OS version (e.g. uname -a)

macOS High Sierra 10.13.5
Darwin Kernel Version 17.6.0

@r9liucc r9liucc changed the title Using underline character('_') as part of SNI leads JVM crash Using underscore character('_') as part of SNI leads JVM crash Jul 24, 2018
@normanmaurer normanmaurer self-assigned this Jul 24, 2018
@normanmaurer
Copy link
Member

interesting... Let me investigate

@r9liucc
Copy link
Author

r9liucc commented Jul 24, 2018

@normanmaurer I have tested this case on netty 4.1.27.Final and netty-tcnative-boringssl-static 2.0.12.Final, the process would not crash any more. It seems that the latest version of boringssl have fixed this issue. Then the validation actually takes place in the io.netty.handler.ssl. ReferenceCountedOpenSslEngine#checkSniHostnameMatch() and it blocks me since underscore character is not allowed in definition of host name in rfc 1123 (which is based on rfc 952).

A "name" (Net, Host, Gateway, or Domain name) is a text string up to 24 characters drawn from the alphabet (A-Z), digits (0-9), minus sign (-), and period (.).

But I‘m just curious about the JDK that provided the abstract class SNIMatcher which used
SNIServerName to represent the "server name", and it seems to be a liberal way to implement SNI. I have also tested my case on JDK provided SSL implementation and the underscore character is passed the test.

normanmaurer added a commit that referenced this issue Jul 25, 2018
Motivation:

We should allow to also validate sni hostname which contains for example underscore when using our native SSL impl. The JDK implementation does this as well.

Modifications:

- Construct the SNIHostName via byte[] and not String.
- Add unit test

Result:

Fixes #8144.
@normanmaurer
Copy link
Member

@r9liucc can you please verify #8150 ?

@normanmaurer normanmaurer added this to the 4.1.28.Final milestone Jul 25, 2018
@r9liucc
Copy link
Author

r9liucc commented Jul 26, 2018

@normanmaurer I have verified this on branch sni_underscore and the results seems ok.
Openssl Client test:

openssl s_client -cert service.crt -key service.key  -CAfile root_ca.pem -connect 127.0.0.1:8080 -servername rb7yh582ov5d0umxr2td.v1_1
CONNECTED(00000003)
depth=1 C = CN, ST = SC, L = CD, O = HCB, OU = HCB, CN = my_org
verify return:1
depth=0 C = CN, ST = SC, L = CD, O = HCB, OU = HCB, CN = my_cn
verify return:1
---
Certificate chain
 0 s:/C=CN/ST=SC/L=CD/O=HCB/OU=HCB/CN=my_cn
   i:/C=CN/ST=SC/L=CD/O=HCB/OU=HCB/CN=my_org
---
Server certificate
-----BEGIN CERTIFICATE-----
...ignore
-----END CERTIFICATE-----
subject=/C=CN/ST=SC/L=CD/O=HCB/OU=HCB/CN=my_cn
issuer=/C=CN/ST=SC/L=CD/O=HCB/OU=HCB/CN=my_org
---
No client certificate CA names sent
Client Certificate Types: RSA sign, ECDSA sign
Requested Signature Algorithms: ECDSA+SHA256:0x04+0x08:RSA+SHA256:ECDSA+SHA384:0x05+0x08:RSA+SHA384:0x06+0x08:RSA+SHA512:RSA+SHA1
Shared Requested Signature Algorithms: ECDSA+SHA256:RSA+SHA256:ECDSA+SHA384:RSA+SHA384:RSA+SHA512:RSA+SHA1
Peer signing digest: SHA256
Server Temp Key: ECDH, P-256, 256 bits
---
SSL handshake has read 1364 bytes and written 2507 bytes
---
New, TLSv1/SSLv3, Cipher is ECDHE-RSA-AES128-GCM-SHA256
Server public key is 2048 bit
Secure Renegotiation IS supported
No ALPN negotiated
SSL-Session:
    Protocol  : TLSv1.2
    Cipher    : ECDHE-RSA-AES128-GCM-SHA256
    Session-ID: 9C33100A039306570D48E007DF129AE9E36EAC63E5E0044D99F52FA5DC4F261F
    Session-ID-ctx:
    Master-Key: 05D02E543544B0F9CC5BB0BC6D38B21FA440C77810B1857553FC24F534E408E4CD727E57CA814B1623B6D85FB7B2B6EA
    Key-Arg   : None
    PSK identity: None
    PSK identity hint: None
    SRP username: None
    Start Time: 1532596523
    Timeout   : 300 (sec)
    Verify return code: 0 (ok)
---
GET /hello HTTP/1.1

HTTP/1.1 200 OK
Content-Type: application/json;charset=UTF-8
Content-Length: 7

"hello"

I have also verified outbound data and the client sni has successfully been sent out for negotiation.
Client code snippets:

private final List<SNIServerName> sni = new ArrayList<>();
// init sni
this.sni.add(new SNIHostName(remoteIdentifier.idDistKey().getBytes(StandardCharsets.UTF_8)));

// set sni
SSLParameters sslParameters = sslEngine.getSSLParameters();
sslParameters.setServerNames(sni);
sslEngine.setSSLParameters(sslParameters);
...

normanmaurer added a commit that referenced this issue Jul 26, 2018
Motivation:

We should allow to also validate sni hostname which contains for example underscore when using our native SSL impl. The JDK implementation does this as well.

Modifications:

- Construct the SNIHostName via byte[] and not String.
- Add unit test

Result:

Fixes #8144.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants