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