Skip to content

Commit

Permalink
[dev.boringcrypto] cmd/compile: hide new boring fields from reflection
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
rsc committed Aug 30, 2017
1 parent 81b9d73 commit 7b49445
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 0 deletions.
19 changes: 19 additions & 0 deletions src/cmd/compile/internal/gc/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -1275,9 +1275,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++
}
Expand Down Expand Up @@ -1305,6 +1321,9 @@ ok:
ot = dextratype(lsym, ot, t, dataAdd)

for _, f := range t.Fields().Slice() {
if omitFieldForAwfulBoringCryptoKludge(f) {
continue
}
// ../../../../runtime/type.go:/structField
ot = dnameField(lsym, ot, pkg, f)
ot = dsymptr(lsym, ot, dtypesym(f.Type).Linksym(), 0)
Expand Down
40 changes: 40 additions & 0 deletions src/crypto/rsa/boring_test.go
Original file line number Diff line number Diff line change
@@ -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")
}
}

0 comments on commit 7b49445

Please sign in to comment.