Use constant time memcmp() when comparing packet HMACs.

This eliminates a timing side channel vulnerability, which could
potentially allow an attacker to compute a valid HMAC, and insert arbitrary
ciphertext data into the connection. If an attacker also identifies packets
with a plaintext it can guess (e.g. small packets every 1s are probably
pings), the attacker can xor the ciphertext to mangle the packet to
arbitrary plaintext. Although this type of attack is rarely seen in the
wild, it is generally considered technically viable.

Signed-off-by: Steffan Karger <steffan@karger.me>

diff --git a/src/net_packet.c b/src/net_packet.c
index 905b944..4341cf0 100644 (file)
--- a/src/net_packet.c
+++ b/src/net_packet.c
@@ -269,7 +269,7 @@ static bool try_mac(const node_t *n, const vpn_packet_t *inpkt) {
 
        HMAC(n->indigest, n->inkey, n->inkeylength, (unsigned char *) &inpkt->seqno, inpkt->len - n->inmaclength, (unsigned char *)hmac, NULL);
 
 
        HMAC(n->indigest, n->inkey, n->inkeylength, (unsigned char *) &inpkt->seqno, inpkt->len - n->inmaclength, (unsigned char *)hmac, NULL);
 

-       return !memcmp(hmac, (char *) &inpkt->seqno + inpkt->len - n->inmaclength, n->inmaclength);
+       return !memcmp_constant_time(hmac, (char *) &inpkt->seqno + inpkt->len - n->inmaclength, n->inmaclength);

 }
 
 static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) {
 }
 
 static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) {


@@ -302,7 +302,7 @@ static void receive_udppacket(node_t *n, vpn_packet_t *inpkt) {
                HMAC(n->indigest, n->inkey, n->inkeylength,
                         (unsigned char *) &inpkt->seqno, inpkt->len, (unsigned char *)hmac, NULL);
 
                HMAC(n->indigest, n->inkey, n->inkeylength,
                         (unsigned char *) &inpkt->seqno, inpkt->len, (unsigned char *)hmac, NULL);
 

-               if(memcmp(hmac, (char *) &inpkt->seqno + inpkt->len, n->inmaclength)) {
+               if(memcmp_constant_time(hmac, (char *) &inpkt->seqno + inpkt->len, n->inmaclength)) {

                        ifdebug(TRAFFIC) logger(LOG_DEBUG, "Got unauthenticated packet from %s (%s)",
                                           n->name, n->hostname);
                        return;
                        ifdebug(TRAFFIC) logger(LOG_DEBUG, "Got unauthenticated packet from %s (%s)",
                                           n->name, n->hostname);
                        return;

diff --git a/src/utils.c b/src/utils.c
index 3b221f5..c629ae0 100644 (file)
--- a/src/utils.c
+++ b/src/utils.c
@@ -78,3 +78,18 @@ unsigned int bitfield_to_int(const void *bitfield, size_t size) {
        memcpy(&value, bitfield, size);
        return value;
 }
        memcpy(&value, bitfield, size);
        return value;
 }

+
+/**
+ * As memcmp(), but constant-time.
+ * Returns 0 when data is equal, non-zero otherwise.
+ */
+int memcmp_constant_time (const void *a, const void *b, size_t size) {
+  const uint8_t *a1 = a, *b1 = b;
+  int ret = 0;
+  size_t i;
+
+  for (i = 0; i < size; i++)
+      ret |= *a1++ ^ *b1++;
+
+  return ret;
+}

diff --git a/src/utils.h b/src/utils.h
index 2e4fcf8..7ab29d2 100644 (file)
--- a/src/utils.h
+++ b/src/utils.h
@@ -42,4 +42,6 @@ extern const char *winerror(int);
 
 extern unsigned int bitfield_to_int(const void *bitfield, size_t size);
 
 
 extern unsigned int bitfield_to_int(const void *bitfield, size_t size);
 

+int memcmp_constant_time (const void *a, const void *b, size_t size);
+

 #endif                                                 /* __TINC_UTILS_H__ */
 #endif                                                 /* __TINC_UTILS_H__ */