Skip to content

go/src/crypto/cipher/gcm.go: package should return error instead of panicking #70087

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

Closed
ashishd-dev opened this issue Oct 29, 2024 · 2 comments
Closed

Comments

@ashishd-dev
Copy link

ashishd-dev commented Oct 29, 2024

Go version

1.23.0

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/ashish/Library/Caches/go-build'
GOENV='/Users/ashish/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/ashish/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/ashish/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.23.0/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.0'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/ashish/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/Users/ashish/zoop-workspace/source-workspace/zoop-vault/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/3m/67_m4cl52491fp444xl3dlxm0000gn/T/go-build674732644=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

this is is a short piece of code I have written for handling AES GCM encryption and I invoked it with incorrect IV Size

`func AesDecrypt(data []byte, aesKey []byte, iv []byte) ([]byte, error) {

block, err := aes.NewCipher(aesKey)
if err != nil {
	return nil, err
}

// Create GCM mode on the cipher block
aesGCM, err := cipher.NewGCM(block)

//here this is needed because aes package panics if size is not right, instead of sending error
if len(iv) != aesGCM.NonceSize() {
	return nil, errors.New(constants.PkgErrors.AES)
}

message, err := aesGCM.Open(nil, iv, data, nil)
if err != nil {
	return nil, err
}

return message, err

}`

The crypto/cipher package in go stdlib - houses AES GCM encryption logic inside the gcm.go file - shown as follows

func (g *gcm) Open(dst, nonce, ciphertext, data []byte) ([]byte, error) { if len(nonce) != g.nonceSize { panic("crypto/cipher: incorrect nonce length given to GCM") } //more code below - refer crypto/cipher/gcm.go }

What did you see happen?

My code panicked instead of returning proper error that IV size is invalid, this prevents me from writing test cases and other code constructs that would otherwise be possible had the package returned an erro

Due to this folks who are not aware of strict restrictions on IV/Nonce perhaps due to some domain knowledge deficient - need to add validations in there code right before invoking the Open() method

Instead if an appropriate error can returned and without panic - the package will be much better for developer experience and application reliability

What did you expect to see?

I expected that the package should return some standard error of roughly the following form

errors.New("crypto/cipher: incorrect nonce length given to GCM")

@seankhliao
Copy link
Member

see #21624 the panics are intentional. Using incorrect nonce sizes are a programmer and must never happen.

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

No branches or pull requests

3 participants