[PATCH v2] store length of ESP encryption and HMAC keys so that they can be manipulated separately for both Juniper and GP
Daniel Lenski
dlenski at gmail.com
Sun May 14 17:06:48 PDT 2017
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 at gmail.com>
---
esp.c | 13 ++++---------
gnutls-esp.c | 7 ++++---
oncp.c | 37 +++++++++++++++++++++++++++----------
openconnect-internal.h | 5 ++++-
openssl-esp.c | 5 +++--
5 files changed, 42 insertions(+), 25 deletions(-)
diff --git a/esp.c b/esp.c
index 44c9407..30ec442 100644
--- 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 1ad4e60..f3fd499 100644
--- 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 3c7cfa1..45c2f15 100644
--- 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 a24a9e4..8bee8ef 100644
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -335,7 +335,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 {
@@ -359,6 +360,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;
diff --git a/openssl-esp.c b/openssl-esp.c
index e20bde0..faba1ff 100644
--- 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);
--
2.7.4
More information about the openconnect-devel
mailing list