[PATCH v3] store length of ESP encryption and HMAC keys so that they can be manipulated separately for both Juniper and GP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Shame on me!!! I tested that this built and worked correctly only with
GnuTLS, not OpenSSL.

David,
Looks like you already caught and patched it yourself:
http://git.infradead.org/users/dwmw2/openconnect.git/blobdiff/b2b1dd0702239d415943cea6f821a345e0d50b63..d8b283f168c8e9574a821eef077b40984fae1026:/openssl-esp.c

A user of openconnect-gp ran into this as well. Sorry about that. :-(

-Dan

On Sun, May 14, 2017 at 9:22 PM, Daniel Lenski <dlenski at gmail.com> wrote:
> 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 | 11 ++++++++++-
>  openssl-esp.c          |  5 +++--
>  5 files changed, 48 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..a54761f 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;
> @@ -684,6 +687,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 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
>



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux