aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRuss Cox <rsc@golang.org>2017-08-28 14:27:03 -0400
committerRuss Cox <rsc@golang.org>2017-09-22 20:07:58 +0000
commit043302c56d8a6f021e097e63ba25e734df1c4cd6 (patch)
treea5081afd91b23c9bfcb8e8ccf279f51c1975f751
parent20cf4713643e1bb4a56cc018ad946cc8cc586bdb (diff)
downloadgo-043302c56d8a6f021e097e63ba25e734df1c4cd6.tar.gz
go-043302c56d8a6f021e097e63ba25e734df1c4cd6.zip
[dev.boringcrypto.go1.8] cmd/compile: hide new boring fields from reflection
This is terrible but much simpler, cleaner, and more effective than all the alternatives I have come up with. Lots of code assumes that reflect.DeepEqual is meaningful on rsa.PublicKey etc, because previously they consisted only of exported meaningful fields. Worse, there exists code that assumes asn1.Marshal can be passed an rsa.PublicKey, because that struct has historically matched exactly the form that would be needed to produce the official ASN.1 DER encoding of an RSA public key. Instead of tracking down and fixing all of that code (and probably more), we can limit the BoringCrypto-induced damage by ensliting the compiler to hide the new field from reflection. Then nothing can get at it and nothing can be disrupted by it. Kill two birds with one cannon ball. I'm very sorry. Change-Id: I0ca4d6047c7e98f880cbb81904048c1952e278cc Reviewed-on: https://go-review.googlesource.com/60271 Reviewed-by: Adam Langley <agl@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-on: https://go-review.googlesource.com/65478 Run-TryBot: Russ Cox <rsc@golang.org>
-rw-r--r--src/cmd/compile/internal/gc/reflect.go19
-rw-r--r--src/crypto/hmac/hmac_test.go25
-rw-r--r--src/crypto/rsa/boring_test.go40
3 files changed, 84 insertions, 0 deletions
diff --git a/src/cmd/compile/internal/gc/reflect.go b/src/cmd/compile/internal/gc/reflect.go
index 61ac67c0bc..0341ccabfa 100644
--- a/src/cmd/compile/internal/gc/reflect.go
+++ b/src/cmd/compile/internal/gc/reflect.go
@@ -1312,9 +1312,25 @@ ok:
// ../../../../runtime/type.go:/structType
// for security, only the exported fields.
case TSTRUCT:
+
+ // omitFieldForAwfulBoringCryptoKludge reports whether
+ // the field t should be omitted from the reflect data.
+ // In the crypto/... packages we omit an unexported field
+ // named "boring", to keep from breaking client code that
+ // expects rsa.PublicKey etc to have only public fields.
+ // As the name suggests, this is an awful kludge, but it is
+ // limited to the dev.boringcrypto branch and avoids
+ // much more invasive effects elsewhere.
+ omitFieldForAwfulBoringCryptoKludge := func(t *types.Field) bool {
+ return strings.HasPrefix(myimportpath, "crypto/") && t.Sym != nil && t.Sym.Name == "boring"
+ }
+
n := 0
for _, t1 := range t.Fields().Slice() {
+ if omitFieldForAwfulBoringCryptoKludge(t1) {
+ continue
+ }
dtypesym(t1.Type)
n++
}
@@ -1342,6 +1358,9 @@ ok:
ot = dextratype(s, ot, t, dataAdd)
for _, f := range t.Fields().Slice() {
+ if omitFieldForAwfulBoringCryptoKludge(f) {
+ continue
+ }
// ../../../../runtime/type.go:/structField
ot = dnameField(s, ot, pkg, f)
ot = dsymptr(s, ot, dtypesym(f.Type), 0)
diff --git a/src/crypto/hmac/hmac_test.go b/src/crypto/hmac/hmac_test.go
index aac9aa96a8..444978001c 100644
--- a/src/crypto/hmac/hmac_test.go
+++ b/src/crypto/hmac/hmac_test.go
@@ -518,6 +518,31 @@ var hmacTests = []hmacTest{
sha512.Size,
sha512.BlockSize,
},
+ // HMAC without key is dumb but should probably not fail.
+ {
+ sha1.New,
+ []byte{},
+ []byte("message"),
+ "d5d1ed05121417247616cfc8378f360a39da7cfa",
+ sha1.Size,
+ sha1.BlockSize,
+ },
+ {
+ sha256.New,
+ []byte{},
+ []byte("message"),
+ "eb08c1f56d5ddee07f7bdf80468083da06b64cf4fac64fe3a90883df5feacae4",
+ sha256.Size,
+ sha256.BlockSize,
+ },
+ {
+ sha512.New,
+ []byte{},
+ []byte("message"),
+ "08fce52f6395d59c2a3fb8abb281d74ad6f112b9a9c787bcea290d94dadbc82b2ca3e5e12bf2277c7fedbb0154d5493e41bb7459f63c8e39554ea3651b812492",
+ sha512.Size,
+ sha512.BlockSize,
+ },
}
func TestHMAC(t *testing.T) {
diff --git a/src/crypto/rsa/boring_test.go b/src/crypto/rsa/boring_test.go
new file mode 100644
index 0000000000..7fbafee16e
--- /dev/null
+++ b/src/crypto/rsa/boring_test.go
@@ -0,0 +1,40 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package rsa
+
+import (
+ "crypto/rand"
+ "encoding/asn1"
+ "reflect"
+ "testing"
+ "unsafe"
+)
+
+func TestBoringASN1Marshal(t *testing.T) {
+ k, err := GenerateKey(rand.Reader, 128)
+ if err != nil {
+ t.Fatal(err)
+ }
+ // This used to fail, because of the unexported 'boring' field.
+ // Now the compiler hides it [sic].
+ _, err = asn1.Marshal(k.PublicKey)
+ if err != nil {
+ t.Fatal(err)
+ }
+}
+
+func TestBoringDeepEqual(t *testing.T) {
+ k, err := GenerateKey(rand.Reader, 128)
+ if err != nil {
+ t.Fatal(err)
+ }
+ k.boring = nil // probably nil already but just in case
+ k2 := *k
+ k2.boring = unsafe.Pointer(k) // anything not nil, for this test
+ if !reflect.DeepEqual(k, &k2) {
+ // compiler should be hiding the boring field from reflection
+ t.Fatalf("DeepEqual compared boring fields")
+ }
+}