From dbd4cdee420729fca969b95f763f78d6fca229a8 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Sat, 29 Oct 2022 09:34:07 +0900 Subject: [PATCH] ecdsa: Add error checks around key generation With older openssl package where smaller curves are hobbled out, ECDSA key generation in tests may fail. This adds checks of return value of GenerateKey() so the test functions stop its execution upon error. Signed-off-by: Daiki Ueno --- patches/000-initial-setup.patch | 264 +++++++++++++++++++------------- 1 file changed, 158 insertions(+), 106 deletions(-) diff --git a/patches/000-initial-setup.patch b/patches/000-initial-setup.patch index d1e235a3d3..18bbd45caa 100644 --- a/patches/000-initial-setup.patch +++ b/patches/000-initial-setup.patch @@ -1,65 +1,5 @@ -diff --git a/src/runtime/pprof/proto_test.go b/src/runtime/pprof/proto_test.go -index 84a051a536..a2cd97f14d 100644 ---- a/src/runtime/pprof/proto_test.go -+++ b/src/runtime/pprof/proto_test.go -@@ -15,6 +15,7 @@ import ( - "os/exec" - "reflect" - "runtime" -+ "strconv" - "strings" - "testing" - "unsafe" -@@ -95,11 +96,15 @@ func testPCs(t *testing.T) (addr1, addr2 uint64, map1, map2 *profile.Mapping) { - // region of memory. - t.Skipf("need 2 or more mappings, got %v", len(mprof.Mapping)) - } -- addr1 = mprof.Mapping[0].Start -+ addr1 = findAddrInExecutableSection(t, mmap, mprof.Mapping[0]) - map1 = mprof.Mapping[0] -+ map1.Offset = (addr1 - map1.Start) + map1.Offset -+ map1.Start = addr1 - map1.BuildID, _ = elfBuildID(map1.File) -- addr2 = mprof.Mapping[1].Start -+ addr2 = findAddrInExecutableSection(t, mmap, mprof.Mapping[1]) - map2 = mprof.Mapping[1] -+ map2.Offset = (addr2 - map2.Start) + map2.Offset -+ map2.Start = addr2 - map2.BuildID, _ = elfBuildID(map2.File) - case "js": - addr1 = uint64(abi.FuncPCABIInternal(f1)) -@@ -115,6 +120,29 @@ func testPCs(t *testing.T) (addr1, addr2 uint64, map1, map2 *profile.Mapping) { - return - } - -+func findAddrInExecutableSection(t *testing.T, mmap []byte, m *profile.Mapping) uint64 { -+ mappings := strings.Split(string(mmap), "\n") -+ for _, mapping := range mappings { -+ parts := strings.Fields(mapping) -+ if len(parts) < 6 { -+ continue -+ } -+ if !strings.Contains(parts[1], "x") { -+ continue -+ } -+ addr, err := strconv.ParseUint(strings.Split(parts[0], "-")[0], 16, 64) -+ if err != nil { -+ t.Fatal(err) -+ } -+ if addr >= m.Start && addr < m.Limit { -+ return addr -+ } -+ } -+ -+ t.Error("could not find executable section in /proc/self/maps") -+ return 0 -+} -+ - func TestConvertCPUProfile(t *testing.T) { - addr1, addr2, map1, map2 := testPCs(t) - diff --git a/api/go1.19.txt b/api/go1.19.txt -index 523f752d70..e9f2f7d173 100644 +index 523f752d70..778e1d5a7f 100644 --- a/api/go1.19.txt +++ b/api/go1.19.txt @@ -290,3 +290,5 @@ pkg sync/atomic, type Uint64 struct #50860 @@ -68,9 +8,27 @@ index 523f752d70..e9f2f7d173 100644 pkg time, method (Time) ZoneBounds() (Time, Time) #50062 +pkg crypto/ecdsa, func HashSign(io.Reader, *PrivateKey, []uint8, crypto.Hash) (*big.Int, *big.Int, error) #000000 +pkg crypto/ecdsa, func HashVerify(*PublicKey, []uint8, *big.Int, *big.Int, crypto.Hash) bool #000000 +diff --git a/src/cmd/go/testdata/script/gopath_std_vendor.txt b/src/cmd/go/testdata/script/gopath_std_vendor.txt +index a0a41a50de..208aa7008a 100644 +--- a/src/cmd/go/testdata/script/gopath_std_vendor.txt ++++ b/src/cmd/go/testdata/script/gopath_std_vendor.txt +@@ -21,11 +21,11 @@ go build . + + go list -deps -f '{{.ImportPath}} {{.Dir}}' . + stdout $GOPATH[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack +-! stdout $GOROOT[/\\]src[/\\]vendor ++! stdout $GOROOT[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack + + go list -test -deps -f '{{.ImportPath}} {{.Dir}}' . + stdout $GOPATH[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack +-! stdout $GOROOT[/\\]src[/\\]vendor ++! stdout $GOROOT[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack + + -- issue16333/issue16333.go -- + package vendoring17 diff --git a/src/crypto/ecdsa/ecdsa_hashsignverify.go b/src/crypto/ecdsa/ecdsa_hashsignverify.go new file mode 100644 -index 0000000000..54db9ae178 +index 0000000000..37f3a18223 --- /dev/null +++ b/src/crypto/ecdsa/ecdsa_hashsignverify.go @@ -0,0 +1,45 @@ @@ -121,7 +79,7 @@ index 0000000000..54db9ae178 +} diff --git a/src/crypto/ecdsa/ecdsa_hashsignverify_test.go b/src/crypto/ecdsa/ecdsa_hashsignverify_test.go new file mode 100644 -index 0000000000..8f95e8af1f +index 0000000000..d12ba2f441 --- /dev/null +++ b/src/crypto/ecdsa/ecdsa_hashsignverify_test.go @@ -0,0 +1,42 @@ @@ -167,24 +125,58 @@ index 0000000000..8f95e8af1f + testHashSignAndHashVerify(t, elliptic.P384(), "p384") + testHashSignAndHashVerify(t, elliptic.P521(), "p521") +} -diff --git a/src/cmd/go/testdata/script/gopath_std_vendor.txt b/src/cmd/go/testdata/script/gopath_std_vendor.txt -index a0a41a50de..208aa7008a 100644 ---- a/src/cmd/go/testdata/script/gopath_std_vendor.txt -+++ b/src/cmd/go/testdata/script/gopath_std_vendor.txt -@@ -21,11 +21,11 @@ go build . +diff --git a/src/crypto/ecdsa/ecdsa_test.go b/src/crypto/ecdsa/ecdsa_test.go +index 77a8134316..1cbade9f78 100644 +--- a/src/crypto/ecdsa/ecdsa_test.go ++++ b/src/crypto/ecdsa/ecdsa_test.go +@@ -62,7 +62,10 @@ func TestSignAndVerify(t *testing.T) { + } - go list -deps -f '{{.ImportPath}} {{.Dir}}' . - stdout $GOPATH[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack --! stdout $GOROOT[/\\]src[/\\]vendor -+! stdout $GOROOT[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack + func testSignAndVerify(t *testing.T, c elliptic.Curve) { +- priv, _ := GenerateKey(c, rand.Reader) ++ priv, err := GenerateKey(c, rand.Reader) ++ if err != nil { ++ t.Fatal(err) ++ } - go list -test -deps -f '{{.ImportPath}} {{.Dir}}' . - stdout $GOPATH[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack --! stdout $GOROOT[/\\]src[/\\]vendor -+! stdout $GOROOT[/\\]src[/\\]vendor[/\\]golang.org[/\\]x[/\\]net[/\\]http2[/\\]hpack + hashed := []byte("testing") + r, s, err := Sign(rand.Reader, priv, hashed) +@@ -86,7 +89,10 @@ func TestSignAndVerifyASN1(t *testing.T) { + } - -- issue16333/issue16333.go -- - package vendoring17 + func testSignAndVerifyASN1(t *testing.T, c elliptic.Curve) { +- priv, _ := GenerateKey(c, rand.Reader) ++ priv, err := GenerateKey(c, rand.Reader) ++ if err != nil { ++ t.Fatal(err) ++ } + + hashed := []byte("testing") + sig, err := SignASN1(rand.Reader, priv, hashed) +@@ -110,7 +116,10 @@ func TestNonceSafety(t *testing.T) { + } + + func testNonceSafety(t *testing.T, c elliptic.Curve) { +- priv, _ := GenerateKey(c, rand.Reader) ++ priv, err := GenerateKey(c, rand.Reader) ++ if err != nil { ++ t.Fatal(err) ++ } + + hashed := []byte("testing") + r0, s0, err := Sign(zeroReader, priv, hashed) +@@ -141,7 +150,10 @@ func TestINDCCA(t *testing.T) { + } + + func testINDCCA(t *testing.T, c elliptic.Curve) { +- priv, _ := GenerateKey(c, rand.Reader) ++ priv, err := GenerateKey(c, rand.Reader) ++ if err != nil { ++ t.Fatal(err) ++ } + + hashed := []byte("testing") + r0, s0, err := Sign(rand.Reader, priv, hashed) diff --git a/src/crypto/ed25519/ed25519_test.go b/src/crypto/ed25519/ed25519_test.go index 7c5181788f..102c4e5355 100644 --- a/src/crypto/ed25519/ed25519_test.go @@ -258,7 +250,7 @@ new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/crypto/internal/backend/nobackend.go b/src/crypto/internal/backend/nobackend.go new file mode 100644 -index 0000000000..98066f55fc +index 0000000000..482ed6f470 --- /dev/null +++ b/src/crypto/internal/backend/nobackend.go @@ -0,0 +1,155 @@ @@ -419,7 +411,7 @@ index 0000000000..98066f55fc +} diff --git a/src/crypto/internal/backend/openssl.go b/src/crypto/internal/backend/openssl.go new file mode 100644 -index 0000000000..7dc24420a0 +index 0000000000..4040c77bc1 --- /dev/null +++ b/src/crypto/internal/backend/openssl.go @@ -0,0 +1,105 @@ @@ -791,6 +783,32 @@ index 314016979a..323d683788 100644 } type x25519Parameters struct { +diff --git a/src/crypto/x509/boring.go b/src/crypto/x509/boring.go +index 4aae90570d..42706f93c4 100644 +--- a/src/crypto/x509/boring.go ++++ b/src/crypto/x509/boring.go +@@ -26,7 +26,7 @@ func boringAllowCert(c *Certificate) bool { + default: + return false + case *rsa.PublicKey: +- if size := k.N.BitLen(); size != 2048 && size != 3072 { ++ if size := k.N.BitLen(); size != 2048 && size != 3072 && size != 4096 { + return false + } + case *ecdsa.PublicKey: +diff --git a/src/crypto/x509/boring_test.go b/src/crypto/x509/boring_test.go +index 7010f44b32..70021f3bdd 100644 +--- a/src/crypto/x509/boring_test.go ++++ b/src/crypto/x509/boring_test.go +@@ -54,7 +54,7 @@ type boringCertificate struct { + + func TestBoringAllowCert(t *testing.T) { + R1 := testBoringCert(t, "R1", boringRSAKey(t, 2048), nil, boringCertCA|boringCertFIPSOK) +- R2 := testBoringCert(t, "R2", boringRSAKey(t, 4096), nil, boringCertCA) ++ R2 := testBoringCert(t, "R2", boringRSAKey(t, 4096), nil, boringCertCA|boringCertFIPSOK) + + M1_R1 := testBoringCert(t, "M1_R1", boringECDSAKey(t, elliptic.P256()), R1, boringCertCA|boringCertFIPSOK) + M2_R1 := testBoringCert(t, "M2_R1", boringECDSAKey(t, elliptic.P224()), R1, boringCertCA) diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index 141fdb9fbd..d8e81d921d 100644 --- a/src/go/build/deps_test.go @@ -847,6 +865,66 @@ index 141fdb9fbd..d8e81d921d 100644 haveImport["C"] = true // kludge: prevent C from appearing in crypto/internal/boring imports } fset := token.NewFileSet() +diff --git a/src/runtime/pprof/proto_test.go b/src/runtime/pprof/proto_test.go +index 84a051a536..a2cd97f14d 100644 +--- a/src/runtime/pprof/proto_test.go ++++ b/src/runtime/pprof/proto_test.go +@@ -15,6 +15,7 @@ import ( + "os/exec" + "reflect" + "runtime" ++ "strconv" + "strings" + "testing" + "unsafe" +@@ -95,11 +96,15 @@ func testPCs(t *testing.T) (addr1, addr2 uint64, map1, map2 *profile.Mapping) { + // region of memory. + t.Skipf("need 2 or more mappings, got %v", len(mprof.Mapping)) + } +- addr1 = mprof.Mapping[0].Start ++ addr1 = findAddrInExecutableSection(t, mmap, mprof.Mapping[0]) + map1 = mprof.Mapping[0] ++ map1.Offset = (addr1 - map1.Start) + map1.Offset ++ map1.Start = addr1 + map1.BuildID, _ = elfBuildID(map1.File) +- addr2 = mprof.Mapping[1].Start ++ addr2 = findAddrInExecutableSection(t, mmap, mprof.Mapping[1]) + map2 = mprof.Mapping[1] ++ map2.Offset = (addr2 - map2.Start) + map2.Offset ++ map2.Start = addr2 + map2.BuildID, _ = elfBuildID(map2.File) + case "js": + addr1 = uint64(abi.FuncPCABIInternal(f1)) +@@ -115,6 +120,29 @@ func testPCs(t *testing.T) (addr1, addr2 uint64, map1, map2 *profile.Mapping) { + return + } + ++func findAddrInExecutableSection(t *testing.T, mmap []byte, m *profile.Mapping) uint64 { ++ mappings := strings.Split(string(mmap), "\n") ++ for _, mapping := range mappings { ++ parts := strings.Fields(mapping) ++ if len(parts) < 6 { ++ continue ++ } ++ if !strings.Contains(parts[1], "x") { ++ continue ++ } ++ addr, err := strconv.ParseUint(strings.Split(parts[0], "-")[0], 16, 64) ++ if err != nil { ++ t.Fatal(err) ++ } ++ if addr >= m.Start && addr < m.Limit { ++ return addr ++ } ++ } ++ ++ t.Error("could not find executable section in /proc/self/maps") ++ return 0 ++} ++ + func TestConvertCPUProfile(t *testing.T) { + addr1, addr2, map1, map2 := testPCs(t) + diff --git a/src/runtime/runtime_boring.go b/src/runtime/runtime_boring.go index 5a98b20253..dc25cdcfd5 100644 --- a/src/runtime/runtime_boring.go @@ -861,29 +939,3 @@ index 5a98b20253..dc25cdcfd5 100644 + return boring_runtime_arg0() +} \ No newline at end of file -diff --git a/src/crypto/x509/boring.go b/src/crypto/x509/boring.go -index 4aae90570d..42706f93c4 100644 ---- a/src/crypto/x509/boring.go -+++ b/src/crypto/x509/boring.go -@@ -26,7 +26,7 @@ func boringAllowCert(c *Certificate) bool { - default: - return false - case *rsa.PublicKey: -- if size := k.N.BitLen(); size != 2048 && size != 3072 { -+ if size := k.N.BitLen(); size != 2048 && size != 3072 && size != 4096 { - return false - } - case *ecdsa.PublicKey: -diff --git a/src/crypto/x509/boring_test.go b/src/crypto/x509/boring_test.go -index 7010f44b32..70021f3bdd 100644 ---- a/src/crypto/x509/boring_test.go -+++ b/src/crypto/x509/boring_test.go -@@ -54,7 +54,7 @@ type boringCertificate struct { - - func TestBoringAllowCert(t *testing.T) { - R1 := testBoringCert(t, "R1", boringRSAKey(t, 2048), nil, boringCertCA|boringCertFIPSOK) -- R2 := testBoringCert(t, "R2", boringRSAKey(t, 4096), nil, boringCertCA) -+ R2 := testBoringCert(t, "R2", boringRSAKey(t, 4096), nil, boringCertCA|boringCertFIPSOK) - - M1_R1 := testBoringCert(t, "M1_R1", boringECDSAKey(t, elliptic.P256()), R1, boringCertCA|boringCertFIPSOK) - M2_R1 := testBoringCert(t, "M2_R1", boringECDSAKey(t, elliptic.P224()), R1, boringCertCA)