add test for GOEXPERIMENT=boringcrypto (#861)

* add test for GOEXPERIMENT=boringcrypto

* fix NebulaCertificate.Sign

Set the PublicKey field in a more compatible way for the tests. The
current method grabs the public key from the certificate, but the
correct thing to do is to derive it from the private key. Either way
doesn't really matter as I don't think the Sign method actually even
uses the PublicKey field.

* assert boring

* cleanup tests

Author: wadey

.github/workflows/test.yml

@@ -52,6 +52,36 @@ jobs:
         path: e2e/mermaid/
         if-no-files-found: warn
 
+  test-linux-boringcrypto:
+    name: Build and test on linux with boringcrypto
+    runs-on: ubuntu-latest
+    steps:
+
+    - name: Set up Go 1.20
+      uses: actions/setup-go@v2
+      with:
+        go-version: "1.20"
+      id: go
+
+    - name: Check out code into the Go module directory
+      uses: actions/checkout@v2
+
+    - uses: actions/cache@v2
+      with:
+        path: ~/go/pkg/mod
+        key: ${{ runner.os }}-go1.20-${{ hashFiles('**/go.sum') }}
+        restore-keys: |
+          ${{ runner.os }}-go1.20-
+
+    - name: Build
+      run: make bin-boringcrypto
+
+    - name: Test
+      run: make test-boringcrypto
+
+    - name: End 2 end
+      run: make e2evv GOEXPERIMENT=boringcrypto CGO_ENABLED=1
+
   test:
     name: Build and test on ${{ matrix.os }}
     runs-on: ${{ matrix.os }}

Makefile

@@ -142,6 +142,9 @@ vet:
 test:
 	go test -v ./...
 
+test-boringcrypto:
+	GOEXPERIMENT=boringcrypto CGO_ENABLED=1 go test -v ./...
+
 test-cov-html:
 	go test -coverprofile=coverage.out
 	go tool cover -html=coverage.out

cert/cert.go

@@ -522,15 +522,15 @@ func (nc *NebulaCertificate) Sign(curve Curve, key []byte) error {
 		signer := ed25519.PrivateKey(key)
 		sig = ed25519.Sign(signer, b)
 	case Curve_P256:
-		x, y := elliptic.Unmarshal(elliptic.P256(), nc.Details.PublicKey)
 		signer := &ecdsa.PrivateKey{
 			PublicKey: ecdsa.PublicKey{
 				Curve: elliptic.P256(),
-				X:     x, Y: y,
 			},
 			// ref: https://github.com/golang/go/blob/go1.19/src/crypto/x509/sec1.go#L95
 			D: new(big.Int).SetBytes(key),
 		}
+		// ref: https://github.com/golang/go/blob/go1.19/src/crypto/x509/sec1.go#L119
+		signer.X, signer.Y = signer.Curve.ScalarBaseMult(key)
 
 		// We need to hash first for ECDSA
 		// - https://pkg.go.dev/crypto/ecdsa#SignASN1

noiseutil/boring_test.go

@@ -4,14 +4,21 @@
 package noiseutil
 
 import (
+	"crypto/boring"
 	"encoding/hex"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
+func TestEncryptLockNeeded(t *testing.T) {
+	assert.True(t, EncryptLockNeeded)
+}
+
 // Ensure NewGCMTLS validates the nonce is non-repeating
 func TestNewGCMTLS(t *testing.T) {
+	assert.True(t, boring.Enabled())
+
 	// Test Case 16 from GCM Spec:
 	//  - (now dead link): http://csrc.nist.gov/groups/ST/toolkit/BCM/documents/proposedmodes/gcm/gcm-spec.pdf
 	//  - as listed in boringssl tests: https://github.com/google/boringssl/blob/fips-20220613/crypto/cipher_extra/test/cipher_tests.txt#L412-L418

noiseutil/notboring_test.go

@@ -4,12 +4,11 @@
 package noiseutil
 
 import (
-	// NOTE: We have to force these imports here or boring_test.go fails to
-	// compile correctly. This seems to be a Go bug:
-	//
-	//     $ GOEXPERIMENT=boringcrypto go test ./noiseutil
-	//     # github.com/slackhq/nebula/noiseutil
-	//     boring_test.go:10:2: cannot find package
-
-	_ "github.com/stretchr/testify/assert"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
+
+func TestEncryptLockNeeded(t *testing.T) {
+	assert.False(t, EncryptLockNeeded)
+}