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

Jump to bottom

dev.boringcrypto: AES_128_CBC_SHA and AES_256_CBC_SHA missing in defaultFIPSCipherSuites #36647

Closed
saurabhsuniljain opened this issue Jan 20, 2020 · 4 comments
Closed

dev.boringcrypto: AES_128_CBC_SHA and AES_256_CBC_SHA missing in defaultFIPSCipherSuites #36647

saurabhsuniljain opened this issue Jan 20, 2020 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@saurabhsuniljain
Copy link

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

$ go version
go version devel +35ba528 Mon Jul 2 18:35:13 2018 -0400 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

Ubuntu 18.04

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/bld"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build346953438=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Just an inquiry why following cipher suites are not available in the defaultFIPSCipherSuites in go/src/crypto/tls/boring.go

TLS_RSA_WITH_AES_128_CBC_SHA,
TLS_RSA_WITH_AES_256_CBC_SHA,

What did you expect to see?

As per the wiki here

AES128-SHA and AES256-SHA are both compatible in TLS 1.0 and 1.1 as well as TLS 1.2

What did you see instead?

I see these cipher suites are missing.
@saurabhsuniljain saurabhsuniljain changed the title AES128-SHA and AES256-SHA missing in defaultFIPSCipherSuites for dev.boringcrypto.go1.12 AES128-SHA and AES256-SHA missing in defaultFIPSCipherSuites for dev.boringcrypto.go1.12 Jan 20, 2020
@toothrot toothrot changed the title AES128-SHA and AES256-SHA missing in defaultFIPSCipherSuites for dev.boringcrypto.go1.12 dev.boringcrypto: AES128-SHA and AES256-SHA missing in defaultFIPSCipherSuites for dev.boringcrypto.go1.12 Jan 21, 2020
@toothrot
Copy link
Contributor

/cc @FiloSottile

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 21, 2020
@toothrot toothrot added this to the Unreleased milestone Jan 21, 2020
@FiloSottile FiloSottile changed the title dev.boringcrypto: AES128-SHA and AES256-SHA missing in defaultFIPSCipherSuites for dev.boringcrypto.go1.12 dev.boringcrypto: AES_128_CBC_SHA and AES_256_CBC_SHA missing in defaultFIPSCipherSuites Feb 4, 2020
@FiloSottile
Copy link
Contributor

The CBC cipher suites have unfortunate timing side channel issues, so that might be why they are not included. @agl originally picked the list and would know why we excluded them.

@agl
Copy link
Contributor

agl commented Feb 4, 2020

The CBC code wrapped from BoringSSL is the CBC implementation exported by the FIPS module, which does not handle TLS record decoding in constant-time. Also, the CBC modes are vestigial and have been removed from TLS 1.3 so, any time we're write cipher suite lists, we're trying not to include them.

@agl agl closed this as completed Feb 4, 2020
@saurabhsuniljain
Copy link
Author

@agl I want to understand the reason for not including them in the default list in more detail.

  1. Is the above-mentioned problem is only with Go's implementation of these cipher suites or the problem is itself with the cipher suites? (From your above comment I felt it's a problem with Go's implementation)
  2. Could explain the problem in some more detail, if possible.
  3. Are there any security issues?
  4. Are there any performance issues?

If I fork dev.boringcrypto branch and add these two cipher suites to the list of defaultFIPSCipherSuites. What will be the impact?

@victorpng victorpng mentioned this issue Mar 17, 2020
Closed
@golang golang locked and limited conversation to collaborators Mar 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@toothrot @agl @FiloSottile @saurabhsuniljain @gopherbot