|
|
4bd34d |
From 51bba6bf325716534c509e0528d2ccfd0050d28c Mon Sep 17 00:00:00 2001
|
|
|
4bd34d |
From: Simo Sorce <simo@redhat.com>
|
|
|
4bd34d |
Date: Wed, 17 Apr 2019 18:00:59 -0400
|
|
|
4bd34d |
Subject: [PATCH] Change the way we handle encrypted buffers
|
|
|
4bd34d |
|
|
|
4bd34d |
The previous change has backwards incompatible behavior that may also
|
|
|
4bd34d |
lead to buffer overruns.
|
|
|
4bd34d |
|
|
|
4bd34d |
Because we have no easy way to indicate a format change and to maintain
|
|
|
4bd34d |
backwards compatibility for the ciphers that were working (those that
|
|
|
4bd34d |
added padding were hopelessly borken anyway) introduce code to simply
|
|
|
4bd34d |
add padding that we can recognize and remove when we read back the token.
|
|
|
4bd34d |
|
|
|
4bd34d |
On ciphers that do not add padding this is basically a no op and the
|
|
|
4bd34d |
tokens will be identical to the ones we previously emitted.
|
|
|
4bd34d |
|
|
|
4bd34d |
On ciphers that add padding we pad the plaintext so that we hit a block
|
|
|
4bd34d |
boundary and cause no extra padding to be added by krb5_c_encrypt
|
|
|
4bd34d |
itself. On decryption we check if padding bytes are appended to the
|
|
|
4bd34d |
buffer and remove them.
|
|
|
4bd34d |
|
|
|
4bd34d |
Signed-off-by: Simo Sorce <simo@redhat.com>
|
|
|
4bd34d |
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
|
|
|
4bd34d |
Merges: #246
|
|
|
4bd34d |
(cherry picked from commit 839be8aa7e54e93819e8291b570e4c7cfe7e98f1)
|
|
|
4bd34d |
---
|
|
|
4bd34d |
src/gp_export.c | 110 +++++++++++++++++++++++++++++++++++++-----------
|
|
|
4bd34d |
1 file changed, 86 insertions(+), 24 deletions(-)
|
|
|
4bd34d |
|
|
|
4bd34d |
diff --git a/src/gp_export.c b/src/gp_export.c
|
|
|
4bd34d |
index aa0a8ec..dbfddeb 100644
|
|
|
4bd34d |
--- a/src/gp_export.c
|
|
|
4bd34d |
+++ b/src/gp_export.c
|
|
|
4bd34d |
@@ -193,9 +193,15 @@ done:
|
|
|
4bd34d |
return ret_maj;
|
|
|
4bd34d |
}
|
|
|
4bd34d |
|
|
|
4bd34d |
-/* We need to include a length in our payloads because krb5_c_decrypt() will
|
|
|
4bd34d |
- * pad the contents for some enctypes, and gss_import_cred() doesn't like
|
|
|
4bd34d |
- * having extra bytes on tokens. */
|
|
|
4bd34d |
+#define ENC_MIN_PAD_LEN 8
|
|
|
4bd34d |
+
|
|
|
4bd34d |
+/* We need to pad our payloads because krb5_c_decrypt() may pad the
|
|
|
4bd34d |
+ * contents for some enctypes, and gss_import_cred() doesn't like
|
|
|
4bd34d |
+ * having extra bytes on tokens.
|
|
|
4bd34d |
+ * Explicit padding and depadding is used in order to maintain backwards
|
|
|
4bd34d |
+ * compatibility over upgrades (and downgrades), it would have been
|
|
|
4bd34d |
+ * better if we simply had a better formatting of the returned blob
|
|
|
4bd34d |
+ * so we could simply change a "blob version" number */
|
|
|
4bd34d |
static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
4bd34d |
size_t len, void *buf, octet_string *out)
|
|
|
4bd34d |
{
|
|
|
4bd34d |
@@ -203,8 +209,9 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
4bd34d |
krb5_data data_in;
|
|
|
4bd34d |
krb5_enc_data enc_handle;
|
|
|
4bd34d |
size_t cipherlen;
|
|
|
4bd34d |
- char *packed = NULL;
|
|
|
4bd34d |
- uint32_t netlen;
|
|
|
4bd34d |
+ size_t padcheck;
|
|
|
4bd34d |
+ uint8_t pad = 0;
|
|
|
4bd34d |
+ char *padded = NULL;
|
|
|
4bd34d |
|
|
|
4bd34d |
if (len > (uint32_t)(-1)) {
|
|
|
4bd34d |
/* Needs to fit in 4 bytes of payload, so... */
|
|
|
4bd34d |
@@ -212,28 +219,72 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
4bd34d |
goto done;
|
|
|
4bd34d |
}
|
|
|
4bd34d |
|
|
|
4bd34d |
- packed = malloc(len);
|
|
|
4bd34d |
- if (!packed) {
|
|
|
4bd34d |
- ret = errno;
|
|
|
4bd34d |
+ ret = krb5_c_encrypt_length(context,
|
|
|
4bd34d |
+ key->enctype,
|
|
|
4bd34d |
+ len, &cipherlen);
|
|
|
4bd34d |
+ if (ret) {
|
|
|
4bd34d |
goto done;
|
|
|
4bd34d |
}
|
|
|
4bd34d |
|
|
|
4bd34d |
- netlen = htonl(len);
|
|
|
4bd34d |
- memcpy(packed, (uint8_t *)&netlen, 4);
|
|
|
4bd34d |
- memcpy(packed + 4, buf, len);
|
|
|
4bd34d |
-
|
|
|
4bd34d |
- data_in.length = len + 4;
|
|
|
4bd34d |
- data_in.data = packed;
|
|
|
4bd34d |
-
|
|
|
4bd34d |
- memset(&enc_handle, '\0', sizeof(krb5_enc_data));
|
|
|
4bd34d |
-
|
|
|
4bd34d |
+ /* try again with len + 1 to see if padding is required */
|
|
|
4bd34d |
ret = krb5_c_encrypt_length(context,
|
|
|
4bd34d |
key->enctype,
|
|
|
4bd34d |
- data_in.length,
|
|
|
4bd34d |
- &cipherlen);
|
|
|
4bd34d |
+ len + 1, &padcheck);
|
|
|
4bd34d |
if (ret) {
|
|
|
4bd34d |
goto done;
|
|
|
4bd34d |
}
|
|
|
4bd34d |
+ if (padcheck == cipherlen) {
|
|
|
4bd34d |
+ int i;
|
|
|
4bd34d |
+ /* padding required */
|
|
|
4bd34d |
+ pad = ENC_MIN_PAD_LEN;
|
|
|
4bd34d |
+ /* always add enough padding that it makes it extremely unlikley
|
|
|
4bd34d |
+ * legitimate plaintext will be incorrectly depadded in the
|
|
|
4bd34d |
+ * decrypt function */
|
|
|
4bd34d |
+ ret = krb5_c_encrypt_length(context,
|
|
|
4bd34d |
+ key->enctype,
|
|
|
4bd34d |
+ len + pad, &cipherlen);
|
|
|
4bd34d |
+ if (ret) {
|
|
|
4bd34d |
+ goto done;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ /* we support only block sizes up to 16 bytes as this is the largest
|
|
|
4bd34d |
+ * supported block size in krb ciphers for now */
|
|
|
4bd34d |
+ for (i = 0; i < 15; i++) {
|
|
|
4bd34d |
+ /* find the point at which padcheck increases, that's when we
|
|
|
4bd34d |
+ * cross a blocksize boundary internally and we can calculate
|
|
|
4bd34d |
+ * the padding that will be used */
|
|
|
4bd34d |
+ ret = krb5_c_encrypt_length(context,
|
|
|
4bd34d |
+ key->enctype,
|
|
|
4bd34d |
+ len + pad + i + 1, &padcheck);
|
|
|
4bd34d |
+ if (ret) {
|
|
|
4bd34d |
+ goto done;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ if (padcheck > cipherlen) {
|
|
|
4bd34d |
+ pad += i;
|
|
|
4bd34d |
+ break;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ if (i > 15) {
|
|
|
4bd34d |
+ ret = EINVAL;
|
|
|
4bd34d |
+ goto done;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+
|
|
|
4bd34d |
+ if (pad != 0) {
|
|
|
4bd34d |
+ padded = malloc(len + pad);
|
|
|
4bd34d |
+ if (!padded) {
|
|
|
4bd34d |
+ ret = errno;
|
|
|
4bd34d |
+ goto done;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+
|
|
|
4bd34d |
+ memcpy(padded, buf, len);
|
|
|
4bd34d |
+ memset(padded + len, pad, pad);
|
|
|
4bd34d |
+
|
|
|
4bd34d |
+ data_in.length = len + pad;
|
|
|
4bd34d |
+ data_in.data = padded;
|
|
|
4bd34d |
+ } else {
|
|
|
4bd34d |
+ data_in.length = len;
|
|
|
4bd34d |
+ data_in.data = buf;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
|
|
|
4bd34d |
enc_handle.ciphertext.length = cipherlen;
|
|
|
4bd34d |
enc_handle.ciphertext.data = malloc(enc_handle.ciphertext.length);
|
|
|
4bd34d |
@@ -261,7 +312,7 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
4bd34d |
}
|
|
|
4bd34d |
|
|
|
4bd34d |
done:
|
|
|
4bd34d |
- free(packed);
|
|
|
4bd34d |
+ free(padded);
|
|
|
4bd34d |
free(enc_handle.ciphertext.data);
|
|
|
4bd34d |
return ret;
|
|
|
4bd34d |
}
|
|
|
4bd34d |
@@ -273,7 +324,8 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
4bd34d |
int ret;
|
|
|
4bd34d |
krb5_data data_out;
|
|
|
4bd34d |
krb5_enc_data enc_handle;
|
|
|
4bd34d |
- uint32_t netlen;
|
|
|
4bd34d |
+ uint8_t pad;
|
|
|
4bd34d |
+ int i, j;
|
|
|
4bd34d |
|
|
|
4bd34d |
memset(&enc_handle, '\0', sizeof(krb5_enc_data));
|
|
|
4bd34d |
|
|
|
4bd34d |
@@ -295,9 +347,19 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
4bd34d |
}
|
|
|
4bd34d |
|
|
|
4bd34d |
/* And handle the padding. */
|
|
|
4bd34d |
- memcpy(&netlen, buf, 4);
|
|
|
4bd34d |
- *len = ntohl(netlen);
|
|
|
4bd34d |
- memmove(buf, buf + 4, *len);
|
|
|
4bd34d |
+ i = data_out.length - 1;
|
|
|
4bd34d |
+ pad = data_out.data[i];
|
|
|
4bd34d |
+ if (pad >= ENC_MIN_PAD_LEN && pad < i) {
|
|
|
4bd34d |
+ j = pad;
|
|
|
4bd34d |
+ while (j > 0) {
|
|
|
4bd34d |
+ j--;
|
|
|
4bd34d |
+ if (pad != data_out.data[i - j]) break;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ if (j == 0) {
|
|
|
4bd34d |
+ data_out.length -= pad;
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ }
|
|
|
4bd34d |
+ *len = data_out.length;
|
|
|
4bd34d |
|
|
|
4bd34d |
return 0;
|
|
|
4bd34d |
}
|