krb5 commit: Fix gss_process_context_token() [CVE-2014-5352]

Greg Hudson ghudson at mit.edu
Wed Feb 4 14:26:26 EST 2015


https://github.com/krb5/krb5/commit/82dc33da50338ac84c7b4102dc6513d897d0506a
commit 82dc33da50338ac84c7b4102dc6513d897d0506a
Author: Greg Hudson <ghudson at mit.edu>
Date:   Wed Nov 5 11:58:04 2014 -0500

    Fix gss_process_context_token() [CVE-2014-5352]
    
    [MITKRB5-SA-2015-001] The krb5 gss_process_context_token() should not
    actually delete the context; that leaves the caller with a dangling
    pointer and no way to know that it is invalid.  Instead, mark the
    context as terminated, and check for terminated contexts in the GSS
    functions which expect established contexts.  Also add checks in
    export_sec_context and pseudo_random, and adjust t_prf.c for the
    pseudo_random check.
    
    ticket: 8055 (new)
    target_version: 1.13.1
    tags: pullup

 src/lib/gssapi/krb5/context_time.c          |    2 +-
 src/lib/gssapi/krb5/export_sec_context.c    |    5 +++++
 src/lib/gssapi/krb5/gssapiP_krb5.h          |    1 +
 src/lib/gssapi/krb5/gssapi_krb5.c           |    2 +-
 src/lib/gssapi/krb5/inq_context.c           |    2 +-
 src/lib/gssapi/krb5/k5seal.c                |    2 +-
 src/lib/gssapi/krb5/k5sealiov.c             |    2 +-
 src/lib/gssapi/krb5/k5unseal.c              |    2 +-
 src/lib/gssapi/krb5/k5unsealiov.c           |    2 +-
 src/lib/gssapi/krb5/lucid_context.c         |    5 +++++
 src/lib/gssapi/krb5/prf.c                   |    4 ++++
 src/lib/gssapi/krb5/process_context_token.c |   17 ++++++++++++-----
 src/lib/gssapi/krb5/wrap_size_limit.c       |    2 +-
 src/tests/gssapi/t_prf.c                    |    1 +
 14 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/src/lib/gssapi/krb5/context_time.c b/src/lib/gssapi/krb5/context_time.c
index b3d1db0..a18cfb0 100644
--- a/src/lib/gssapi/krb5/context_time.c
+++ b/src/lib/gssapi/krb5/context_time.c
@@ -40,7 +40,7 @@ krb5_gss_context_time(minor_status, context_handle, time_rec)
 
     ctx = (krb5_gss_ctx_id_rec *) context_handle;
 
-    if (! ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return(GSS_S_NO_CONTEXT);
     }
diff --git a/src/lib/gssapi/krb5/export_sec_context.c b/src/lib/gssapi/krb5/export_sec_context.c
index 18a3a34..1b3de68 100644
--- a/src/lib/gssapi/krb5/export_sec_context.c
+++ b/src/lib/gssapi/krb5/export_sec_context.c
@@ -45,6 +45,11 @@ krb5_gss_export_sec_context(minor_status, context_handle, interprocess_token)
     *minor_status = 0;
 
     ctx = (krb5_gss_ctx_id_t) *context_handle;
+    if (ctx->terminated) {
+        *minor_status = KG_CTX_INCOMPLETE;
+        return (GSS_S_NO_CONTEXT);
+    }
+
     context = ctx->k5_context;
     kret = krb5_gss_ser_init(context);
     if (kret)
diff --git a/src/lib/gssapi/krb5/gssapiP_krb5.h b/src/lib/gssapi/krb5/gssapiP_krb5.h
index 7e807cc..a0e8625 100644
--- a/src/lib/gssapi/krb5/gssapiP_krb5.h
+++ b/src/lib/gssapi/krb5/gssapiP_krb5.h
@@ -206,6 +206,7 @@ typedef struct _krb5_gss_ctx_id_rec {
     unsigned int established : 1;
     unsigned int have_acceptor_subkey : 1;
     unsigned int seed_init : 1;  /* XXX tested but never actually set */
+    unsigned int terminated : 1;
     OM_uint32 gss_flags;
     unsigned char seed[16];
     krb5_gss_name_t here;
diff --git a/src/lib/gssapi/krb5/gssapi_krb5.c b/src/lib/gssapi/krb5/gssapi_krb5.c
index 6456b23..77b7fff 100644
--- a/src/lib/gssapi/krb5/gssapi_krb5.c
+++ b/src/lib/gssapi/krb5/gssapi_krb5.c
@@ -369,7 +369,7 @@ krb5_gss_inquire_sec_context_by_oid (OM_uint32 *minor_status,
 
     ctx = (krb5_gss_ctx_id_rec *) context_handle;
 
-    if (!ctx->established)
+    if (ctx->terminated || !ctx->established)
         return GSS_S_NO_CONTEXT;
 
     for (i = 0; i < sizeof(krb5_gss_inquire_sec_context_by_oid_ops)/
diff --git a/src/lib/gssapi/krb5/inq_context.c b/src/lib/gssapi/krb5/inq_context.c
index eacb0fd..096df2a 100644
--- a/src/lib/gssapi/krb5/inq_context.c
+++ b/src/lib/gssapi/krb5/inq_context.c
@@ -105,7 +105,7 @@ krb5_gss_inquire_context(minor_status, context_handle, initiator_name,
 
     ctx = (krb5_gss_ctx_id_rec *) context_handle;
 
-    if (! ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return(GSS_S_NO_CONTEXT);
     }
diff --git a/src/lib/gssapi/krb5/k5seal.c b/src/lib/gssapi/krb5/k5seal.c
index 7665cba..f1c74dd 100644
--- a/src/lib/gssapi/krb5/k5seal.c
+++ b/src/lib/gssapi/krb5/k5seal.c
@@ -342,7 +342,7 @@ kg_seal(minor_status, context_handle, conf_req_flag, qop_req,
 
     ctx = (krb5_gss_ctx_id_rec *) context_handle;
 
-    if (! ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return(GSS_S_NO_CONTEXT);
     }
diff --git a/src/lib/gssapi/krb5/k5sealiov.c b/src/lib/gssapi/krb5/k5sealiov.c
index a129670..b53e348 100644
--- a/src/lib/gssapi/krb5/k5sealiov.c
+++ b/src/lib/gssapi/krb5/k5sealiov.c
@@ -281,7 +281,7 @@ kg_seal_iov(OM_uint32 *minor_status,
     }
 
     ctx = (krb5_gss_ctx_id_rec *)context_handle;
-    if (!ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return GSS_S_NO_CONTEXT;
     }
diff --git a/src/lib/gssapi/krb5/k5unseal.c b/src/lib/gssapi/krb5/k5unseal.c
index 0573958..673c883 100644
--- a/src/lib/gssapi/krb5/k5unseal.c
+++ b/src/lib/gssapi/krb5/k5unseal.c
@@ -492,7 +492,7 @@ kg_unseal(minor_status, context_handle, input_token_buffer,
 
     ctx = (krb5_gss_ctx_id_rec *) context_handle;
 
-    if (! ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return(GSS_S_NO_CONTEXT);
     }
diff --git a/src/lib/gssapi/krb5/k5unsealiov.c b/src/lib/gssapi/krb5/k5unsealiov.c
index f34d802..8b67042 100644
--- a/src/lib/gssapi/krb5/k5unsealiov.c
+++ b/src/lib/gssapi/krb5/k5unsealiov.c
@@ -625,7 +625,7 @@ kg_unseal_iov(OM_uint32 *minor_status,
     OM_uint32 code;
 
     ctx = (krb5_gss_ctx_id_rec *)context_handle;
-    if (!ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return GSS_S_NO_CONTEXT;
     }
diff --git a/src/lib/gssapi/krb5/lucid_context.c b/src/lib/gssapi/krb5/lucid_context.c
index 85df7fd..449e71f 100644
--- a/src/lib/gssapi/krb5/lucid_context.c
+++ b/src/lib/gssapi/krb5/lucid_context.c
@@ -75,6 +75,11 @@ gss_krb5int_export_lucid_sec_context(
     *minor_status = 0;
     *data_set = GSS_C_NO_BUFFER_SET;
 
+    if (ctx->terminated || !ctx->established) {
+        *minor_status = KG_CTX_INCOMPLETE;
+        return GSS_S_NO_CONTEXT;
+    }
+
     retval = generic_gss_oid_decompose(minor_status,
                                        GSS_KRB5_EXPORT_LUCID_SEC_CONTEXT_OID,
                                        GSS_KRB5_EXPORT_LUCID_SEC_CONTEXT_OID_LENGTH,
diff --git a/src/lib/gssapi/krb5/prf.c b/src/lib/gssapi/krb5/prf.c
index e19291f..e897074 100644
--- a/src/lib/gssapi/krb5/prf.c
+++ b/src/lib/gssapi/krb5/prf.c
@@ -58,6 +58,10 @@ krb5_gss_pseudo_random(OM_uint32 *minor_status,
     ns.data = NULL;
 
     ctx = (krb5_gss_ctx_id_t)context;
+    if (ctx->terminated || !ctx->established) {
+        *minor_status = KG_CTX_INCOMPLETE;
+        return GSS_S_NO_CONTEXT;
+    }
 
     switch (prf_key) {
     case GSS_C_PRF_KEY_FULL:
diff --git a/src/lib/gssapi/krb5/process_context_token.c b/src/lib/gssapi/krb5/process_context_token.c
index ae33180..a672f48 100644
--- a/src/lib/gssapi/krb5/process_context_token.c
+++ b/src/lib/gssapi/krb5/process_context_token.c
@@ -39,11 +39,18 @@ krb5_gss_process_context_token(minor_status, context_handle,
 
     ctx = (krb5_gss_ctx_id_t) context_handle;
 
-    if (! ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return(GSS_S_NO_CONTEXT);
     }
 
+    /* We only support context deletion tokens for now, and RFC 4121 does not
+     * define a context deletion token. */
+    if (ctx->proto) {
+        *minor_status = 0;
+        return(GSS_S_DEFECTIVE_TOKEN);
+    }
+
     /* "unseal" the token */
 
     if (GSS_ERROR(majerr = kg_unseal(minor_status, context_handle,
@@ -52,8 +59,8 @@ krb5_gss_process_context_token(minor_status, context_handle,
                                      KG_TOK_DEL_CTX)))
         return(majerr);
 
-    /* that's it.  delete the context */
-
-    return(krb5_gss_delete_sec_context(minor_status, &context_handle,
-                                       GSS_C_NO_BUFFER));
+    /* Mark the context as terminated, but do not delete it (as that would
+     * leave the caller with a dangling context handle). */
+    ctx->terminated = 1;
+    return(GSS_S_COMPLETE);
 }
diff --git a/src/lib/gssapi/krb5/wrap_size_limit.c b/src/lib/gssapi/krb5/wrap_size_limit.c
index 7bc4221..ed5c599 100644
--- a/src/lib/gssapi/krb5/wrap_size_limit.c
+++ b/src/lib/gssapi/krb5/wrap_size_limit.c
@@ -95,7 +95,7 @@ krb5_gss_wrap_size_limit(minor_status, context_handle, conf_req_flag,
     }
 
     ctx = (krb5_gss_ctx_id_rec *) context_handle;
-    if (! ctx->established) {
+    if (ctx->terminated || !ctx->established) {
         *minor_status = KG_CTX_INCOMPLETE;
         return(GSS_S_NO_CONTEXT);
     }
diff --git a/src/tests/gssapi/t_prf.c b/src/tests/gssapi/t_prf.c
index 254f8fb..7f04899 100644
--- a/src/tests/gssapi/t_prf.c
+++ b/src/tests/gssapi/t_prf.c
@@ -127,6 +127,7 @@ main(int argc, char *argv[])
     uctx.mech_type = &mech_krb5;
     uctx.internal_ctx_id = (gss_ctx_id_t)&kgctx;
     kgctx.k5_context = NULL;
+    kgctx.established = 1;
     kgctx.have_acceptor_subkey = 1;
     kb1.contents = k1buf;
     kb2.contents = k2buf;


More information about the cvs-krb5 mailing list