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

proposal: crypto/tls: expose a session identifier #46718

Closed
drakkan opened this issue Jun 12, 2021 · 5 comments
Closed

proposal: crypto/tls: expose a session identifier #46718

drakkan opened this issue Jun 12, 2021 · 5 comments
Assignees
@FiloSottile
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Proposals (old)
Milestone
Proposal

Comments

@drakkan
Copy link
Member

drakkan commented Jun 12, 2021

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

$ go version
go version go1.16.5 linux/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=""
GOCACHE="/home/nicola/.cache/go-build"
GOENV="/home/nicola/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/nicola/go/pkg/mod"
GONOPROXY="gl.syncplify.me/devteam/*"
GONOSUMDB="gl.syncplify.me/devteam/*"
GOOS="linux"
GOPATH="/home/nicola/go"
GOPRIVATE="gl.syncplify.me/devteam/*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.5"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build2054184453=/tmp/go-build -gno-record-gcc-switches"

crypto/tls has no API to verify which TLS session was resumed. This is an issue for FTPS: to avoid data connection stealing vulnerability we need to require TLS session resumption and to enforce that the TLS session on the data connection was resumed from the one on the control connection.

Please take a look here for more details.

I propose to add two new API to the ConnectionState struct:

// GetID returns a unique identifier for a TLS connection
GetID() []byte
// ResumedFrom returns the session identifier from which this session was resumed.
// It returns nil if the session was not resumed
ResumedFrom() []byte

this way we can store/get the session ID for the FTP control connection and check that ResumedFrom matches the expected ID.

I'm aware that a unique TLS identifier is not easy to expose and that it is difficult to match TLS sessions (but at least possible) also with OpenSSL
@gopherbot gopherbot added this to the Proposal milestone Jun 12, 2021
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Jun 12, 2021
@seankhliao
Copy link
Member

Duplicate of #25228 , related #18346

@drakkan
Copy link
Member Author

drakkan commented Jun 12, 2021 •
edited

@seankhliao I saw the linked tickets, I think this is not a duplicate of #25228: the session resumption already supported in Go works fine for my use case, but it is not possible to check which session was resumed. So this proposal isn't about implementing session id resumption, session tickets are ok.

Regarding #18346 this is a different use case and TLSUnique is now deprecated and it is nil for resumed sessions, so it is not useful for the exposed use case.

I'm unable to find any existing ticket/proposal about a API to verify which TLS session was resumed, this is the reason I opened a new proposal, thank you

@seankhliao
Copy link
Member

cc @FiloSottile

@seankhliao seankhliao reopened this Jun 12, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jun 12, 2021
@FiloSottile FiloSottile self-assigned this Mar 2, 2022
@FiloSottile FiloSottile modified the milestones: Proposal, Go1.19 Mar 2, 2022
@FiloSottile FiloSottile mentioned this issue May 10, 2022
Closed
@ianlancetaylor ianlancetaylor modified the milestones: Go1.19, Proposal Jun 24, 2022
@FiloSottile FiloSottile mentioned this issue Jan 12, 2023
Open
@FiloSottile FiloSottile mentioned this issue May 10, 2023
Open
@gopherbot
Copy link

Change https://go.dev/cl/496822 mentions this issue: crypto/tls: add SessionState.Extra and ConnectionState.Session

@drakkan
Copy link
Member Author

drakkan commented Aug 17, 2023 •
edited

I think this is now possible using WrapSession/UnwrapSession and SessionState.Extra. Thank you!

@drakkan drakkan closed this as completed Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@FiloSottile FiloSottile

Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Proposals
Status: Incoming
Release Status & Release Blockers
Status: Done
Proposals (old)
Incoming
Milestone
Proposal
Development

No branches or pull requests
5 participants
@drakkan @FiloSottile @ianlancetaylor @gopherbot @seankhliao