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

x/crypto/openpgp: new entities cannot be encrypted to by default #37646

Closed
BrendanBall opened this issue Mar 4, 2020 · 6 comments
Closed

x/crypto/openpgp: new entities cannot be encrypted to by default #37646

BrendanBall opened this issue Mar 4, 2020 · 6 comments

Comments

@BrendanBall
Copy link

@BrendanBall BrendanBall commented Mar 4, 2020

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

$ go version
go version go1.14 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/brendan/.cache/go-build"
GOENV="/home/brendan/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/brendan/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/brendan/dev/scratch/golang-pgp/go.mod"
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-build998074745=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tried creating a new pgp private key with openpgp.NewEntity and use it to encrypt some text. I passed nil as config for sensible defaults as per the docs: If config is nil, sensible defaults will be used.

What did you expect to see?

That the encryption works with no errors

What did you see instead?

an error: openpgp: invalid argument: cannot encrypt because no candidate hash functions are compiled in. (Wanted RIPEMD160 in this case.)

Note there's an old closed issue for this issue 12153, however it was closed without being fixed properly. Looking at the PR this is easy to see that the case where config is nil is not handled correctly, at least as far as the docs say that there'll be sensible defaults.

@gopherbot gopherbot added this to the Unreleased milestone Mar 4, 2020
@dmitshur dmitshur changed the title x/crypto: openpgp: new entities cannot be encrypted to by default x/crypto/openpgp: new entities cannot be encrypted to by default Mar 4, 2020
@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented Mar 9, 2020

@x4m
Copy link

@x4m x4m commented Mar 23, 2020

Yup. I've almost had to add RIPEMD160 to WAL-G due to this wal-g/wal-g#362
It goes like this
https://github.com/golang/crypto/blob/master/openpgp/write.go#L358

If you do not prefer anything there should be sane SHA-256 but what we get is
defaultHashes := candidateHashes[len(candidateHashes)-1:]

@adamdecaf
Copy link
Contributor

@adamdecaf adamdecaf commented Jun 11, 2020

Wouldn't we want to take the first hash? RIPEMD160 is deprecated and staticcheck errors as such.

I've tried specifying SHA256 and still been unable to encrypt. The error I get is similar to the original report.

@adamdecaf
Copy link
Contributor

@adamdecaf adamdecaf commented Jun 12, 2020

I'm trying to trace this down and it looks like the conversion between crypto.Hash and openpgp/s2k. HashIdToHash is broken? Or I'm passing in the wrong typed uint8. @FiloSottile can you help?

crypto.SHA256 from the stdlib has a value of 5 -- https://github.com/golang/go/blob/master/src/crypto/crypto.go#L73

Inside openpgp.Encrypt we register SHA256 as a candidate hash and get its s2k mapping value: https://github.com/golang/crypto/blob/master/openpgp/write.go#L273

For SHA256 this seems to compare 5 against 8 in the s2k lookup: https://github.com/golang/crypto/blob/master/openpgp/s2k/s2k.go#L236

That comparison seems wrong. Shouldn't it be 5? None of those mappings seem correct, so I'm not sure how this ever worked. That makes me think I'm missing a mapping or using the wrong typed uint8.

Specifying crypto. RIPEMD160 works, but it's deprecated and insecure.

@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented Mar 29, 2021

Per the accepted #44226 proposal and due to lack of maintenance, the golang.org/x/crypto/openpgp package is now frozen and deprecated. No new changes will be accepted except for security fixes. The package will not be removed.

If this is a security issue, please email security@golang.org and we will assess it and provide a fix.

If you're looking for alternatives, consider the crypto/ed25519 package for simple signatures, golang.org/x/mod/sumdb/note for inline signatures, or filippo.io/age for encryption. You can read a summary of OpenPGP issues and alternatives here.

If you are required to interoperate with OpenPGP systems and need a maintained package, we suggest considering one of multiple community forks of golang.org/x/crypto/openpgp. We don't endorse any specific one.

@x4m
Copy link

@x4m x4m commented Mar 30, 2021

due to lack of maintenance...

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

Successfully merging a pull request may close this issue.

6 participants