diff --git a/connection_state.go b/connection_state.go index 2a7be15..4f8a577 100644 --- a/connection_state.go +++ b/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) } diff --git a/inside.go b/inside.go index 21e2ab7..457fcac 100644 --- a/inside.go +++ b/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). diff --git a/noiseutil/boring.go b/noiseutil/boring.go new file mode 100644 index 0000000..e9ad19b --- /dev/null +++ b/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) +} diff --git a/noiseutil/boring_test.go b/noiseutil/boring_test.go new file mode 100644 index 0000000..bc5ff50 --- /dev/null +++ b/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) + }) +} diff --git a/noiseutil/notboring.go b/noiseutil/notboring.go new file mode 100644 index 0000000..be746f4 --- /dev/null +++ b/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 diff --git a/noiseutil/notboring_test.go b/noiseutil/notboring_test.go new file mode 100644 index 0000000..a27dbbd --- /dev/null +++ b/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" +)