store length of ESP encryption and HMAC keys so that they can be manipulated separate...

David Woodhouse wrote:
> Daniel Lenski wrote:
>> -       unsigned char secrets[0x40];
>> +       unsigned char secrets[0x40]; /* Encryption key bytes, then HMAC key bytes */
>
> You're allowed to object to that horridness and split it into two
> separate fields for the encryption and HMAC keys, instead of just
> documenting it.
>
> In fact, one might argue that would be the better approach...

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
Signed-off-by: David Woodhouse <dwmw2@infradead.org>

diff --git a/esp.c b/esp.c
index 57ff6cf167f1a82830bc5113c35059b18ddc0bdd..5b038c454416e77b3c3bb0dba5b7cdc711d1281e 100644 (file)
--- a/esp.c
+++ b/esp.c
@@ -32,16 +32,13 @@ int print_esp_keys(struct openconnect_info *vpninfo, const char *name, struct es
        int i;
        const char *enctype, *mactype;
        char enckey[256], mackey[256];
-       int enclen, maclen;
 
        switch(vpninfo->esp_enc) {
        case 0x02:
                enctype = "AES-128-CBC (RFC3602)";
-               enclen = 16;
                break;
        case 0x05:
                enctype = "AES-256-CBC (RFC3602)";
-               enclen = 32;
                break;
        default:
                return -EINVAL;
@@ -49,20 +46,18 @@ int print_esp_keys(struct openconnect_info *vpninfo, const char *name, struct es
        switch(vpninfo->esp_hmac) {
        case 0x01:
                mactype = "HMAC-MD5-96 (RFC2403)";
-               maclen = 16;
                break;
        case 0x02:
                mactype = "HMAC-SHA-1-96 (RFC2404)";
-               maclen = 20;
                break;
        default:
                return -EINVAL;
        }
 
-       for (i = 0; i < enclen; i++)
-               sprintf(enckey + (2 * i), "%02x", esp->secrets[i]);
-       for (i = 0; i < maclen; i++)
-               sprintf(mackey + (2 * i), "%02x", esp->secrets[enclen + i]);
+       for (i = 0; i < vpninfo->enc_key_len; i++)
+               sprintf(enckey + (2 * i), "%02x", esp->enc_key[i]);
+       for (i = 0; i < vpninfo->hmac_key_len; i++)
+               sprintf(mackey + (2 * i), "%02x", esp->hmac_key[i]);
 
        vpn_progress(vpninfo, PRG_TRACE,
                     _("Parameters for %s ESP: SPI 0x%08x\n"),

diff --git a/gnutls-esp.c b/gnutls-esp.c
index 1ad4e607239e90117f8bc98686f17fbee5e70964..f3fd499c2b1f8865da475acb68b5034e78777b68 100644 (file)
--- a/gnutls-esp.c
+++ b/gnutls-esp.c
@@ -48,7 +48,7 @@ static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp,
        destroy_esp_ciphers(esp);
 
        enc_key.size = gnutls_cipher_get_key_size(encalg);
-       enc_key.data = esp->secrets;
+       enc_key.data = esp->enc_key;
 
        err = gnutls_cipher_init(&esp->cipher, encalg, &enc_key, NULL);
        if (err) {
@@ -59,7 +59,7 @@ static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp,
        }
 
        err = gnutls_hmac_init(&esp->hmac, macalg,
-                              esp->secrets + enc_key.size,
+                              esp->hmac_key,
                               gnutls_hmac_get_len(macalg));
        if (err) {
                vpn_progress(vpninfo, PRG_ERR,
@@ -111,7 +111,8 @@ int setup_esp_keys(struct openconnect_info *vpninfo)
        esp_in = &vpninfo->esp_in[vpninfo->current_esp_in];
 
        if ((ret = gnutls_rnd(GNUTLS_RND_NONCE, &esp_in->spi, sizeof(esp_in->spi))) ||
-           (ret = gnutls_rnd(GNUTLS_RND_RANDOM, &esp_in->secrets, sizeof(esp_in->secrets)))) {
+                   (ret = gnutls_rnd(GNUTLS_RND_RANDOM, &esp_in->enc_key, vpninfo->enc_key_len)) ||
+                   (ret = gnutls_rnd(GNUTLS_RND_RANDOM, &esp_in->hmac_key, vpninfo->hmac_key_len)) ) {
                vpn_progress(vpninfo, PRG_ERR,
                             _("Failed to generate random keys for ESP: %s\n"),
                             gnutls_strerror(ret));

diff --git a/oncp.c b/oncp.c
index 5bd8b9e03adf32d699d01834e55b99426a7fbd9b..0155f416e4e04a49113002a9dfc11424adeeed6b 100644 (file)
--- a/oncp.c
+++ b/oncp.c
@@ -302,11 +302,13 @@ static int process_attr(struct openconnect_info *vpninfo, int group, int attr,
 
                if (attrlen != 1)
                        goto badlen;
-               if (data[0] == 0x02)
+               if (data[0] == ENC_AES_128_CBC) {
                        enctype = "AES-128";
-               else if (data[0] == 0x05)
+                       vpninfo->enc_key_len = 16;
+               } else if (data[0] == ENC_AES_256_CBC) {
                        enctype = "AES-256";
-               else
+                       vpninfo->enc_key_len = 32;
+               } else
                        enctype = "unknown";
                vpn_progress(vpninfo, PRG_DEBUG, _("ESP encryption: 0x%02x (%s)\n"),
                              data[0], enctype);
@@ -319,11 +321,13 @@ static int process_attr(struct openconnect_info *vpninfo, int group, int attr,
 
                if (attrlen != 1)
                        goto badlen;
-               if (data[0] == 0x01)
+               if (data[0] == HMAC_MD5) {
                        mactype = "MD5";
-               else if (data[0] == 0x02)
+                       vpninfo->hmac_key_len = 16;
+               } else if (data[0] == HMAC_SHA1) {
                        mactype = "SHA1";
-               else
+                       vpninfo->hmac_key_len = 20;
+               } else
                        mactype = "unknown";
                vpn_progress(vpninfo, PRG_DEBUG, _("ESP HMAC: 0x%02x (%s)\n"),
                              data[0], mactype);
@@ -389,7 +393,8 @@ static int process_attr(struct openconnect_info *vpninfo, int group, int attr,
        case GRP_ATTR(7, 2):
                if (attrlen != 0x40)
                        goto badlen;
-               memcpy(vpninfo->esp_out.secrets, data, 0x40);
+               /* data contains enc_key and hmac_key concatenated */
+               memcpy(vpninfo->esp_out.enc_key, data, 0x40);
                vpn_progress(vpninfo, PRG_DEBUG, _("%d bytes of ESP secrets\n"),
                             attrlen);
                break;
@@ -490,6 +495,7 @@ static int parse_conf_pkt(struct openconnect_info *vpninfo, unsigned char *bytes
 {
        int kmplen, kmpend, grouplen, groupend, group, attr, attrlen;
        int ofs = 0;
+       int split_enc_hmac_keys = 0;
 
        kmplen = load_be16(bytes + ofs + 18);
        kmpend = ofs + kmplen;
@@ -533,12 +539,21 @@ static int parse_conf_pkt(struct openconnect_info *vpninfo, unsigned char *bytes
                                goto eparse;
                        if (process_attr(vpninfo, group, attr, bytes + ofs, attrlen))
                                goto eparse;
+                       if (GRP_ATTR(group, attr)==GRP_ATTR(7, 2))
+                               split_enc_hmac_keys = 1;
                        ofs += attrlen;
                }
        }
+
+       /* The encryption and HMAC keys are sent concatenated together in a block of 0x40 bytes;
+          we can't split them apart until we know how long the encryption key is. */
+       if (split_enc_hmac_keys)
+               memcpy(vpninfo->esp_out.hmac_key, vpninfo->esp_out.enc_key + vpninfo->enc_key_len, vpninfo->hmac_key_len);
+
        return 0;
 }
 
+
 int oncp_connect(struct openconnect_info *vpninfo)
 {
        int ret, len, kmp, kmplen, group;
@@ -786,7 +801,8 @@ int oncp_connect(struct openconnect_info *vpninfo)
                buf_append_bytes(reqbuf, esp_kmp_hdr, sizeof(esp_kmp_hdr));
                buf_append_bytes(reqbuf, &esp->spi, sizeof(esp->spi));
                buf_append_bytes(reqbuf, esp_kmp_part2, sizeof(esp_kmp_part2));
-               buf_append_bytes(reqbuf, &esp->secrets, sizeof(esp->secrets));
+               buf_append_bytes(reqbuf, &esp->enc_key, vpninfo->enc_key_len);
+               buf_append_bytes(reqbuf, &esp->hmac_key, 0x40 - vpninfo->enc_key_len);
                if (buf_error(reqbuf)) {
                        vpn_progress(vpninfo, PRG_ERR,
                                     _("Error negotiating ESP keys\n"));
@@ -840,8 +856,9 @@ static int oncp_receive_espkeys(struct openconnect_info *vpninfo, int len)
                p += sizeof(esp->spi);
                memcpy(p, esp_kmp_part2, sizeof(esp_kmp_part2));
                p += sizeof(esp_kmp_part2);
-               memcpy(p, esp->secrets, sizeof(esp->secrets));
-               p += sizeof(esp->secrets);
+               memcpy(p, esp->enc_key, vpninfo->enc_key_len);
+               memcpy(p+vpninfo->enc_key_len, esp->hmac_key, 0x40 - vpninfo->enc_key_len);
+               p += 0x40;
                vpninfo->cstp_pkt->len = p - vpninfo->cstp_pkt->data;
                store_le16(vpninfo->cstp_pkt->oncp.rec,
                           (p - vpninfo->cstp_pkt->oncp.kmp));

diff --git a/openconnect-internal.h b/openconnect-internal.h
index 4aac2ce6aa75bee889f31187d25ada8eae00ce76..67b73f46506f6bb61236b6c5542121c719091a14 100644 (file)
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -338,7 +338,8 @@ struct esp {
        uint64_t seq_backlog;
        uint64_t seq;
        uint32_t spi; /* Stored network-endian */
-       unsigned char secrets[0x40];
+       unsigned char enc_key[0x40]; /* Encryption key */
+       unsigned char hmac_key[0x40]; /* HMAC key */
 };
 
 struct openconnect_info {
@@ -362,6 +363,8 @@ struct openconnect_info {
        int old_esp_maxseq;
        struct esp esp_in[2];
        struct esp esp_out;
+       int enc_key_len;
+       int hmac_key_len;
 
        int tncc_fd; /* For Juniper TNCC */
        const char *csd_xmltag;
@@ -690,6 +693,12 @@ struct openconnect_info {
 #define AC_PKT_COMPRESSED      8       /* Compressed data */
 #define AC_PKT_TERM_SERVER     9       /* Server kick */
 
+/* Encryption and HMAC algorithms (matching Juniper's binary encoding) */
+#define ENC_AES_128_CBC                2
+#define ENC_AES_256_CBC                5
+#define HMAC_MD5               1
+#define HMAC_SHA1              2
+
 #define vpn_progress(_v, lvl, ...) do {                                        \
        if ((_v)->verbose >= (lvl))                                     \
                (_v)->progress((_v)->cbdata, lvl, __VA_ARGS__); \

diff --git a/openssl-esp.c b/openssl-esp.c
index e20bde00292eb6ce48723041ea72204df9b05886..faba1ff433f483097cf90a9ff2aabda4f63f157d 100644 (file)
--- a/openssl-esp.c
+++ b/openssl-esp.c
@@ -99,7 +99,7 @@ static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp,
                destroy_esp_ciphers(esp);
                return -ENOMEM;
        }
-       if (!HMAC_Init_ex(esp->hmac, esp->secrets + EVP_CIPHER_key_length(encalg),
+       if (!HMAC_Init_ex(esp->hmac, esp->hmac_key,
                          EVP_MD_size(macalg), macalg, NULL)) {
                vpn_progress(vpninfo, PRG_ERR,
                             _("Failed to initialize ESP HMAC\n"));
@@ -151,7 +151,8 @@ int setup_esp_keys(struct openconnect_info *vpninfo)
        esp_in = &vpninfo->esp_in[vpninfo->current_esp_in];
 
        if (!RAND_bytes((void *)&esp_in->spi, sizeof(esp_in->spi)) ||
-           !RAND_bytes((void *)&esp_in->secrets, sizeof(esp_in->secrets))) {
+                   !RAND_bytes((void *)&esp_in->enc_key, vpninfo->enc_key_len)) ||
+                   !RAND_bytes((void *)&esp_in->hmac_key, vpninfo->hmac_key_len)) ) {
                vpn_progress(vpninfo, PRG_ERR,
                             _("Failed to generate random keys for ESP:\n"));
                openconnect_report_ssl_errors(vpninfo);