Revert "Use aes-256-gcm rather than aes-128-cbc"

This reverts commit e9c92795d87a316ea47f6bf37c9636e86eec57e7.

AESGCM is a neat idea but it is not really appropriate to be used in
mod_auth_gssapi because we cannot gurantee that the nonce will never be
reused. It is not very probable, and it is also not easy to force the
server to generate so many encyrpted sessions to have a good chance of
a collision that I know of, but better to avoid the whole issue, than
risk unforseen cases where it may happen.

diff --git a/src/crypto.c b/src/crypto.c
index 9e11deb..a5dea45 100644 (file)
--- a/src/crypto.c
+++ b/src/crypto.c
@@ -6,15 +6,15 @@
 #include <stdbool.h>
 #include "crypto.h"
 
-#define TAGSIZE 16
-
 struct seal_key {
     const EVP_CIPHER *cipher;
+    const EVP_MD *md;
     unsigned char *ekey;
+    unsigned char *hkey;
 };
 
 apr_status_t SEAL_KEY_CREATE(apr_pool_t *p, struct seal_key **skey,
-                             struct databuf *key)
+                             struct databuf *keys)
 {
     struct seal_key *n;
     int keylen;
@@ -23,7 +23,7 @@ apr_status_t SEAL_KEY_CREATE(apr_pool_t *p, struct seal_key **skey,
     n = apr_pcalloc(p, sizeof(*n));
     if (!n) return ENOMEM;
 
-    n->cipher = EVP_aes_256_gcm();
+    n->cipher = EVP_aes_128_cbc();
     if (!n->cipher) {
         ret = EFAULT;
         goto done;
@@ -31,30 +31,50 @@ apr_status_t SEAL_KEY_CREATE(apr_pool_t *p, struct seal_key **skey,
 
     keylen = n->cipher->key_len;
 
+    n->md = EVP_sha256();
+    if (!n->md) {
+        ret = EFAULT;
+        goto done;
+    }
+
     n->ekey = apr_palloc(p, keylen);
     if (!n->ekey) {
         ret = ENOMEM;
         goto done;
     }
 
-    if (key) {
-        if (key->length < keylen) {
+    n->hkey = apr_palloc(p, keylen);
+    if (!n->hkey) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    if (keys) {
+        if (keys->length != (keylen * 2)) {
             ret = EINVAL;
             goto done;
         }
-        memcpy(n->ekey, key->value, keylen);
+        memcpy(n->ekey, keys->value, keylen);
+        memcpy(n->hkey, keys->value + keylen, keylen);
     } else {
         ret = apr_generate_random_bytes(n->ekey, keylen);
         if (ret != 0) {
             ret = EFAULT;
             goto done;
         }
+
+        ret = apr_generate_random_bytes(n->hkey, keylen);
+        if (ret != 0) {
+            ret = EFAULT;
+            goto done;
+        }
     }
 
     ret = 0;
 done:
     if (ret) {
         free(n->ekey);
+        free(n->hkey);
         free(n);
     } else {
         *skey = n;
@@ -65,58 +85,71 @@ done:
 apr_status_t SEAL_BUFFER(apr_pool_t *p, struct seal_key *skey,
                          struct databuf *plain, struct databuf *cipher)
 {
+    int blksz = skey->cipher->block_size;
     apr_status_t err = EFAULT;
-    EVP_CIPHER_CTX ctx = {};
-    int minlen;
-    int outlen;
+    EVP_CIPHER_CTX ctx = { 0 };
+    HMAC_CTX hmac_ctx = { 0 };
+    uint8_t rbuf[blksz];
+    unsigned int len;
+    int outlen, totlen;
     int ret;
 
     EVP_CIPHER_CTX_init(&ctx);
 
-       /* Add space for padding, IV and tag. */
-    minlen = plain->length / skey->cipher->block_size + 1;
-    minlen *= skey->cipher->block_size;
-    minlen += skey->cipher->iv_len + TAGSIZE;
-    if (cipher->length < minlen) {
-        cipher->length = minlen;
-        cipher->value = apr_palloc(p, cipher->length);
+    /* confounder to avoid exposing random numbers directly to clients
+     * as IVs */
+    ret = apr_generate_random_bytes(rbuf, sizeof(rbuf));
+    if (ret != 0) goto done;
+
+    if (cipher->length == 0) {
+        /* add space for confounder and padding and MAC */
+        cipher->length = (plain->length / blksz + 2) * blksz;
+        cipher->value = apr_palloc(p, cipher->length + skey->md->md_size);
         if (!cipher->value) {
             err = ENOMEM;
             goto done;
         }
     }
 
-    /* Generate IV. */
-    ret = apr_generate_random_bytes(cipher->value, skey->cipher->iv_len);
-    if (ret != 0) goto done;
-    cipher->length = skey->cipher->iv_len;
-
-    ret = EVP_EncryptInit_ex(&ctx, skey->cipher, NULL,
-                             skey->ekey, cipher->value);
-    if (ret != 1) goto done;
-
-    /* Encrypt the data. */
-    outlen = 0;
-    ret = EVP_EncryptUpdate(&ctx, &cipher->value[cipher->length],
-                            &outlen, plain->value, plain->length);
-    if (ret != 1) goto done;
-    cipher->length += outlen;
-
-    outlen = 0;
-    ret = EVP_EncryptFinal_ex(&ctx, &cipher->value[cipher->length], &outlen);
-    if (ret != 1) goto done;
-    cipher->length += outlen;
-
-    /* Get the tag */
-    ret = EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_GET_TAG, TAGSIZE,
-                              &cipher->value[cipher->length]);
-    if (ret != 1) goto done;
-    cipher->length += TAGSIZE;
+    ret = EVP_EncryptInit_ex(&ctx, skey->cipher, NULL, skey->ekey, NULL);
+    if (ret == 0) goto done;
+    totlen = 0;
+
+    outlen = cipher->length;
+    ret = EVP_EncryptUpdate(&ctx, cipher->value, &outlen, rbuf, sizeof(rbuf));
+    if (ret == 0) goto done;
+    totlen += outlen;
+
+    outlen = cipher->length - totlen;
+    ret = EVP_EncryptUpdate(&ctx, &cipher->value[totlen], &outlen,
+                            plain->value, plain->length);
+    if (ret == 0) goto done;
+    totlen += outlen;
+
+    outlen = cipher->length - totlen;
+    ret = EVP_EncryptFinal_ex(&ctx, &cipher->value[totlen], &outlen);
+    if (ret == 0) goto done;
+    totlen += outlen;
 
+    /* now MAC the buffer */
+    HMAC_CTX_init(&hmac_ctx);
+
+    ret = HMAC_Init_ex(&hmac_ctx, skey->hkey,
+                       skey->cipher->key_len, skey->md, NULL);
+    if (ret == 0) goto done;
+
+    ret = HMAC_Update(&hmac_ctx, cipher->value, totlen);
+    if (ret == 0) goto done;
+
+    ret = HMAC_Final(&hmac_ctx, &cipher->value[totlen], &len);
+    if (ret == 0) goto done;
+
+    cipher->length = totlen + len;
     err = 0;
 
 done:
     EVP_CIPHER_CTX_cleanup(&ctx);
+    HMAC_CTX_cleanup(&hmac_ctx);
     return err;
 }
 
@@ -124,14 +157,41 @@ apr_status_t UNSEAL_BUFFER(apr_pool_t *p, struct seal_key *skey,
                            struct databuf *cipher, struct databuf *plain)
 {
     apr_status_t err = EFAULT;
-    EVP_CIPHER_CTX ctx = {};
-    int outlen;
-    int ret;
+    EVP_CIPHER_CTX ctx = { 0 };
+    HMAC_CTX hmac_ctx = { 0 };
+    unsigned char mac[skey->md->md_size];
+    unsigned int len;
+    int outlen, totlen;
+    volatile bool equal = true;
+    int ret, i;
+
+    /* check MAC first */
+    HMAC_CTX_init(&hmac_ctx);
+
+    ret = HMAC_Init_ex(&hmac_ctx, skey->hkey,
+                       skey->cipher->key_len, skey->md, NULL);
+    if (ret == 0) goto done;
+
+    cipher->length -= skey->md->md_size;
+
+    ret = HMAC_Update(&hmac_ctx, cipher->value, cipher->length);
+    if (ret == 0) goto done;
+
+    ret = HMAC_Final(&hmac_ctx, mac, &len);
+    if (ret == 0) goto done;
+
+    if (len != skey->md->md_size) goto done;
+    for (i = 0; i < skey->md->md_size; i++) {
+        if (cipher->value[cipher->length + i] != mac[i]) equal = false;
+        /* not breaking intentionally,
+         * or we would allow an oracle attack */
+    }
+    if (!equal) goto done;
 
     EVP_CIPHER_CTX_init(&ctx);
 
-    if (plain->length < cipher->length - skey->cipher->iv_len - TAGSIZE) {
-        plain->length = cipher->length - skey->cipher->iv_len - TAGSIZE;
+    if (plain->length == 0) {
+        plain->length = cipher->length;
         plain->value = apr_palloc(p, plain->length);
         if (!plain->value) {
             err = ENOMEM;
@@ -139,30 +199,30 @@ apr_status_t UNSEAL_BUFFER(apr_pool_t *p, struct seal_key *skey,
         }
     }
 
-    ret = EVP_DecryptInit_ex(&ctx, skey->cipher, NULL,
-                             skey->ekey, cipher->value);
-    if (ret != 1) goto done;
-    plain->length = 0;
+    ret = EVP_DecryptInit_ex(&ctx, skey->cipher, NULL, skey->ekey, NULL);
+    if (ret == 0) goto done;
 
-    outlen = 0;
+    totlen = 0;
+    outlen = plain->length;
     ret = EVP_DecryptUpdate(&ctx, plain->value, &outlen,
-                            &cipher->value[skey->cipher->iv_len],
-                            cipher->length - skey->cipher->iv_len - TAGSIZE);
-    if (ret != 1) goto done;
-    plain->length += outlen;
+                            cipher->value, cipher->length);
+    if (ret == 0) goto done;
 
-    ret = EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_SET_TAG, TAGSIZE,
-                              &cipher->value[cipher->length - TAGSIZE]);
-    if (ret != 1) goto done;
+    totlen += outlen;
+    outlen = plain->length - totlen;
+    ret = EVP_DecryptFinal_ex(&ctx, plain->value, &outlen);
+    if (ret == 0) goto done;
 
-    outlen = 0;
-    ret = EVP_DecryptFinal_ex(&ctx, &plain->value[plain->length], &outlen);
-    if (ret != 1) goto done;
-    plain->length += outlen;
+    totlen += outlen;
+    /* now remove the confounder */
+    totlen -= skey->cipher->block_size;
+    memmove(plain->value, plain->value + skey->cipher->block_size, totlen);
 
+    plain->length = totlen;
     err = 0;
 
 done:
     EVP_CIPHER_CTX_cleanup(&ctx);
+    HMAC_CTX_cleanup(&hmac_ctx);
     return err;
 }

diff --git a/src/mod_auth_gssapi.c b/src/mod_auth_gssapi.c
index 9cb53ec..db33853 100644 (file)
--- a/src/mod_auth_gssapi.c
+++ b/src/mod_auth_gssapi.c
@@ -668,7 +668,7 @@ static const char *mag_use_s4u2p(cmd_parms *parms, void *mconfig, int on)
 static const char *mag_sess_key(cmd_parms *parms, void *mconfig, const char *w)
 {
     struct mag_config *cfg = (struct mag_config *)mconfig;
-    struct databuf key;
+    struct databuf keys;
     unsigned char *val;
     apr_status_t rc;
     const char *k;
@@ -689,16 +689,16 @@ static const char *mag_sess_key(cmd_parms *parms, void *mconfig, const char *w)
         return NULL;
     }
 
-    key.length = (int)apr_base64_decode_binary(val, k);
-    key.value = (unsigned char *)val;
+    keys.length = (int)apr_base64_decode_binary(val, k);
+    keys.value = (unsigned char *)val;
 
-    if (key.length < 32) {
+    if (keys.length != 32) {
         ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, parms->server,
-                     "Invalid key length, expected >=32 got %d", key.length);
+                     "Invalid key lenght, expected 32 got %d", keys.length);
         return NULL;
     }
 
-    rc = SEAL_KEY_CREATE(cfg->pool, &cfg->mag_skey, &key);
+    rc = SEAL_KEY_CREATE(cfg->pool, &cfg->mag_skey, &keys);
     if (rc != OK) {
         ap_log_error(APLOG_MARK, APLOG_ERR|APLOG_NOERRNO, 0, parms->server,
                      "Failed to import sealing key!");