|
|
18e43e |
From d5e68e9d1776eb0d1d3deaae461a376a42bda3a5 Mon Sep 17 00:00:00 2001
|
|
|
18e43e |
From: Robbie Harwood <rharwood@redhat.com>
|
|
|
18e43e |
Date: Apr 16 2019 19:12:03 +0000
|
|
|
18e43e |
Subject: Handle gss_import_cred() failure when importing gssx creds
|
|
|
18e43e |
|
|
|
18e43e |
|
|
|
18e43e |
Otherwise, we might attempt to set options on a non-existent handle,
|
|
|
18e43e |
leading to a segfault.
|
|
|
18e43e |
|
|
|
18e43e |
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
|
|
18e43e |
|
|
|
18e43e |
---
|
|
|
18e43e |
|
|
|
18e43e |
diff --git a/src/gp_export.c b/src/gp_export.c
|
|
|
18e43e |
index 403e339..7ad8037 100644
|
|
|
18e43e |
--- a/src/gp_export.c
|
|
|
18e43e |
+++ b/src/gp_export.c
|
|
|
18e43e |
@@ -476,6 +476,10 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall,
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
ret_maj = gss_import_cred(&ret_min, &token, out);
|
|
|
18e43e |
+ if (ret_maj) {
|
|
|
18e43e |
+ GPDEBUG("gss_import_cred failed when importing gssx cred\n");
|
|
|
18e43e |
+ goto done;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
|
|
|
18e43e |
/* check if there is any client option we need to set on credentials */
|
|
|
18e43e |
gp_set_cred_options(cred, *out);
|
|
|
18e43e |
|
|
|
18e43e |
From 2555d76fe3e5ce7b3a04f137054da0ba1f1a9374 Mon Sep 17 00:00:00 2001
|
|
|
18e43e |
From: Robbie Harwood <rharwood@redhat.com>
|
|
|
18e43e |
Date: Apr 16 2019 19:11:59 +0000
|
|
|
18e43e |
Subject: Always initialize out cred in gp_import_gssx_cred()
|
|
|
18e43e |
|
|
|
18e43e |
|
|
|
18e43e |
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
|
|
18e43e |
|
|
|
18e43e |
---
|
|
|
18e43e |
|
|
|
18e43e |
diff --git a/src/gp_export.c b/src/gp_export.c
|
|
|
18e43e |
index 5e8e160..403e339 100644
|
|
|
18e43e |
--- a/src/gp_export.c
|
|
|
18e43e |
+++ b/src/gp_export.c
|
|
|
18e43e |
@@ -449,6 +449,8 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall,
|
|
|
18e43e |
uint32_t ret_min = 0;
|
|
|
18e43e |
int ret;
|
|
|
18e43e |
|
|
|
18e43e |
+ *out = GSS_C_NO_CREDENTIAL;
|
|
|
18e43e |
+
|
|
|
18e43e |
handle = gp_service_get_creds_handle(gpcall->service);
|
|
|
18e43e |
if (!handle) {
|
|
|
18e43e |
ret_maj = GSS_S_FAILURE;
|
|
|
18e43e |
@@ -470,7 +472,6 @@ uint32_t gp_import_gssx_cred(uint32_t *min, struct gp_call_ctx *gpcall,
|
|
|
18e43e |
if (ret) {
|
|
|
18e43e |
/* Allow for re-issuance of the keytab. */
|
|
|
18e43e |
GPDEBUG("Stored ccache failed to decrypt; treating as empty\n");
|
|
|
18e43e |
- *out = GSS_C_NO_CREDENTIAL;
|
|
|
18e43e |
goto done;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
|
|
|
18e43e |
From 25ff6504f18b12ee1a65215808c661518802b832 Mon Sep 17 00:00:00 2001
|
|
|
18e43e |
From: Robbie Harwood <rharwood@redhat.com>
|
|
|
18e43e |
Date: Apr 16 2019 20:52:13 +0000
|
|
|
18e43e |
Subject: Include length when using krb5_c_decrypt()
|
|
|
18e43e |
|
|
|
18e43e |
|
|
|
18e43e |
For some enctypes, krb5_c_decrypt() will add padding bytes which are
|
|
|
18e43e |
included in the returned length. However, functions which use the
|
|
|
18e43e |
objects we're storing aren't always prepared for that: in particular,
|
|
|
18e43e |
gss_import_cred() will declare a token invalid if there's trailing
|
|
|
18e43e |
garbage.
|
|
|
18e43e |
|
|
|
18e43e |
Work around this by including 4 bytes of length on encrypted objects.
|
|
|
18e43e |
|
|
|
18e43e |
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
|
|
|
18e43e |
|
|
|
18e43e |
---
|
|
|
18e43e |
|
|
|
18e43e |
diff --git a/src/gp_export.c b/src/gp_export.c
|
|
|
18e43e |
index 7ad8037..aa0a8ec 100644
|
|
|
18e43e |
--- a/src/gp_export.c
|
|
|
18e43e |
+++ b/src/gp_export.c
|
|
|
18e43e |
@@ -193,6 +193,9 @@ done:
|
|
|
18e43e |
return ret_maj;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
+/* We need to include a length in our payloads because krb5_c_decrypt() will
|
|
|
18e43e |
+ * pad the contents for some enctypes, and gss_import_cred() doesn't like
|
|
|
18e43e |
+ * having extra bytes on tokens. */
|
|
|
18e43e |
static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
size_t len, void *buf, octet_string *out)
|
|
|
18e43e |
{
|
|
|
18e43e |
@@ -200,9 +203,27 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
krb5_data data_in;
|
|
|
18e43e |
krb5_enc_data enc_handle;
|
|
|
18e43e |
size_t cipherlen;
|
|
|
18e43e |
+ char *packed = NULL;
|
|
|
18e43e |
+ uint32_t netlen;
|
|
|
18e43e |
|
|
|
18e43e |
- data_in.length = len;
|
|
|
18e43e |
- data_in.data = buf;
|
|
|
18e43e |
+ if (len > (uint32_t)(-1)) {
|
|
|
18e43e |
+ /* Needs to fit in 4 bytes of payload, so... */
|
|
|
18e43e |
+ ret = ENOMEM;
|
|
|
18e43e |
+ goto done;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+
|
|
|
18e43e |
+ packed = malloc(len);
|
|
|
18e43e |
+ if (!packed) {
|
|
|
18e43e |
+ ret = errno;
|
|
|
18e43e |
+ goto done;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+
|
|
|
18e43e |
+ netlen = htonl(len);
|
|
|
18e43e |
+ memcpy(packed, (uint8_t *)&netlen, 4);
|
|
|
18e43e |
+ memcpy(packed + 4, buf, len);
|
|
|
18e43e |
+
|
|
|
18e43e |
+ data_in.length = len + 4;
|
|
|
18e43e |
+ data_in.data = packed;
|
|
|
18e43e |
|
|
|
18e43e |
memset(&enc_handle, '\0', sizeof(krb5_enc_data));
|
|
|
18e43e |
|
|
|
18e43e |
@@ -240,16 +261,19 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
done:
|
|
|
18e43e |
+ free(packed);
|
|
|
18e43e |
free(enc_handle.ciphertext.data);
|
|
|
18e43e |
return ret;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
+/* See comment above on gp_encrypt_buffer(). */
|
|
|
18e43e |
static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
- octet_string *in, size_t *len, void *buf)
|
|
|
18e43e |
+ octet_string *in, size_t *len, char *buf)
|
|
|
18e43e |
{
|
|
|
18e43e |
int ret;
|
|
|
18e43e |
krb5_data data_out;
|
|
|
18e43e |
krb5_enc_data enc_handle;
|
|
|
18e43e |
+ uint32_t netlen;
|
|
|
18e43e |
|
|
|
18e43e |
memset(&enc_handle, '\0', sizeof(krb5_enc_data));
|
|
|
18e43e |
|
|
|
18e43e |
@@ -270,7 +294,10 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
return ret;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
- *len = data_out.length;
|
|
|
18e43e |
+ /* And handle the padding. */
|
|
|
18e43e |
+ memcpy(&netlen, buf, 4);
|
|
|
18e43e |
+ *len = ntohl(netlen);
|
|
|
18e43e |
+ memmove(buf, buf + 4, *len);
|
|
|
18e43e |
|
|
|
18e43e |
return 0;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
From 839be8aa7e54e93819e8291b570e4c7cfe7e98f1 Mon Sep 17 00:00:00 2001
|
|
|
18e43e |
From: Simo Sorce <simo@redhat.com>
|
|
|
18e43e |
Date: Apr 18 2019 15:23:49 +0000
|
|
|
18e43e |
Subject: Change the way we handle encrypted buffers
|
|
|
18e43e |
|
|
|
18e43e |
|
|
|
18e43e |
The previous change has backwards incompatible behavior that may also
|
|
|
18e43e |
lead to buffer overruns.
|
|
|
18e43e |
|
|
|
18e43e |
Because we have no easy way to indicate a format change and to maintain
|
|
|
18e43e |
backwards compatibility for the ciphers that were working (those that
|
|
|
18e43e |
added padding were hopelessly borken anyway) introduce code to simply
|
|
|
18e43e |
add padding that we can recognize and remove when we read back the token.
|
|
|
18e43e |
|
|
|
18e43e |
On ciphers that do not add padding this is basically a no op and the
|
|
|
18e43e |
tokens will be identical to the ones we previously emitted.
|
|
|
18e43e |
|
|
|
18e43e |
On ciphers that add padding we pad the plaintext so that we hit a block
|
|
|
18e43e |
boundary and cause no extra padding to be added by krb5_c_encrypt
|
|
|
18e43e |
itself. On decryption we check if padding bytes are appended to the
|
|
|
18e43e |
buffer and remove them.
|
|
|
18e43e |
|
|
|
18e43e |
Signed-off-by: Simo Sorce <simo@redhat.com>
|
|
|
18e43e |
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
|
|
|
18e43e |
Merges: #246
|
|
|
18e43e |
|
|
|
18e43e |
---
|
|
|
18e43e |
|
|
|
18e43e |
diff --git a/src/gp_export.c b/src/gp_export.c
|
|
|
18e43e |
index aa0a8ec..dbfddeb 100644
|
|
|
18e43e |
--- a/src/gp_export.c
|
|
|
18e43e |
+++ b/src/gp_export.c
|
|
|
18e43e |
@@ -193,9 +193,15 @@ done:
|
|
|
18e43e |
return ret_maj;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
-/* We need to include a length in our payloads because krb5_c_decrypt() will
|
|
|
18e43e |
- * pad the contents for some enctypes, and gss_import_cred() doesn't like
|
|
|
18e43e |
- * having extra bytes on tokens. */
|
|
|
18e43e |
+#define ENC_MIN_PAD_LEN 8
|
|
|
18e43e |
+
|
|
|
18e43e |
+/* We need to pad our payloads because krb5_c_decrypt() may pad the
|
|
|
18e43e |
+ * contents for some enctypes, and gss_import_cred() doesn't like
|
|
|
18e43e |
+ * having extra bytes on tokens.
|
|
|
18e43e |
+ * Explicit padding and depadding is used in order to maintain backwards
|
|
|
18e43e |
+ * compatibility over upgrades (and downgrades), it would have been
|
|
|
18e43e |
+ * better if we simply had a better formatting of the returned blob
|
|
|
18e43e |
+ * so we could simply change a "blob version" number */
|
|
|
18e43e |
static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
size_t len, void *buf, octet_string *out)
|
|
|
18e43e |
{
|
|
|
18e43e |
@@ -203,8 +209,9 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
krb5_data data_in;
|
|
|
18e43e |
krb5_enc_data enc_handle;
|
|
|
18e43e |
size_t cipherlen;
|
|
|
18e43e |
- char *packed = NULL;
|
|
|
18e43e |
- uint32_t netlen;
|
|
|
18e43e |
+ size_t padcheck;
|
|
|
18e43e |
+ uint8_t pad = 0;
|
|
|
18e43e |
+ char *padded = NULL;
|
|
|
18e43e |
|
|
|
18e43e |
if (len > (uint32_t)(-1)) {
|
|
|
18e43e |
/* Needs to fit in 4 bytes of payload, so... */
|
|
|
18e43e |
@@ -212,28 +219,72 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
goto done;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
- packed = malloc(len);
|
|
|
18e43e |
- if (!packed) {
|
|
|
18e43e |
- ret = errno;
|
|
|
18e43e |
+ ret = krb5_c_encrypt_length(context,
|
|
|
18e43e |
+ key->enctype,
|
|
|
18e43e |
+ len, &cipherlen);
|
|
|
18e43e |
+ if (ret) {
|
|
|
18e43e |
goto done;
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
- netlen = htonl(len);
|
|
|
18e43e |
- memcpy(packed, (uint8_t *)&netlen, 4);
|
|
|
18e43e |
- memcpy(packed + 4, buf, len);
|
|
|
18e43e |
-
|
|
|
18e43e |
- data_in.length = len + 4;
|
|
|
18e43e |
- data_in.data = packed;
|
|
|
18e43e |
-
|
|
|
18e43e |
- memset(&enc_handle, '\0', sizeof(krb5_enc_data));
|
|
|
18e43e |
-
|
|
|
18e43e |
+ /* try again with len + 1 to see if padding is required */
|
|
|
18e43e |
ret = krb5_c_encrypt_length(context,
|
|
|
18e43e |
key->enctype,
|
|
|
18e43e |
- data_in.length,
|
|
|
18e43e |
- &cipherlen);
|
|
|
18e43e |
+ len + 1, &padcheck);
|
|
|
18e43e |
if (ret) {
|
|
|
18e43e |
goto done;
|
|
|
18e43e |
}
|
|
|
18e43e |
+ if (padcheck == cipherlen) {
|
|
|
18e43e |
+ int i;
|
|
|
18e43e |
+ /* padding required */
|
|
|
18e43e |
+ pad = ENC_MIN_PAD_LEN;
|
|
|
18e43e |
+ /* always add enough padding that it makes it extremely unlikley
|
|
|
18e43e |
+ * legitimate plaintext will be incorrectly depadded in the
|
|
|
18e43e |
+ * decrypt function */
|
|
|
18e43e |
+ ret = krb5_c_encrypt_length(context,
|
|
|
18e43e |
+ key->enctype,
|
|
|
18e43e |
+ len + pad, &cipherlen);
|
|
|
18e43e |
+ if (ret) {
|
|
|
18e43e |
+ goto done;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ /* we support only block sizes up to 16 bytes as this is the largest
|
|
|
18e43e |
+ * supported block size in krb ciphers for now */
|
|
|
18e43e |
+ for (i = 0; i < 15; i++) {
|
|
|
18e43e |
+ /* find the point at which padcheck increases, that's when we
|
|
|
18e43e |
+ * cross a blocksize boundary internally and we can calculate
|
|
|
18e43e |
+ * the padding that will be used */
|
|
|
18e43e |
+ ret = krb5_c_encrypt_length(context,
|
|
|
18e43e |
+ key->enctype,
|
|
|
18e43e |
+ len + pad + i + 1, &padcheck);
|
|
|
18e43e |
+ if (ret) {
|
|
|
18e43e |
+ goto done;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ if (padcheck > cipherlen) {
|
|
|
18e43e |
+ pad += i;
|
|
|
18e43e |
+ break;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ if (i > 15) {
|
|
|
18e43e |
+ ret = EINVAL;
|
|
|
18e43e |
+ goto done;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+
|
|
|
18e43e |
+ if (pad != 0) {
|
|
|
18e43e |
+ padded = malloc(len + pad);
|
|
|
18e43e |
+ if (!padded) {
|
|
|
18e43e |
+ ret = errno;
|
|
|
18e43e |
+ goto done;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+
|
|
|
18e43e |
+ memcpy(padded, buf, len);
|
|
|
18e43e |
+ memset(padded + len, pad, pad);
|
|
|
18e43e |
+
|
|
|
18e43e |
+ data_in.length = len + pad;
|
|
|
18e43e |
+ data_in.data = padded;
|
|
|
18e43e |
+ } else {
|
|
|
18e43e |
+ data_in.length = len;
|
|
|
18e43e |
+ data_in.data = buf;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
|
|
|
18e43e |
enc_handle.ciphertext.length = cipherlen;
|
|
|
18e43e |
enc_handle.ciphertext.data = malloc(enc_handle.ciphertext.length);
|
|
|
18e43e |
@@ -261,7 +312,7 @@ static int gp_encrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
done:
|
|
|
18e43e |
- free(packed);
|
|
|
18e43e |
+ free(padded);
|
|
|
18e43e |
free(enc_handle.ciphertext.data);
|
|
|
18e43e |
return ret;
|
|
|
18e43e |
}
|
|
|
18e43e |
@@ -273,7 +324,8 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
int ret;
|
|
|
18e43e |
krb5_data data_out;
|
|
|
18e43e |
krb5_enc_data enc_handle;
|
|
|
18e43e |
- uint32_t netlen;
|
|
|
18e43e |
+ uint8_t pad;
|
|
|
18e43e |
+ int i, j;
|
|
|
18e43e |
|
|
|
18e43e |
memset(&enc_handle, '\0', sizeof(krb5_enc_data));
|
|
|
18e43e |
|
|
|
18e43e |
@@ -295,9 +347,19 @@ static int gp_decrypt_buffer(krb5_context context, krb5_keyblock *key,
|
|
|
18e43e |
}
|
|
|
18e43e |
|
|
|
18e43e |
/* And handle the padding. */
|
|
|
18e43e |
- memcpy(&netlen, buf, 4);
|
|
|
18e43e |
- *len = ntohl(netlen);
|
|
|
18e43e |
- memmove(buf, buf + 4, *len);
|
|
|
18e43e |
+ i = data_out.length - 1;
|
|
|
18e43e |
+ pad = data_out.data[i];
|
|
|
18e43e |
+ if (pad >= ENC_MIN_PAD_LEN && pad < i) {
|
|
|
18e43e |
+ j = pad;
|
|
|
18e43e |
+ while (j > 0) {
|
|
|
18e43e |
+ j--;
|
|
|
18e43e |
+ if (pad != data_out.data[i - j]) break;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ if (j == 0) {
|
|
|
18e43e |
+ data_out.length -= pad;
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ }
|
|
|
18e43e |
+ *len = data_out.length;
|
|
|
18e43e |
|
|
|
18e43e |
return 0;
|
|
|
18e43e |
}
|
|
|
18e43e |
|