From 31ed9269d7976b287c3a322deaaf5c32d3a3a08c Mon Sep 17 00:00:00 2001 From: Wade Simmons Date: Mon, 8 May 2023 13:27:01 -0400 Subject: [PATCH] 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 --- .github/workflows/test.yml | 30 ++++++++++++++++++++++++++++++ Makefile | 3 +++ cert/cert.go | 4 ++-- noiseutil/boring_test.go | 7 +++++++ noiseutil/notboring_test.go | 13 ++++++------- 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 71e68ab..05aff78 100644 --- a/.github/workflows/test.yml +++ b/.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 }} diff --git a/Makefile b/Makefile index 512fdc2..68f5ca7 100644 --- a/Makefile +++ b/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 diff --git a/cert/cert.go b/cert/cert.go index 04cde59..c344292 100644 --- a/cert/cert.go +++ b/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 diff --git a/noiseutil/boring_test.go b/noiseutil/boring_test.go index bc5ff50..8c88439 100644 --- a/noiseutil/boring_test.go +++ b/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 diff --git a/noiseutil/notboring_test.go b/noiseutil/notboring_test.go index a27dbbd..b865391 100644 --- a/noiseutil/notboring_test.go +++ b/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 + "testing" - _ "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" ) + +func TestEncryptLockNeeded(t *testing.T) { + assert.False(t, EncryptLockNeeded) +}