From 347060a852832f622ca13784a83c7a9e7eb8d7c2 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Mon, 15 Apr 2019 13:43:06 +0100 Subject: [PATCH] Consolidate common parts of setup_esp_keys() There was a fair amount of redundancy between the OpenSSL and GnuTLS variants. Create a new common function for that instead. Signed-off-by: David Woodhouse --- esp.c | 49 +++++++++++++++++++++++++++++++++++ gnutls-esp.c | 58 ++++++++---------------------------------- gpst.c | 2 +- oncp.c | 4 +-- openconnect-internal.h | 3 ++- openssl-esp.c | 49 ++++++++--------------------------- 6 files changed, 75 insertions(+), 90 deletions(-) diff --git a/esp.c b/esp.c index 2a4d1b60..1b907435 100644 --- a/esp.c +++ b/esp.c @@ -347,3 +347,52 @@ void esp_shutdown(struct openconnect_info *vpninfo) if (vpninfo->dtls_state != DTLS_DISABLED) vpninfo->dtls_state = DTLS_NOSECRET; } + +int openconnect_setup_esp_keys(struct openconnect_info *vpninfo, int new_keys) +{ + struct esp *esp_in; + int ret; + + if (vpninfo->dtls_state == DTLS_DISABLED) + return -EOPNOTSUPP; + if (!vpninfo->dtls_addr) + return -EINVAL; + + if (new_keys) { + vpninfo->old_esp_maxseq = vpninfo->esp_in[vpninfo->current_esp_in].seq + 32; + vpninfo->current_esp_in ^= 1; + } + + esp_in = &vpninfo->esp_in[vpninfo->current_esp_in]; + + if (new_keys) { + if (openconnect_random(&esp_in->spi, sizeof(esp_in->spi)) || + openconnect_random((void *)&esp_in->enc_key, vpninfo->enc_key_len) || + openconnect_random((void *)&esp_in->hmac_key, vpninfo->hmac_key_len)) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed to generate random keys for ESP\n")); + return -EIO; + } + } + + if (openconnect_random(vpninfo->esp_out.iv, sizeof(vpninfo->esp_out.iv))) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed to generate initial IV for ESP\n")); + return -EIO; + } + + /* This is the minimum; some implementations may increase it */ + vpninfo->pkt_trailer = 17 + 16 + 20; /* 17 for pad, 16 for IV, 20 for HMAC (of which we send 12) */ + + vpninfo->esp_out.seq = vpninfo->esp_out.seq_backlog = 0; + esp_in->seq = esp_in->seq_backlog = 0; + + ret = init_esp_ciphers(vpninfo, &vpninfo->esp_out, esp_in); + if (ret) + return ret; + + if (vpninfo->dtls_state == DTLS_NOSECRET) + vpninfo->dtls_state = DTLS_SECRET; + + return 0; +} diff --git a/gnutls-esp.c b/gnutls-esp.c index d8a8d7c0..08adc115 100644 --- a/gnutls-esp.c +++ b/gnutls-esp.c @@ -39,8 +39,8 @@ void destroy_esp_ciphers(struct esp *esp) } } -static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp, - gnutls_mac_algorithm_t macalg, gnutls_cipher_algorithm_t encalg) +static int init_esp_cipher(struct openconnect_info *vpninfo, struct esp *esp, + gnutls_mac_algorithm_t macalg, gnutls_cipher_algorithm_t encalg) { gnutls_datum_t enc_key; int err; @@ -67,28 +67,20 @@ static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp, gnutls_strerror(err)); destroy_esp_ciphers(esp); } - esp->seq = 0; - esp->seq_backlog = 0; return 0; } -int setup_esp_keys(struct openconnect_info *vpninfo, int new_keys) +int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp_out, struct esp *esp_in) { - struct esp *esp_in; gnutls_mac_algorithm_t macalg; gnutls_cipher_algorithm_t encalg; int ret; - if (vpninfo->dtls_state == DTLS_DISABLED) - return -EOPNOTSUPP; - if (!vpninfo->dtls_addr) - return -EINVAL; - switch (vpninfo->esp_enc) { - case 0x02: + case ENC_AES_128_CBC: encalg = GNUTLS_CIPHER_AES_128_CBC; break; - case 0x05: + case ENC_AES_256_CBC: encalg = GNUTLS_CIPHER_AES_256_CBC; break; default: @@ -96,56 +88,28 @@ int setup_esp_keys(struct openconnect_info *vpninfo, int new_keys) } switch (vpninfo->esp_hmac) { - case 0x01: + case HMAC_MD5: macalg = GNUTLS_MAC_MD5; break; - case 0x02: + case HMAC_SHA1: macalg = GNUTLS_MAC_SHA1; break; default: return -EINVAL; } - if (new_keys) { - vpninfo->old_esp_maxseq = vpninfo->esp_in[vpninfo->current_esp_in].seq + 32; - vpninfo->current_esp_in ^= 1; - } - - esp_in = &vpninfo->esp_in[vpninfo->current_esp_in]; - - if (new_keys) { - if ((ret = gnutls_rnd(GNUTLS_RND_NONCE, &esp_in->spi, sizeof(esp_in->spi))) || - (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)); - return -EIO; - } - } - - ret = init_esp_ciphers(vpninfo, &vpninfo->esp_out, macalg, encalg); + ret = init_esp_cipher(vpninfo, esp_out, macalg, encalg); if (ret) return ret; - if (gnutls_rnd(GNUTLS_RND_NONCE, vpninfo->esp_out.iv, sizeof(vpninfo->esp_out.iv))) { - vpn_progress(vpninfo, PRG_ERR, _("Failed to generate ESP IV\n")); - destroy_esp_ciphers(&vpninfo->esp_out); - return -EIO; - } - gnutls_cipher_set_iv(vpninfo->esp_out.cipher, vpninfo->esp_out.iv, - sizeof(vpninfo->esp_out.iv)); - + gnutls_cipher_set_iv(esp_out->cipher, esp_out->iv, sizeof(esp_out->iv)); - ret = init_esp_ciphers(vpninfo, esp_in, macalg, encalg); + ret = init_esp_cipher(vpninfo, esp_in, macalg, encalg); if (ret) { - destroy_esp_ciphers(&vpninfo->esp_out); + destroy_esp_ciphers(esp_out); return ret; } - if (vpninfo->dtls_state == DTLS_NOSECRET) - vpninfo->dtls_state = DTLS_SECRET; - vpninfo->pkt_trailer = 16 + 20; /* 16 for pad, 20 for HMAC (of which we use 16) */ return 0; } diff --git a/gpst.c b/gpst.c index 80af5f09..96efa11b 100644 --- a/gpst.c +++ b/gpst.c @@ -550,7 +550,7 @@ static int gpst_parse_config_xml(struct openconnect_info *vpninfo, xmlNode *xml_ else if (!xmlnode_get_val(member, "ipsec-mode", &s) && strcmp(s, "esp-tunnel")) vpn_progress(vpninfo, PRG_ERR, _("GlobalProtect config sent ipsec-mode=%s (expected esp-tunnel)\n"), s); } - if (setup_esp_keys(vpninfo, 0)) + if (openconnect_setup_esp_keys(vpninfo, 0)) vpn_progress(vpninfo, PRG_ERR, "Failed to setup ESP keys.\n"); else /* prevent race condition between esp_mainloop() and gpst_mainloop() timers */ diff --git a/oncp.c b/oncp.c index 4c389f5a..24b1e56c 100644 --- a/oncp.c +++ b/oncp.c @@ -752,7 +752,7 @@ int oncp_connect(struct openconnect_info *vpninfo) put_len16(reqbuf, kmp); #ifdef HAVE_ESP - if (!setup_esp_keys(vpninfo, 1)) { + if (!openconnect_setup_esp_keys(vpninfo, 1)) { struct esp *esp = &vpninfo->esp_in[vpninfo->current_esp_in]; /* Since we'll want to do this in the oncp_mainloop too, where it's easier * *not* to have an oc_text_buf and build it up manually, and since it's @@ -806,7 +806,7 @@ static int oncp_receive_espkeys(struct openconnect_info *vpninfo, int len) int ret; ret = parse_conf_pkt(vpninfo, vpninfo->cstp_pkt->oncp.kmp, len + 20, 301); - if (!ret && !setup_esp_keys(vpninfo, 1)) { + if (!ret && !openconnect_setup_esp_keys(vpninfo, 1)) { struct esp *esp = &vpninfo->esp_in[vpninfo->current_esp_in]; unsigned char *p = vpninfo->cstp_pkt->oncp.kmp; diff --git a/openconnect-internal.h b/openconnect-internal.h index 6304084b..b7ac3ba9 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -930,10 +930,11 @@ int esp_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable); void esp_close(struct openconnect_info *vpninfo); void esp_shutdown(struct openconnect_info *vpninfo); int print_esp_keys(struct openconnect_info *vpninfo, const char *name, struct esp *esp); +int openconnect_setup_esp_keys(struct openconnect_info *vpninfo, int new_keys); /* {gnutls,openssl}-esp.c */ -int setup_esp_keys(struct openconnect_info *vpninfo, int new_keys); void destroy_esp_ciphers(struct esp *esp); +int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *out, struct esp *in); int decrypt_esp_packet(struct openconnect_info *vpninfo, struct esp *esp, struct pkt *pkt); int encrypt_esp_packet(struct openconnect_info *vpninfo, struct pkt *pkt); diff --git a/openssl-esp.c b/openssl-esp.c index 9f6531c2..364bb283 100644 --- a/openssl-esp.c +++ b/openssl-esp.c @@ -57,7 +57,7 @@ void destroy_esp_ciphers(struct esp *esp) } } -static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp, +static int init_esp_cipher(struct openconnect_info *vpninfo, struct esp *esp, const EVP_MD *macalg, const EVP_CIPHER *encalg, int decrypt) { int ret; @@ -78,8 +78,7 @@ static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp, if (decrypt) ret = EVP_DecryptInit_ex(esp->cipher, encalg, NULL, esp->enc_key, NULL); else { - ret = RAND_bytes((void *)&esp->iv, sizeof(esp->iv)) && - EVP_EncryptInit_ex(esp->cipher, encalg, NULL, esp->enc_key, esp->iv); + ret = EVP_EncryptInit_ex(esp->cipher, encalg, NULL, esp->enc_key, esp->iv); } if (!ret) { @@ -103,28 +102,21 @@ static int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp, openconnect_report_ssl_errors(vpninfo); destroy_esp_ciphers(esp); } - esp->seq = 0; - esp->seq_backlog = 0; + return 0; } -int setup_esp_keys(struct openconnect_info *vpninfo, int new_keys) +int init_esp_ciphers(struct openconnect_info *vpninfo, struct esp *esp_out, struct esp *esp_in) { - struct esp *esp_in; const EVP_CIPHER *encalg; const EVP_MD *macalg; int ret; - if (vpninfo->dtls_state == DTLS_DISABLED) - return -EOPNOTSUPP; - if (!vpninfo->dtls_addr) - return -EINVAL; - switch (vpninfo->esp_enc) { - case 0x02: + case ENC_AES_128_CBC: encalg = EVP_aes_128_cbc(); break; - case 0x05: + case ENC_AES_256_CBC: encalg = EVP_aes_256_cbc(); break; default: @@ -132,47 +124,26 @@ int setup_esp_keys(struct openconnect_info *vpninfo, int new_keys) } switch (vpninfo->esp_hmac) { - case 0x01: + case HMAC_MD5: macalg = EVP_md5(); break; - case 0x02: + case HMAC_SHA1: macalg = EVP_sha1(); break; default: return -EINVAL; } - if (new_keys) { - vpninfo->old_esp_maxseq = vpninfo->esp_in[vpninfo->current_esp_in].seq + 32; - vpninfo->current_esp_in ^= 1; - } - - esp_in = &vpninfo->esp_in[vpninfo->current_esp_in]; - - if (new_keys) { - if (!RAND_bytes((void *)&esp_in->spi, sizeof(esp_in->spi)) || - !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); - return -EIO; - } - } - - ret = init_esp_ciphers(vpninfo, &vpninfo->esp_out, macalg, encalg, 0); + ret = init_esp_cipher(vpninfo, &vpninfo->esp_out, macalg, encalg, 0); if (ret) return ret; - ret = init_esp_ciphers(vpninfo, esp_in, macalg, encalg, 1); + ret = init_esp_cipher(vpninfo, esp_in, macalg, encalg, 1); if (ret) { destroy_esp_ciphers(&vpninfo->esp_out); return ret; } - if (vpninfo->dtls_state == DTLS_NOSECRET) - vpninfo->dtls_state = DTLS_SECRET; - vpninfo->pkt_trailer = 16 + 20; /* 16 for pad, 20 for HMAC (of which we use 16) */ return 0; } -- 2.49.0