crypto/tls: extension (5) error with TLS 1.3 with Go server and Java client

[adobley]

What version of Go are you using (go version)?

$ go version
go version go1.13.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/h7/7dckdqy514j9bjr451bjx8rr0000gn/T/go-build975453702=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

We have a Go server that uses TLS1.3. We are able to successfully connect to it with a Go client.

We have a Java application running on OpenJDK 11.0.5_10. When connecting to the server we get an exception: javax.net.ssl.SSLHandshakeException: extension (5) should not be presented in certificate_request.

When we disable TLS1.3 on either side it works. For example, setting GODEBUG to have tls13=0 or using an older version of Java that does not support TLS1.3.

❓ Has anyone else seen this problem?
❓ Is there a way to change the extensions presented?

Unfortunately, we haven't been able to reproduce this on a fully local environment due to java-localhost-cert problems.

What did you expect to see?

A successful TLS handshake and the http request to complete.

What did you see instead?

We see an exception in the Java client javax.net.ssl.SSLHandshakeException: extension (5) should not be presented in certificate_request and an error reported on the Go server remote error: tls: unsupported extension

For redundancy, our main questions here are:
❓ Has anyone else seen this problem?
❓ Is there a way to change the extensions presented?

cc/ @ameowlia

[odeke-em]

Hello there @adobley, thank you for this bug report and welcome to the Go project!

I just saw this issue right before bed and spun up for you something in Go and Java, to hopefully complete your bug report and fully diagnose the problem at https://github.com/odeke-em/bugs/tree/master/golang/35722 whose structure will look like this

$ tree .
.
├── cert.pem
├── jettyclient
│   ├── pom.xml
│   └── src
│       └── main
│           └── java
│               └── org
│                   └── golang
│                       └── bugs
│                           └── b35722
│                               └── Client.java
├── key.pem
└── main.go

and with the files inlined below:

Details

main.go -- contains the server

package main

import (
	"fmt"
	"net/http"
	"time"
)

func main() {
	mux := http.NewServeMux()
	mux.HandleFunc("/hello", func(w http.ResponseWriter, r *http.Request) {
		fmt.Fprintf(w, "%s", time.Now())
	})
	addr := ":8888"
	println("Serving on " + addr)
	if err := http.ListenAndServeTLS(addr, "cert.pem", "key.pem", mux); err != nil {
		panic(err)
	}
}

jettyclient/src/main/java/org/golang/bugs/b35722/Client.java

package org.golang.bugs.b35722;

import java.nio.charset.StandardCharsets;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.HttpRequest;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.eclipse.jetty.client.api.ContentResponse;

public class Client {
    public static void main(String[] args) {
        SslContextFactory insecureSkipVerifyFactory = new SslContextFactory.Client(true);
        HttpClient httpClient = new HttpClient(insecureSkipVerifyFactory);

        try {
            httpClient.start();
        } catch (Exception e) {
            System.err.println("Failed to start the HTTP client " + e);
            return;
        }

        String url = "https://localhost:8888/hello";
        if (args.length > 0 && !args[0].isEmpty())
            url = args[0];

        HttpRequest req = (HttpRequest) httpClient.newRequest(url);
        try {
            ContentResponse res = req.send();
            String payload = new String(res.getContent(), StandardCharsets.UTF_8);
            System.out.println("Response:\n" + payload);
        } catch (Exception e) {
            System.err.println("Exception instead of response " + e);
        }
    }
}

jettyclient/pom.xml

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
    <modelVersion>4.0.0</modelVersion>
    <groupId>org.golang.bugs</groupId>
    <artifactId>bug-35722</artifactId>
    <packaging>jar</packaging>
    <version>0.0.1</version>
    <name>gobugs</name>
    <url>http://maven.apache.org</url>

    <properties>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <!-- Change this to the Jetty version that you would like to use -->
        <jetty.version>9.4.18.v20190429</jetty.version>
    </properties>

    <dependencies>
        <dependency>
            <groupId>org.eclipse.jetty</groupId>
            <artifactId>jetty-client</artifactId>
            <version>${jetty.version}</version>
        </dependency>
    </dependencies>

    <build>
        <extensions>
            <extension>
                <groupId>kr.motd.maven</groupId>
                <artifactId>os-maven-plugin</artifactId>
                <version>1.5.0.Final</version>
            </extension>
        </extensions>

        <pluginManagement>
            <plugins>
                <plugin>
                    <groupId>org.apache.maven.plugins</groupId>
                    <artifactId>maven-compiler-plugin</artifactId>
                    <version>3.7.0</version>
                    <configuration>
                        <source>1.8</source>
                        <target>1.8</target>
                    </configuration>
                </plugin>
            </plugins>
        </pluginManagement>

        <plugins>
            <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>appassembler-maven-plugin</artifactId>
                <version>1.10</version>
                <configuration>
                    <programs>
                        <program>
                            <id>SQLApp</id>
                            <mainClass>org.golang.bugs.b35722.Client</mainClass>
                        </program>
                    </programs>
                </configuration>
            </plugin>
        </plugins>
    </build>
</project>

Running it

Run the server first

$ go run main.go 
Serving on :8888

Run the Jetty client

$ mvn install && mvn exec:java -Dexec.mainClass=org.golang.bugs.b35722.Client

and that produces for the client

2019-11-21 02:00:18.524:INFO::org.golang.bugs.b35722.Client.main(): Logging initialized @1836ms to org.eclipse.jetty.util.log.StdErrLog
2019-11-21 02:00:18.672:WARN:oejusS.config:org.golang.bugs.b35722.Client.main(): Trusting all certificates configured for Client@7215a16f[provider=null,keyStore=null,trustStore=null]
2019-11-21 02:00:18.673:WARN:oejusS.config:org.golang.bugs.b35722.Client.main(): No Client EndPointIdentificationAlgorithm configured for Client@7215a16f[provider=null,keyStore=null,trustStore=null]
Response:
2019-11-21 02:00:18.836025 -0500 EST m=+1072.942000100

and from above you'll see that with Jetty we are able to get a successful response (by skipping verification of certificates) so perhaps you might have to make a custom TrustManager, but anyways the repros I've provided above should perhaps help seed you with an MVP to complete this bug report and diagnosis.

I am off to sleep but I shall kindly ping @FiloSottile @katiehockman @agl to take a look or be ware of this report.

[FiloSottile]

Extension 5 is status_request. Starting in TLS 1.3 status_request can be sent as an empty extension in CertificateRequest to request the client send an OCSP response.

  A server MAY request that a client present an OCSP response with its
   certificate by sending an empty "status_request" extension in its
   CertificateRequest message.  If the client opts to send an OCSP
   response, the body of its "status_request" extension MUST be a
   CertificateStatus structure as defined in [RFC6066].

CertificateRequest extensions are new in TLS 1.3, and OpenSSL also got this wrong and broke down talking to Go (#34040). It looks like OpenJDK 11 is rejecting a spec-compliant behavior on our side.

@davidben have you seen this before?

[davidben]

I haven't, though we don't send status_request in CertificateRequest. I agree with you that rejecting the extension in CertificateRequest is non-compliant. Although it's unclear to me what 'empty "status_request" extension' in RFC 8446 is trying to say, since a "semantically empty" status_request extension isn't quite empty.
https://tools.ietf.org/html/rfc6066#section-8

I suspect this is a mistake and the RFC shouldn't have said "empty" there. What to do about it now, I'm not sure. :-(

Do you know if OpenJDK 11 is rejecting the extension because it is not a valid CertificateStatusRequest, or because it's present at all? The latter is definitely wrong, but the former is unclear.

[Lekensteyn]

The change that adds "empty status_request" was done in tlswg/tls13-spec#910, but I can't exactly remember the reason for doing it. The PR lacks a discussion too. My guess is that the change was done based on a misreading or observed implementation behavior.

When the CH includes status_request with CertificateStatusRequest, the server should reply with a SH status_request extension to acknowledge its support, and send a CertificateStatus handshake message later. The SH extension carries no further meaning and is therefore empty.

In TLS 1.3 the server can also send a status_request extension in the CR extensions. Unlike the earlier server-side status_request extension, this one functions like the CH extension and should therefore not be empty.

It should probably be filed as errata, allowing non-empty extensions again, but I am afraid it will break implementations that strictly follow the spec. Sorry for this mess.

References:

• https://tools.ietf.org/html/rfc8446#section-4.4.2.1
• https://tools.ietf.org/html/rfc6066#section-8

[davidben]

I suppose, worst case, we can always give up on status_request in CertificateRequest and allocate status_request_oops_lets_try_that_again. :-)

[FiloSottile]

Yeah, this is unfortunate. It definitely makes sense for it to contain a CertificateStatusRequest, but I think the language of the spec at this point is pretty clearly defining it to be zero length.

Wireshark also expects it to be empty, and I am strictly enforcing it in Go, so that ship might have sailed unless we want to leave behind a big chunk of Go versions.

Anyway, I really can't read Java, but it looks to me like OpenJDK simply doesn't know status_request is allowed in CertificateRequest. https://github.com/openjdk/jdk/blob/92d5dcf3335179ff68b761aa9e2cc83bf74d47fe/src/java.base/share/classes/sun/security/ssl/CertStatusExtension.java

[ianlancetaylor]

@FiloSottile Is there anything we should do here for 1.14?

[martinuy]

I've raised this issue in OpenJDK's upstream [1], and has been acknowledged [2].

BTW, I completely agree with @Lekensteyn in the empty status_request comment.

--
[1] - https://mail.openjdk.java.net/pipermail/security-dev/2019-December/021036.html
[2] - https://mail.openjdk.java.net/pipermail/security-dev/2019-December/021037.html

[FiloSottile]

@FiloSottile Is there anything we should do here for 1.14?

If the WG decides to issue an errata we should probably relax our client implementation to allow non-empty extensions, to mitigate the compatibility mess. It's a very very safe change. However, I'm mildly opposed to it still, because even if we backport it there will be many Go clients out there that are intolerant to the "rich" extension, because we followed the spec as it was written initially (even if I agree it would make sense for it not to be empty).