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.

Sign up for GitHub

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

crypto/ecdsa: custom curve implementations are not supported with boringcrypto #54486

Closed
Jorropo opened this issue Aug 16, 2022 · 2 comments
Closed

crypto/ecdsa: custom curve implementations are not supported with boringcrypto #54486

Jorropo opened this issue Aug 16, 2022 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@Jorropo
Copy link
Member

Jorropo commented Aug 16, 2022 •
edited

I don't know if you are interested with boringcrypto related reports, if not just close this issue.

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

$ go version
go version go1.19 X:boringcrypto linux/amd64

Does this issue reproduce with the latest release?

Yes (assuming you build with boringcrypto).
No if you use the go native implementation.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/hugo/.cache/go-build"
GOENV="/home/hugo/.config/go/env"
GOEXE=""
GOEXPERIMENT="boringcrypto"
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/hugo/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/hugo/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/home/hugo/k/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/hugo/k/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.19 X:boringcrypto"
GCCGO="gccgo"
GOAMD64="v3"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/hugo/k/go/src/go.mod"
GOWORK=""
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 -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2573245019=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ git clone https://github.com/Jorropo/no-custom-curves-repro && cd no-custom-curves-repro
$ go test .

What did you expect to see?

ok  	github.com/Jorropo/no-custom-curves-repro	0.012s

What did you see instead?

--- FAIL: TestSecp256k1 (0.00s)
    a_test.go:12: boringcrypto: unknown elliptic curve
FAIL
FAIL	github.com/Jorropo/no-custom-curves-repro	0.004s
FAIL

This is because the boring crypto tries to match the curve name to P-224, P-256, P-384 or P-521 and if this fails return an error:

go/src/crypto/internal/boring/ecdsa.go

Lines 40 to 52 in e49e876

func curveNID(curve string) (C.int, error) {
switch curve {
case "P-224":
return C.GO_NID_secp224r1, nil
case "P-256":
return C.GO_NID_X9_62_prime256v1, nil
case "P-384":
return C.GO_NID_secp384r1, nil
case "P-521":
return C.GO_NID_secp521r1, nil
}
return 0, errUnknownCurve
}

In my case secp256p1 is implement by having custom ECC methods:
https://github.com/decred/dcrd/blob/master/dcrec/secp256k1/ellipticadaptor.go

I expect crypto/ecdsa to fallback to the native golang implementation which would call into thoses custom methods and work (like it already do if I build without boringcrypto) if I pass in a custom curve.

Adding secp256k1 support to the boringcrypto wrapper (if possible) would also solve my issue, but it doesn't feel like the right thing to do if done alone.
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 16, 2022
@seankhliao
Copy link
Member

cc @golang/security

@rolandshoemaker
Copy link
Member

The boringcrypto module is only intended to work with a small set of certified algorithms and parameters (in this case, the NIST defined curves), and as such cannot support custom curves.

@Jorropo Jorropo mentioned this issue Aug 17, 2022
Closed
@golang golang locked and limited conversation to collaborators Aug 17, 2023
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
No milestone
Development

No branches or pull requests
4 participants
@rolandshoemaker @gopherbot @seankhliao @Jorropo