Use NewGCMTLS (when using experiment boringcrypto) (#803)

* Use NewGCMTLS (when using experiment boringcrypto)

This change only affects builds built using `GOEXPERIMENT=boringcrypto`.
When built with this experiment, we use the NewGCMTLS() method exposed by
goboring, which validates that the nonce is strictly monotonically increasing.
This is the TLS 1.2 specification for nonce generation (which also matches the
method used by the Noise Protocol)

- https://github.com/golang/go/blob/go1.19/src/crypto/tls/cipher_suites.go#L520-L522
- https://github.com/golang/go/blob/go1.19/src/crypto/internal/boring/aes.go#L235-L237
- https://github.com/golang/go/blob/go1.19/src/crypto/internal/boring/aes.go#L250
- https://github.com/google/boringssl/blob/ae223d6138807a13006342edfeef32e813246b39/include/openssl/aead.h#L379-L381
- https://github.com/google/boringssl/blob/ae223d6138807a13006342edfeef32e813246b39/crypto/fipsmodule/cipher/e_aes.c#L1082-L1093

* need to lock around EncryptDanger in SendVia

* fix link to test vector

connection_state.go

@@ -9,6 +9,7 @@ import (
 	"github.com/flynn/noise"
 	"github.com/sirupsen/logrus"
 	"github.com/slackhq/nebula/cert"
+	"github.com/slackhq/nebula/noiseutil"
 )
 
 const ReplayWindow = 1024
@@ -28,7 +29,7 @@ type ConnectionState struct {
 }
 
 func (f *Interface) newConnectionState(l *logrus.Logger, initiator bool, pattern noise.HandshakePattern, psk []byte, pskStage int) *ConnectionState {
-	cs := noise.NewCipherSuite(noise.DH25519, noise.CipherAESGCM, noise.HashSHA256)
+	cs := noise.NewCipherSuite(noise.DH25519, noiseutil.CipherAESGCM, noise.HashSHA256)
 	if f.cipher == "chachapoly" {
 		cs = noise.NewCipherSuite(noise.DH25519, noise.CipherChaChaPoly, noise.HashSHA256)
 	}

inside.go

@@ -6,6 +6,7 @@ import (
 	"github.com/slackhq/nebula/firewall"
 	"github.com/slackhq/nebula/header"
 	"github.com/slackhq/nebula/iputil"
+	"github.com/slackhq/nebula/noiseutil"
 	"github.com/slackhq/nebula/udp"
 )
 
@@ -256,6 +257,11 @@ func (f *Interface) SendVia(viaIfc interface{},
 ) {
 	via := viaIfc.(*HostInfo)
 	relay := relayIfc.(*Relay)
+
+	if noiseutil.EncryptLockNeeded {
+		// NOTE: for goboring AESGCMTLS we need to lock because of the nonce check
+		via.ConnectionState.writeLock.Lock()
+	}
 	c := via.ConnectionState.messageCounter.Add(1)
 
 	out = header.Encode(out, header.Version, header.Message, header.MessageRelay, relay.RemoteIndex, c)
@@ -264,6 +270,9 @@ func (f *Interface) SendVia(viaIfc interface{},
 	// Authenticate the header and payload, but do not encrypt for this message type.
 	// The payload consists of the inner, unencrypted Nebula header, as well as the end-to-end encrypted payload.
 	if len(out)+len(ad)+via.ConnectionState.eKey.Overhead() > cap(out) {
+		if noiseutil.EncryptLockNeeded {
+			via.ConnectionState.writeLock.Unlock()
+		}
 		via.logger(f.l).
 			WithField("outCap", cap(out)).
 			WithField("payloadLen", len(ad)).
@@ -285,6 +294,9 @@ func (f *Interface) SendVia(viaIfc interface{},
 
 	var err error
 	out, err = via.ConnectionState.eKey.EncryptDanger(out, out, nil, c, nb)
+	if noiseutil.EncryptLockNeeded {
+		via.ConnectionState.writeLock.Unlock()
+	}
 	if err != nil {
 		via.logger(f.l).WithError(err).Info("Failed to EncryptDanger in sendVia")
 		return
@@ -313,8 +325,10 @@ func (f *Interface) sendNoMetrics(t header.MessageType, st header.MessageSubType
 		out = out[header.Len:]
 	}
 
-	//TODO: enable if we do more than 1 tun queue
-	//ci.writeLock.Lock()
+	if noiseutil.EncryptLockNeeded {
+		// NOTE: for goboring AESGCMTLS we need to lock because of the nonce check
+		ci.writeLock.Lock()
+	}
 	c := ci.messageCounter.Add(1)
 
 	//l.WithField("trace", string(debug.Stack())).Error("out Header ", &Header{Version, t, st, 0, hostinfo.remoteIndexId, c}, p)
@@ -335,8 +349,9 @@ func (f *Interface) sendNoMetrics(t header.MessageType, st header.MessageSubType
 
 	var err error
 	out, err = ci.eKey.EncryptDanger(out, out, p, c, nb)
-	//TODO: see above note on lock
-	//ci.writeLock.Unlock()
+	if noiseutil.EncryptLockNeeded {
+		ci.writeLock.Unlock()
+	}
 	if err != nil {
 		hostinfo.logger(f.l).WithError(err).
 			WithField("udpAddr", remote).WithField("counter", c).

noiseutil/boring.go

@@ -0,0 +1,80 @@
+//go:build boringcrypto
+// +build boringcrypto
+
+package noiseutil
+
+import (
+	"crypto/aes"
+	"crypto/cipher"
+	"encoding/binary"
+
+	// unsafe needed for go:linkname
+	_ "unsafe"
+
+	"github.com/flynn/noise"
+)
+
+// EncryptLockNeeded indicates if calls to Encrypt need a lock
+// This is true for boringcrypto because the Seal function verifies that the
+// nonce is strictly increasing.
+const EncryptLockNeeded = true
+
+// NewGCMTLS is no longer exposed in go1.19+, so we need to link it in
+// See: https://github.com/golang/go/issues/56326
+//
+// NewGCMTLS is the internal method used with boringcrypto that provices a
+// validated mode of AES-GCM which enforces the nonce is strictly
+// monotonically increasing.  This is the TLS 1.2 specification for nonce
+// generation (which also matches the method used by the Noise Protocol)
+//
+// - https://github.com/golang/go/blob/go1.19/src/crypto/tls/cipher_suites.go#L520-L522
+// - https://github.com/golang/go/blob/go1.19/src/crypto/internal/boring/aes.go#L235-L237
+// - https://github.com/golang/go/blob/go1.19/src/crypto/internal/boring/aes.go#L250
+// - https://github.com/google/boringssl/blob/ae223d6138807a13006342edfeef32e813246b39/include/openssl/aead.h#L379-L381
+// - https://github.com/google/boringssl/blob/ae223d6138807a13006342edfeef32e813246b39/crypto/fipsmodule/cipher/e_aes.c#L1082-L1093
+//
+//go:linkname newGCMTLS crypto/internal/boring.NewGCMTLS
+func newGCMTLS(c cipher.Block) (cipher.AEAD, error)
+
+type cipherFn struct {
+	fn   func([32]byte) noise.Cipher
+	name string
+}
+
+func (c cipherFn) Cipher(k [32]byte) noise.Cipher { return c.fn(k) }
+func (c cipherFn) CipherName() string             { return c.name }
+
+// CipherAESGCM is the AES256-GCM AEAD cipher (using NewGCMTLS when GoBoring is present)
+var CipherAESGCM noise.CipherFunc = cipherFn{cipherAESGCMBoring, "AESGCM"}
+
+func cipherAESGCMBoring(k [32]byte) noise.Cipher {
+	c, err := aes.NewCipher(k[:])
+	if err != nil {
+		panic(err)
+	}
+	gcm, err := newGCMTLS(c)
+	if err != nil {
+		panic(err)
+	}
+	return aeadCipher{
+		gcm,
+		func(n uint64) []byte {
+			var nonce [12]byte
+			binary.BigEndian.PutUint64(nonce[4:], n)
+			return nonce[:]
+		},
+	}
+}
+
+type aeadCipher struct {
+	cipher.AEAD
+	nonce func(uint64) []byte
+}
+
+func (c aeadCipher) Encrypt(out []byte, n uint64, ad, plaintext []byte) []byte {
+	return c.Seal(out, c.nonce(n), plaintext, ad)
+}
+
+func (c aeadCipher) Decrypt(out []byte, n uint64, ad, ciphertext []byte) ([]byte, error) {
+	return c.Open(out, c.nonce(n), ciphertext, ad)
+}

noiseutil/boring_test.go

@@ -0,0 +1,39 @@
+//go:build boringcrypto
+// +build boringcrypto
+
+package noiseutil
+
+import (
+	"encoding/hex"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// Ensure NewGCMTLS validates the nonce is non-repeating
+func TestNewGCMTLS(t *testing.T) {
+	// 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
+	key, _ := hex.DecodeString("feffe9928665731c6d6a8f9467308308feffe9928665731c6d6a8f9467308308")
+	iv, _ := hex.DecodeString("cafebabefacedbaddecaf888")
+	plaintext, _ := hex.DecodeString("d9313225f88406e5a55909c5aff5269a86a7a9531534f7da2e4c303d8a318a721c3c0c95956809532fcf0e2449a6b525b16aedf5aa0de657ba637b39")
+	aad, _ := hex.DecodeString("feedfacedeadbeeffeedfacedeadbeefabaddad2")
+	expected, _ := hex.DecodeString("522dc1f099567d07f47f37a32a84427d643a8cdcbfe5c0c97598a2bd2555d1aa8cb08e48590dbb3da7b08b1056828838c5f61e6393ba7a0abcc9f662")
+	expectedTag, _ := hex.DecodeString("76fc6ece0f4e1768cddf8853bb2d551b")
+
+	expected = append(expected, expectedTag...)
+
+	var keyArray [32]byte
+	copy(keyArray[:], key)
+	c := CipherAESGCM.Cipher(keyArray)
+	aead := c.(aeadCipher).AEAD
+
+	dst := aead.Seal([]byte{}, iv, plaintext, aad)
+	assert.Equal(t, expected, dst)
+
+	// We expect this to fail since we are re-encrypting with a repeat IV
+	assert.PanicsWithError(t, "boringcrypto: EVP_AEAD_CTX_seal failed", func() {
+		dst = aead.Seal([]byte{}, iv, plaintext, aad)
+	})
+}

noiseutil/notboring.go

@@ -0,0 +1,14 @@
+//go:build !boringcrypto
+// +build !boringcrypto
+
+package noiseutil
+
+import (
+	"github.com/flynn/noise"
+)
+
+// EncryptLockNeeded indicates if calls to Encrypt need a lock
+const EncryptLockNeeded = false
+
+// CipherAESGCM is the standard noise.CipherAESGCM when boringcrypto is not enabled
+var CipherAESGCM noise.CipherFunc = noise.CipherAESGCM

noiseutil/notboring_test.go

@@ -0,0 +1,15 @@
+//go:build !boringcrypto
+// +build !boringcrypto
+
+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"
+)