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

crypto/tls: TLS 1.3 ciphers are not configurable #29349

Closed
crvv opened this issue Dec 20, 2018 · 31 comments
Closed

crypto/tls: TLS 1.3 ciphers are not configurable #29349

crvv opened this issue Dec 20, 2018 · 31 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@crvv
Copy link
Contributor

crvv commented Dec 20, 2018

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

go version devel +e3b4b7baad Tue Dec 18 23:01:06 2018 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes if 1.12beta1 is the latest release.

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

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/crvv/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/crvv/Develop/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/crvv/Develop/goroot"
GOTMPDIR=""
GOTOOLDIR="/Users/crvv/Develop/goroot/pkg/tool/darwin_amd64"
GCCGO="gccgo"
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/x1/1pp78x6d3n99gpx9f7rz2_rh0000gn/T/go-build965128833=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run the following code with GODEBUG=http2server=0 go run main.go

package main

import (
	"crypto/tls"
	"log"
	"net/http"
)

func main() {
	server := http.Server{
		Addr: "localhost:2443",
		TLSConfig: &tls.Config{
			CipherSuites: []uint16{tls.TLS_CHACHA20_POLY1305_SHA256},
			MinVersion:   tls.VersionTLS13,
		},
	}
	log.Fatal(server.ListenAndServeTLS("localhost.pem", "localhost-key.pem"))
}

and openssl s_client -connect localhost:2443

What did you expect to see?

A TLS 1.3 connection with TLS_CHACHA20_POLY1305_SHA256 cipher.

What did you see instead?

SSL handshake has read 1603 bytes and written 391 bytes
Verification error: unable to verify the first certificate
---
New, TLSv1.3, Cipher is TLS_AES_256_GCM_SHA384

It looks like Config.CipherSuites isn't used in TLS 1.3.
If this is the desired behavior, I think it should be documented in https://tip.golang.org/pkg/crypto/tls/

@agnivade
Copy link
Contributor

From the release notes -

TLS 1.3 cipher suites are not configurable. All supported cipher suites are safe, and if PreferServerCipherSuites is set in Config the preference order is based on the available hardware.

@crvv
Copy link
Contributor Author

crvv commented Dec 20, 2018

I understand the reason to make it not configurable.

But there are only two kinds of CipherSuites.

[]uint16{TLS_AES_128_GCM_SHA256, TLS_CHACHA20_POLY1305_SHA256, TLS_AES_256_GCM_SHA384}

and

[]uint16{TLS_CHACHA20_POLY1305_SHA256, TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384}

That is to say, if the server and client are both written in Go, TLS_AES_256_GCM_SHA384 can never be used, which I think is kind of strange.

Besides the documentation, I think this can be reconsidered or maybe tls.Listen should return an error if Config.CipherSuites contains TLS 1.3 ciphers.

@agnivade
Copy link
Contributor

/cc @FiloSottile

@ulikunitz
Copy link
Contributor

The TLS 1.3 implementation should at least allow the developer to select cipher suites from the supported set of cipher suites and change the priority of the supported cipher suites.

Why? It is obvious that the security assessment of the ciphers defined by TLS 1.3 might change. I'm old enough to know that SHA1 was the recommended hash algorithm for quite some time. Allowing the developer to exclude an algorithm gives him a chance to react for instance to meet internal compliance requirements while the cipher suite must still be supported in general to support users with legacy software.

The capability to mask interoperability issues with another TLS implementation instantly, is another reason to provide developers with the capability to change priorities of the supported algorithms.

@FiloSottile FiloSottile changed the title crypto/tls: Config.CipherSuites doesn't work for TLS 1.3 crypto/tls: reject TLS 1.3 ciphers in Config.CipherSuites Dec 20, 2018
@FiloSottile
Copy link
Contributor

Configuration is necessary if the ecosystem has issues that require manual mitigation. The TLS 1.3 ecosystem thankfully doesn't yet, so until it's necessary we will not add a config option. Cipher suites happen to be something that required configuration in TLS 1.2, but by the argument that everything should be configurable in case it becomes necessary later, there are half a dozen axes in TLS 1.3 (cipher suites, groups, signature algorithms, certificate signature algorithms, PSK modes...) and we are not adding a config option for each.

Returning an error if Config.CipherSuites contains TLS 1.3 ciphers sounds like a good idea, though.

@FiloSottile FiloSottile self-assigned this Dec 21, 2018
@FiloSottile FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 21, 2018
@FiloSottile FiloSottile added this to the Go1.12 milestone Dec 21, 2018
@ulikunitz
Copy link
Contributor

Yes, the usage of cryptogragraphic methods must be configurable. Beside the two reasons, change of security assessment of a cryptographic algorithm and handling of interoperability issues, I have already given, there is a third one: compliance to government or internal policies. Such a policy might forbid ChaCha20, because the algorithm is not supported by government standards. How does a developer address such a requirement?

Not everybody shares your assessment that ciphers in TLS 1.3 have no issues. Dan Bernstein and Tanja Lange regard P256 as unsafe as documented in https://safecurves.cr.yp.to/. Apparently the Go implementation shares that assessment, because it prefers X25519 over P256. Burt what about developers that want to disable P256 completely?

@FiloSottile
Copy link
Contributor

@ulikunitz curves have nothing to do with cipher suites in TLS 1.3.

tls.Listen-time check are probably not worth it (#29779 (comment)) so I'm going to add a note to the Config.CipherSuites docs, and close this.

@gopherbot
Copy link

Change https://golang.org/cl/158637 mentions this issue: crypto/tls: expand Config.CipherSuites docs

@ulikunitz
Copy link
Contributor

@FiloSottile

Let me write at first that I'm aware that you were deeply involved in the development of TLS 1.3. I watched your presentation at the Chaos Communication Congress regarding it. I appreciate the work you put in the development in the new algorithm specifically the work on TLS 1.3.

Let me clarify the point regarding P265. In my opinion every TLS implementation should at least provide the capability to disable any cryptographic algorithm for which there are multiple alternatives available independent how they are negotiated in the protocol. So my ask is actually independent of the cipher suites.

The rationale is:

  1. A lot of organisations have policies in place that define the set of algorithms including important parameters like key lengths that can be supported in the organisation. Not providing a capability to disable certain algorithms will create issues for the use of the Go language in those organisations.

  2. In the practice there are issues for specific algorithms in TLS implementations. We observe regularly TLS connection resets at a rate of ca. 1/3000 in our production environment with Oracle's Java implementation of ECDHE for TLS 1.2. Since Java supports the disablement of crypto algorithms for TLS, we are able to work around the issue until it can be resolved with Oracle.

  3. In the case of the identification of a vulnerability or general cryptographic problem of the TLS 1.3 Golang implementation there is only the option of updating the software. In my experience a software update triggers more dependencies and requires more testing effort then a software configuration change. The time frame differs here between minutes to sevaral hours for the configuration change and days to weeks for the software update.

Since I'm not providing patches to do what I see as necessary, I will stop discussing the point any further. I'm convinced that you and I want the best for the Golang community while we disagree that this
includes configurability of the cryptographic algorithms for the Golang TLS 1.3 implementation.

@kopax
Copy link

kopax commented Dec 25, 2019

I am having issue reverse proxying https with traefik v2.1.0, I have TLS_AES_128_GCM_SHA256 error.

Caused by: java.lang.IllegalArgumentException: No enum constant org.sonarsource.scanner.api.internal.shaded.okhttp.CipherSuite.TLS_AES_128_GCM_SHA256
	at java.base/java.lang.Enum.valueOf(Enum.java:240)
	at org.sonarsource.scanner.api.internal.shaded.okhttp.CipherSuite.valueOf(CipherSuite.java:32)

This prevent me from running sonarqube 6.3.1 test, do you guys know how I can fix this ?

Thanks and merry Christmas

@jangaraj
Copy link

@kopax You have Java exception, which is really out of the Golang scope.

You should configure both client/server side (it looks like SonarQube side has a problem in this case) to support that cipher TLS_AES_128_GCM_SHA256. Or enable additional ciphers supported by SonarQube in Traefik.

But again SonarQube (eventually Traefik) configuration is out of the scope of this Golang issue. You should discuss it in your SonarQube thread.

@p-kraszewski
Copy link

I'm convinced that you and I want the best for the Golang community while we disagree that this includes configurability of the cryptographic algorithms for the Golang TLS 1.3 implementation.

So there's no Go code, that would demonstrate tls.VersionTLS13+tls.TLS_AES_256_GCM_SHA384 between Go server and Go client?

This implies TLS1.3 in its current state in Go is essentially worthless for policy-governed applications with requirement of AES256, as there is no way to prioritize AES256, spare disable AES128. Not cool.

I had to lock TLS .MaxVersion at tls.VersionTLS12 (because without that link springs up to 1.3 and then degrades cipher without any notification) and I'm happy now with tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384.

I'm not accusing, I'm not ranting, I'm just stating a fact.

@FiloSottile
Copy link
Contributor

The only cryptographic benefits of AES-256 I'm aware of are better conjectured post-quantum resistance, and safer margins in the multi-user setting. The former is irrelevant in TLS 1.3 as the key exchange is itself not post-quantum, and multi-user attacks are not a problem thanks to the randomized nonces.

Go does not add options for irrelevant choices unless forced to by the ecosystem. The move to TLS 1.3 is hopefully a good opportunity for those policies to be updated not to mandate irrelevant choices.

Likewise, responding to @ulikunitz's second point, TLS 1.3 hopefully lets the implementation ecosystem start fresh, and fix issues rather than work around them. About cipher suites specifically, they are all so similar and abstracted in TLS 1.3 that I would be surprised if issues specific to one of them were to arise, and I haven't seen anything of the sort yet.

Automatic, secure interoperability is better than an ecosystem where you have to pick the right configuration to talk to each specific peer. If we can help force this ecosystem direction, all TLS 1.3 users, not just Go users, will benefit.

Finally, about @ulikunitz's third point: if a security issue were to be found, we would publish a small security patch for all supported releases, like any other security vulnerability. A deployment that can't apply those timely has a bigger problem and won't be saved by TLS cipher suite configurability.

Hopefully the time of TLS vulnerabilities being so frequent that they deserve special treatment is well behind us, and anyway I'd like to point out that I don't remember a crypto/tls vulnerability, thanks I believe to its strong stance against complexity, which this decision is in line with.

@p-kraszewski
Copy link

The only cryptographic benefits of AES-256 I'm aware of are better conjectured post-quantum resistance

Last time it were "two digits to represent a year are enough" and "nobody sane would ever need a file larger than 4GB".

As for "better conjectured post-quantum resistance" - this is bread and butter of security-related companies and govt agencies (let's say, I work for one of them). Lack of AES256 is "take that technology off the table" scenario.

  • Essentially every current CPU has AES implemented in-hardware.
  • There isn't that much difference in speed/load/energy between hardware assisted AES128 and AES256, especially when the usual bottleneck is I/O (be it storage or network, depending on which you encrypt) and even the fanciest encryption modes are 10 to 100-fold faster than necessary without noticeable CPU overhead.

What's saddening is "we know better" approach. I'm not against AES128 as a default. I'm against not being able to force AES256 when legally required (as in my case). Luckily I found a workaround that saved my project.

@Sebi2020

This comment has been minimized.

@crvv
Copy link
Contributor Author

crvv commented Apr 13, 2020

The point is whether AES-256 is worthy to use.
Obviously the authors of TLS 1.3 think it is worthy, so I can't accept that AES-256 is worthless.
Maybe AES-256 is worthy but not for Go applications. This will have more persuasiveness if it is true.

@volschin
Copy link

I think you guys should follow the standard and make it configurable for the admins they want to do so.
I have no understanding for rejecting this.

@ulikunitz
Copy link
Contributor

The Cockroach project disabled TLS 1.3 because there are bugs in the TLS 1.3 implementation in Java that have been fixed in recent updates, but have not been widely deployed.

See cockroachdb/cockroach#48294

@euca01
Copy link

euca01 commented Jun 6, 2020

Anyone know if this feature will be added ?

@volschin
Copy link

volschin commented Jun 7, 2020

Not as long as Google follows the opinion "640 kB ought to be enough for anybody."

@euca01
Copy link

euca01 commented Jun 7, 2020

Not as long as Google follows the opinion "640 kB ought to be enough for anybody."

@FiloSottile What can we you do to try to change your mind ?
(Let me be clear, i've have Oreo :) nobody can resist Oreo :) )

@FiloSottile FiloSottile changed the title crypto/tls: reject TLS 1.3 ciphers in Config.CipherSuites crypto/tls: TLS 1.3 ciphers are not configurable Jun 7, 2020
@FiloSottile
Copy link
Contributor

@FiloSottile What can we you do to try to change your mind ?
(Let me be clear, i've have Oreo :) nobody can resist Oreo :) )

The same as with every decision: present a case for why and how adding this configuration option is beneficial and necessary for common, reasonable users, and consistent with https://golang.org/design/cryptography-principles. (And, I mean, of course Oreo.)

So far, the two arguments here have been: compliance with policies (requiring AES-256), which we tend to heavily discount especially when they add complexity for no coherent security benefit (which AES-256 doesn't have #29349 (comment)); and agility in the face of hypothetical security or compatibility issues, which is definitely an anti-goal of the Go cryptography libraries where we believe it's the job of the library, not the developer, to figure out the intricacies of the TLS ecosystem, and which would be an argument to make every single negotiated list of codepoints configurable (of which there are half a dozen).

@ulikunitz
Copy link
Contributor

I have to correct the statement regarding the CockroachDB issue. It is caused by #35722, which isn't related to a cipher implementation, so a cipher configuration option would not have helped.

@bwesterb
Copy link

With regards to the quantum argument: we expect AES 128 to be secure for a long time after the first RSA and elliptic curves keys have been broken by quantum computers. The reason is that Grover's algorithm has a long runtime which requires immense error correction (and thus additional qubits).

@jared2501
Copy link

Just flagging that we've had to disable Go's TLS 1.3 implementation at my work since our software is deployed by government agencies that require us to run FIPS 140-2 compliant crypto algorithms (i.e. RSA, ECDSA, & AES). While I agree that reducing configurability in the name of reducing complexity is laudable, it does lock out entire classes of users (namely, government & financial ones).

@FiloSottile I doubt that you would consider the ability for TLS 1.3 to grow a "FIPS mode" option that would restrict the negotiated algorithms (i.e. rather than making them fully configurable)? I suppose the answer for users wanting FIPS compliant crypto is to run https://github.com/golang/go/tree/dev.boringcrypto?

@p-kraszewski
Copy link

p-kraszewski commented Aug 25, 2020

Following @jared2501 comment, to the powers-to-be of Go crypto systems. Please understand. We are programmers and developers working in predefined environment. We don't set rules, we don't set requirements, we don't have any influence on any of those aspects. We are expected to comply.
If Go people wish to discuss security requirements with government officials, I suppose we would be able to provide contacts to some of their secretaries. But please, make a podcast of the talk and give us an hour earlier notice to let us all buy cola and popcorn to watch the show.
There are few industries (mil, gov, med, fintech, automotive) where you either comply or GTFO. And if Golang is fine with GTFO, it is sad news for us, developers writing in Go.

@FiloSottile
Copy link
Contributor

I suppose the answer for users wanting FIPS compliant crypto is to run https://github.com/golang/go/tree/dev.boringcrypto?

Correct. By my understanding of FIPS 140, selecting ciphersuites is not nearly enough for compliance at any level.

@ericlagergren
Copy link
Contributor

ericlagergren commented Aug 25, 2020 via email

@jared2501
Copy link

jared2501 commented Aug 25, 2020

Yeah, there's a difference between "FIPS validated" and "FIPS compliant" - for "FIPS compliant", your ciphersuite selection is enough.

@crvv
Copy link
Contributor Author

crvv commented Aug 26, 2020

The same as with every decision: present a case for why and how adding this configuration option is beneficial and necessary for common, reasonable users, and consistent with https://golang.org/design/cryptography-principles. (And, I mean, of course Oreo.)

I don't think the minority can be ignored here.
You shouldn't discriminate against those programs running in government.

@FiloSottile
Copy link
Contributor

FiloSottile commented Aug 26, 2020

Yeah, there's a difference between "FIPS validated" and "FIPS compliant" - for "FIPS compliant", you ciphersuite selection is enough.

Can you point me to somewhere I can read more? I never heard of that interpretation. AFAIK, TLS 1.3 itself is not FIPS 140-2 compliant.

Rather than commenting on a closed issue, what would work here would be to open a new one about a use case (not about a specific solution) with supporting information to show what the requirements are like and how common they are. They don't need to be the majority, but they need to be concrete and achievable and different from the BoringCrypto ones.

aminjam pushed a commit to cloudfoundry/cf-networking-helpers that referenced this issue Aug 9, 2021
This is setting a max version for TLS 1.2 since TLS 1.3
ciphers is not configurable (golang/go#29349)
aminjam pushed a commit to cloudfoundry/cf-networking-helpers that referenced this issue Aug 9, 2021
This is setting a max version for TLS 1.2 since TLS 1.3
ciphers is not configurable (golang/go#29349)
@golang golang locked and limited conversation to collaborators Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests