otr: Fix revealing MAC keys when a DH key is rotated

The existing implementation did not save all the fields in keySlot, which not
only caused a cache miss on every calcDataKeys() but also caused the rotate keys
functions to not find the MAC keys that should be revealed.

It also stops revealing the sending MAC keys. The finite-state analysis of the
otr v2 spec[1] revealed an attack on message integrity when sending MAC keys are
revealed. The spec had been updated accordingly [2].

1 - http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.165.7945&rep=rep1&type=pdf
2 - http://sourceforge.net/p/otr/libotr/ci/58fd90cb77c836ff9fa762e91d2b2becc6d5aae8/

Change-Id: Iee36205994ebdb27d8c890ae25fd9981326401df
Reviewed-on: https://go-review.googlesource.com/12781
Reviewed-by: Adam Langley <agl@golang.org>

otr/otr.go

@@ -848,7 +848,6 @@ func (c *Conversation) rotateDHKeys() {
 		slot := &c.keySlots[i]
 		if slot.used && slot.myKeyId == c.myKeyId-1 {
 			slot.used = false
-			c.oldMACs = append(c.oldMACs, slot.sendMACKey...)
 			c.oldMACs = append(c.oldMACs, slot.recvMACKey...)
 		}
 	}
@@ -924,7 +923,6 @@ func (c *Conversation) processData(in []byte) (out []byte, tlvs []tlv, err error
 			slot := &c.keySlots[i]
 			if slot.used && slot.theirKeyId == theirKeyId-1 {
 				slot.used = false
-				c.oldMACs = append(c.oldMACs, slot.sendMACKey...)
 				c.oldMACs = append(c.oldMACs, slot.recvMACKey...)
 			}
 		}
@@ -1096,6 +1094,10 @@ func (c *Conversation) calcDataKeys(myKeyId, theirKeyId uint32) (slot *keySlot,
 	h.Write(slot.recvAESKey)
 	slot.recvMACKey = h.Sum(slot.recvMACKey[:0])
 
+	slot.theirKeyId = theirKeyId
+	slot.myKeyId = myKeyId
+	slot.used = true
+
 	zero(slot.theirLastCtr[:])
 	return
 }

otr/otr_test.go

@@ -194,23 +194,75 @@ func TestConversation(t *testing.T) {
 		t.Error("Bob doesn't believe that the conversation is secure")
 	}
 
-	var testMessage = []byte("hello Bob")
-	alicesMessage, err = alice.Send(testMessage)
-	for i, msg := range alicesMessage {
-		out, encrypted, _, _, err := bob.Receive(msg)
-		if err != nil {
-			t.Errorf("Error generated while processing test message: %s", err.Error())
+	var testMessages = [][]byte{
+		[]byte("hello"), []byte("bye"),
+	}
+
+	for j, testMessage := range testMessages {
+		alicesMessage, err = alice.Send(testMessage)
+
+		if len(alice.oldMACs) != 0 {
+			t.Errorf("Alice has not revealed all MAC keys")
 		}
-		if len(out) > 0 {
-			if i != len(alicesMessage)-1 {
-				t.Fatal("Bob produced a message while processing a fragment of Alice's")
+
+		for i, msg := range alicesMessage {
+			out, encrypted, _, _, err := bob.Receive(msg)
+
+			if err != nil {
+				t.Errorf("Error generated while processing test message: %s", err.Error())
 			}
-			if !encrypted {
-				t.Errorf("Message was not marked as encrypted")
+			if len(out) > 0 {
+				if i != len(alicesMessage)-1 {
+					t.Fatal("Bob produced a message while processing a fragment of Alice's")
+				}
+				if !encrypted {
+					t.Errorf("Message was not marked as encrypted")
+				}
+				if !bytes.Equal(out, testMessage) {
+					t.Errorf("Message corrupted: got %x, want %x", out, testMessage)
+				}
+			}
+		}
+
+		if j == 0 {
+			if len(bob.oldMACs) != 0 {
+				t.Errorf("Bob should not have MAC keys to reveal")
 			}
-			if !bytes.Equal(out, testMessage) {
-				t.Errorf("Message corrupted: got %x, want %x", out, testMessage)
+		} else if len(bob.oldMACs) != 40 {
+			t.Errorf("Bob does not have MAC keys to reveal")
+		}
+
+		bobsMessage, err = bob.Send(testMessage)
+
+		if len(bob.oldMACs) != 0 {
+			t.Errorf("Bob has not revealed all MAC keys")
+		}
+
+		for i, msg := range bobsMessage {
+			out, encrypted, _, _, err := alice.Receive(msg)
+
+			if err != nil {
+				t.Errorf("Error generated while processing test message: %s", err.Error())
+			}
+			if len(out) > 0 {
+				if i != len(bobsMessage)-1 {
+					t.Fatal("Alice produced a message while processing a fragment of Bob's")
+				}
+				if !encrypted {
+					t.Errorf("Message was not marked as encrypted")
+				}
+				if !bytes.Equal(out, testMessage) {
+					t.Errorf("Message corrupted: got %x, want %x", out, testMessage)
+				}
+			}
+		}
+
+		if j == 0 {
+			if len(alice.oldMACs) != 20 {
+				t.Errorf("Alice does not have MAC keys to reveal")
 			}
+		} else if len(alice.oldMACs) != 40 {
+			t.Errorf("Alice does not have MAC keys to reveal")
 		}
 	}
 }