Permalink
Browse files

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>
  • Loading branch information...
juniorz authored and agl committed Jul 28, 2015
1 parent d5c5f17 commit aedad9a179ec1ea11b7064c57cbc6dc30d7724ec
Showing with 69 additions and 15 deletions.
  1. +4 −2 otr/otr.go
  2. +65 −13 otr/otr_test.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
}
@@ -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")
}
}
}

0 comments on commit aedad9a

Please sign in to comment.