From 4748e0884cf38774119d873bac1853a63197111b Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Thu, 22 Jan 2015 12:27:02 +0000 Subject: [PATCH] Implement sequence number checking Signed-off-by: David Woodhouse --- Makefile.am | 2 +- esp.c | 108 +++++++++++++++++++++++++++++++++++++++++ gnutls-esp.c | 45 +++++++++-------- openconnect-internal.h | 18 +++++-- 4 files changed, 148 insertions(+), 25 deletions(-) create mode 100644 esp.c diff --git a/Makefile.am b/Makefile.am index e5788739..b913bb44 100644 --- a/Makefile.am +++ b/Makefile.am @@ -25,7 +25,7 @@ openconnect_LDADD = libopenconnect.la $(LIBXML2_LIBS) $(LIBPROXY_LIBS) $(INTL_LI library_srcs = ssl.c http.c auth-common.c library.c compat.c lzs.c mainloop.c script.c ntlm.c digest.c lib_srcs_cisco = auth.c cstp.c dtls.c -lib_srcs_juniper = oncp.c gnutls-esp.c +lib_srcs_juniper = oncp.c gnutls-esp.c esp.c lib_srcs_gnutls = gnutls.c gnutls_pkcs12.c gnutls_tpm.c lib_srcs_openssl = openssl.c openssl-pkcs11.c lib_srcs_win32 = tun-win32.c sspi.c diff --git a/esp.c b/esp.c new file mode 100644 index 00000000..e8bfbd03 --- /dev/null +++ b/esp.c @@ -0,0 +1,108 @@ +/* + * OpenConnect (SSL + DTLS) VPN client + * + * Copyright © 2008-2015 Intel Corporation. + * + * Author: David Woodhouse + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public License + * version 2.1, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + */ + +#include + +#include +#include +#include +#include +#include +#include + +#include "openconnect-internal.h" + +/* Eventually we're going to have to have more than one incoming ESP + context at a time, to allow for the overlap period during a rekey. + So pass the 'esp' even though for now it's redundant. */ +int verify_packet_seqno(struct openconnect_info *vpninfo, + struct esp *esp, uint32_t seq) +{ + /* + * For incoming, esp->seq is the next *expected* packet, being + * the sequence number *after* the latest we have received. + * + * Since it must always be true that packet esp->seq-1 has been + * received, so there's no need to explicitly record that. + * + * So the backlog bitmap covers the 32 packets prior to that, + * with the LSB representing packet (esp->seq - 2), and the MSB + * representing (esp->seq - 33). A received packet is represented + * by a zero bit, and a missing packet is represented by a one. + * + * Thus we can allow out-of-order reception of packets that are + * within a reasonable interval of the latest packet received. + */ + + if (seq == esp->seq) { + /* The common case. This is the packet we expected next. */ + esp->seq_backlog <<= 1; + esp->seq++; + return 0; + } else if (seq + 33 < esp->seq) { + /* Too old. We can't know if it's a replay. */ + vpn_progress(vpninfo, PRG_DEBUG, + _("Discarding ancient ESP packet with seq %u (expected %u)\n"), + seq, esp->seq); + return -EINVAL; + } else if (seq < esp->seq) { + /* Within the backlog window, so we remember whether we've seen it or not. */ + uint32_t mask = 1 << (esp->seq - seq - 2); + + if (esp->seq_backlog & mask) { + vpn_progress(vpninfo, PRG_TRACE, + _("Accepting out-of-order ESP packet with seq %u (expected %u)\n"), + seq, esp->seq); + esp->seq_backlog &= ~mask; + return 0; + } + vpn_progress(vpninfo, PRG_DEBUG, + _("Discarding replayed ESP packet with seq %u\n"), + seq); + return -EINVAL; + } else { + /* The packet we were expecting has gone missing; this one is newer. */ + int delta = seq - esp->seq; + + if (delta >= 32) { + /* We jumped a long way into the future. We have not seen + * any of the previous 32 packets so set the backlog bitmap + * to all ones. */ + esp->seq_backlog = 0xffffffff; + } else if (delta == 31) { + /* Avoid undefined behaviour that shifting by 32 would incur. + * The (clear) top bit represents the packet which is currently + * esp->seq - 1, which we know was already received. */ + esp->seq_backlog = 0x7fffffff; + } else { + /* We have missed (delta) packets. Shift the backlog by that + * amount *plus* the one we would have shifted it anyway if + * we'd received the packet we were expecting. The zero bit + * representing the packet which is currently esp->seq - 1, + * which we know has been received, ends up at bit position + * (1<seq_backlog <<= delta + 1; + esp->seq_backlog |= (1<seq); + esp->seq = seq + 1; + return 0; + } +} diff --git a/gnutls-esp.c b/gnutls-esp.c index b6c1ef06..9c2a8b47 100644 --- a/gnutls-esp.c +++ b/gnutls-esp.c @@ -25,9 +25,6 @@ #include #include -#include -#include - #include "openconnect-internal.h" void destroy_esp_ciphers(struct esp *esp) @@ -76,6 +73,7 @@ static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp, destroy_esp_ciphers(esp); } esp->seq = 0; + esp->seq_backlog = 0; return 0; } @@ -137,24 +135,21 @@ int setup_esp_keys(struct openconnect_info *vpninfo) int decrypt_and_queue_esp_packet(struct openconnect_info *vpninfo, unsigned char *esp, int len) { + struct esp_hdr *hdr = (void *)esp; struct pkt *pkt; unsigned char hmac_buf[20]; - const int ivsize = sizeof(pkt->esp.iv); + int err; - if (len < 20 + ivsize) + if (len < sizeof(*hdr) + 12) return -EINVAL; - if (memcmp(esp, vpninfo->esp_in.spi, 4)) { + if (memcmp(hdr->spi, vpninfo->esp_in.spi, 4)) { vpn_progress(vpninfo, PRG_DEBUG, _("Received ESP packet with invalid SPI %02x%02x%02x%02x\n"), esp[0], esp[1], esp[2], esp[3]); return -EINVAL; } - /* XXX: Implement seq checking. Record the highest seq# received, and keep - a small bitmap covering a sliding window just before that, so out-of-order - packets can be accepted within reason but each packet only once. */ - gnutls_hmac(vpninfo->esp_in.hmac, esp, len - 12); gnutls_hmac_output(vpninfo->esp_in.hmac, hmac_buf); if (memcmp(hmac_buf, esp + len - 12, 12)) { @@ -163,18 +158,28 @@ int decrypt_and_queue_esp_packet(struct openconnect_info *vpninfo, unsigned char return -EINVAL; } - gnutls_cipher_set_iv(vpninfo->esp_in.cipher, esp + 8, ivsize); + /* Why in $DEITY's name would you ever *not* set this? Perhaps we + * should do th check anyway, but only warn instead of discarding + * the packet? */ + if (vpninfo->esp_replay_protect && + verify_packet_seqno(vpninfo, &vpninfo->esp_in, ntohl(hdr->seq))) + return -EINVAL; - len -= 20 + ivsize; + gnutls_cipher_set_iv(vpninfo->esp_in.cipher, hdr->iv, sizeof(hdr->iv)); + + len -= sizeof(*hdr) + 12; pkt = malloc(sizeof(struct pkt) + len); if (!pkt) return -ENOMEM; - if (gnutls_cipher_decrypt2(vpninfo->esp_in.cipher, - pkt->data + 8 + ivsize, len, - pkt->data, len)) { - printf("decrypt fail\n"); + err = gnutls_cipher_decrypt2(vpninfo->esp_in.cipher, + hdr->payload, len, + pkt->data, len); + if (err) { + vpn_progress(vpninfo, PRG_ERR, + _("Decrypting ESP packet failed: %s\n"), + gnutls_strerror(err)); return -EINVAL; } @@ -186,10 +191,12 @@ int decrypt_and_queue_esp_packet(struct openconnect_info *vpninfo, unsigned char return -EINVAL; } if (len <= 2 + pkt->data[len - 2]) { - printf("Invalid padding length %02x in ESP\n", - pkt->data[len - 2]); + vpn_progress(vpninfo, PRG_ERR, + _("Invalid padding length %02x in ESP\n"), + pkt->data[len - 2]); return -EINVAL; } + /* XXX: Actually check the padding bytes too. */ pkt->len = len - 2 + pkt->data[len - 2]; queue_packet(&vpninfo->incoming_queue, pkt); @@ -204,7 +211,7 @@ int encrypt_esp_packet(struct openconnect_info *vpninfo, struct pkt *pkt) /* This gets much more fun if the IV is variable-length */ memcpy(pkt->esp.spi, vpninfo->esp_out.spi, 4); - pkt->esp.seq = vpninfo->esp_out.seq++; + pkt->esp.seq = htonl(vpninfo->esp_out.seq++); err = gnutls_rnd(GNUTLS_RND_RANDOM, pkt->esp.iv, sizeof(pkt->esp.iv)); if (err) { vpn_progress(vpninfo, PRG_ERR, diff --git a/openconnect-internal.h b/openconnect-internal.h index 1560a076..b351f6a1 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -118,15 +118,18 @@ /****************************************************************************/ +struct esp_hdr { + unsigned char spi[4]; + uint32_t seq; + unsigned char iv[16]; + unsigned char payload[]; +}; + struct pkt { int len; struct pkt *next; union { - struct { - unsigned char spi[4]; - uint32_t seq; - unsigned char iv[16]; - } esp; + struct esp_hdr esp; struct { unsigned char oncp_pad[2]; unsigned char oncp_hdr[22]; @@ -253,6 +256,7 @@ struct esp { #error No OpenSSL support for ESP yet #endif uint32_t seq; + uint32_t seq_backlog; unsigned char spi[4]; unsigned char secrets[0x40]; }; @@ -758,6 +762,10 @@ void openconnect_clear_cookies(struct openconnect_info *vpninfo); int load_pkcs11_key(struct openconnect_info *vpninfo); int load_pkcs11_certificate(struct openconnect_info *vpninfo); +/* esp.c */ +int verify_packet_seqno(struct openconnect_info *vpninfo, + struct esp *esp, uint32_t seq); + /* gnutls-esp.c */ int setup_esp_keys(struct openconnect_info *vpninfo); void destroy_esp_ciphers(struct esp *esp);