brontide: fix bug in final sender/receiver key derivation

This commit fixes a bug in our key derivation for the final step of the
key exchange. In our code we were swapping the order of the salt and
input keyeing material to the HKDF function. This was triggered by the
argument order of the golang implementation we’re currently using has
the “secret” of IKM argument first, instead of second as defined within
rfc5869.

To fix this, we simply need to swap function arguments in two places:
within the split() function and during key rotation.

This bug was discovered by Rusty Russell, thanks!

brontide/noise.go

@@ -142,7 +142,7 @@ func (c *cipherState) rotateKey() {
 	)
 
 	oldKey := c.secretKey
-	h := hkdf.New(sha256.New, c.salt[:], oldKey[:], info)
+	h := hkdf.New(sha256.New, oldKey[:], c.salt[:], info)
 
 	// hkdf(ck, k, zero)
 	// |
@@ -192,7 +192,7 @@ func (s *symmetricState) mixKey(input []byte) {
 	salt := s.chainingKey
 	h := hkdf.New(sha256.New, secret, salt[:], info)
 
-	// hkdf(input, ck, zero)
+	// hkdf(ck, input, zero)
 	// |
 	// | \
 	// |  \
@@ -535,7 +535,6 @@ func (b *BrontideMachine) GenActThree() ([ActThreeSize]byte, error) {
 	ourPubkey := b.localStatic.PubKey().SerializeCompressed()
 	ciphertext := b.EncryptAndHash(ourPubkey)
 
-
 	s := ecdh(b.remoteEphemeral, b.localStatic)
 	b.mixKey(s)
 
@@ -610,7 +609,7 @@ func (b *BrontideMachine) split() {
 		recvKey [32]byte
 	)
 
-	h := hkdf.New(sha256.New, b.chainingKey[:], empty, empty)
+	h := hkdf.New(sha256.New, empty, b.chainingKey[:], empty)
 
 	// If we're the initiator the first 32 bytes are used to encrypt our
 	// messages and the second 32-bytes to decrypt their messages. For the

brontide/noise_test.go

@@ -63,7 +63,6 @@ func establishTestConnection() (net.Conn, net.Conn, error) {
 
 	return localConn, remoteConn, nil
 }
-
 func TestConnectionCorrectness(t *testing.T) {
 	// Create a test connection, grabbing either side of the connection
 	// into local variables. If the initial crypto handshake fails, then
@@ -164,7 +163,7 @@ func TestWriteMessageChunking(t *testing.T) {
 	// size.
 	largeMessage := bytes.Repeat([]byte("kek"), math.MaxUint16*3)
 
-	// Launch a new goroutine to write the lerge message generated above in
+	// Launch a new goroutine to write the large message generated above in
 	// chunks. We spawn a new goroutine because otherwise, we may block as
 	// the kernal waits for the buffer to flush.
 	var wg sync.WaitGroup
@@ -198,7 +197,3 @@ func TestWriteMessageChunking(t *testing.T) {
 		t.Fatalf("bytes don't match")
 	}
 }
-
-func TestNoiseIdentityHiding(t *testing.T) {
-	// TODO(roasbeef): fin
-}