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/clearsign: incomplete fix in CL 173778 #41200

Closed
brianmay opened this issue Sep 2, 2020 · 3 comments
Closed

x/crypto/openpgp/clearsign: incomplete fix in CL 173778 #41200

brianmay opened this issue Sep 2, 2020 · 3 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

@brianmay
Copy link

brianmay commented Sep 2, 2020

Summary: CVE-2019-11841 was fixed by https://go.googlesource.com/crypto/+/c05e17bb3b2dca130fc919668a96b4bec9eb9442 but this fix is incomplete, and appears to fix only one of the test cases.

https://security-tracker.debian.org/tracker/CVE-2019-11841

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

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

What did you do?

I ran the script supplied from https://packetstormsecurity.com/files/152840/Go-Cryptography-Libraries-Cleartext-Message-Spoofing.html after fixing an error in the PGP key - see https://salsa.debian.org/bam/cve-2019-11841/-/blob/master/sig_spoof.go

What did you expect to see?

The first test should succeed, the second and third tests should fail.

What did you see instead?

The first test succeed, the second test succeeded, and only the third test failed. The 2nd test should not succeed because the hash header was tampered with.

$ go get golang.org/x/crypto/openpgp/clearsign
$ go run sig_spoof.go                                                                
Verifying not tampered...
Signature accepted!

Verifying spoofed hash...
Signature accepted!

Verifying spoofed cleartext...
No clearsign text found
@gopherbot gopherbot added this to the Unreleased milestone Sep 2, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Sep 2, 2020

/cc @FiloSottile

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 2, 2020
@dmitshur dmitshur changed the title x/crypto: CVE-2019-11841 incomplete fix x/crypto/openpgp/clearsign: incomplete fix in CL 173778 Sep 2, 2020
@FiloSottile
Copy link
Contributor

That's intentional and documented in the package and in the commit message you link to. The hash header value has no security purposes.

@brianmay
Copy link
Author

brianmay commented Sep 3, 2020

I not convinced that is the case that the value of the Hash header is insignificant. Using the example in the commit message, I can maliciously change the hash header to:

Hash: I need you to wire $100k to 12345566 as soon as possible.

And the signature check will still pass.

Not to mention, as per the example I referenced, some programs might think or display that the message was signed with a secure hash algorithm when they are not.

@golang golang locked and limited conversation to collaborators Sep 3, 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
4 participants
@brianmay @FiloSottile @dmitshur @gopherbot