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

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
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@BrendanBall
Copy link

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 dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 9, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Mar 9, 2020

/cc @FiloSottile @katiehockman

@x4m
Copy link

x4m commented Mar 23, 2020 •
edited

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:]

@x4m x4m mentioned this issue Mar 23, 2020
Closed
@adamdecaf
Copy link
Contributor

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 adamdecaf mentioned this issue Jun 11, 2020
Closed
@adamdecaf
Copy link
Contributor

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

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 commented Mar 30, 2021

due to lack of maintenance...

@golang golang locked and limited conversation to collaborators Mar 30, 2022
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

Successfully merging a pull request may close this issue.

Use correct default hashes and default ciphers when no preferences given x4m/crypto
6 participants
@adamdecaf @FiloSottile @dmitshur @x4m @BrendanBall @gopherbot