[krb5] Backport fixes for krb5_copy_context
Nalin Dahyabhai
nalin at fedoraproject.org
Wed Dec 18 22:39:07 UTC 2013
commit 82c5b9f9b2b638bd04c8062c4113a4479640af81
Author: Nalin Dahyabhai <nalin at dahyabhai.net>
Date: Wed Dec 18 17:38:54 2013 -0500
Backport fixes for krb5_copy_context
- backport fixes to krb5_copy_context (RT#7807, #1044735/#1044739)
krb5-1.12-copy_context.patch | 302 ++++++++++++++++++++++++++++++++++++++++++
krb5.spec | 7 +-
2 files changed, 308 insertions(+), 1 deletions(-)
---
diff --git a/krb5-1.12-copy_context.patch b/krb5-1.12-copy_context.patch
new file mode 100644
index 0000000..dde811f
--- /dev/null
+++ b/krb5-1.12-copy_context.patch
@@ -0,0 +1,302 @@
+Adjusted for 1.12, which still have vtbl and locate_fptrs fields, and drop the
+hunk that touched .gitignore.
+
+commit c452644d91d57d8b05ef396a029e34d0c7a48920
+Author: Greg Hudson <ghudson at mit.edu>
+Date: Wed Dec 18 15:03:03 2013 -0500
+
+ Fix krb5_copy_context
+
+ krb5_copy_context has been broken since 1.8 (it broke in r22456)
+ because k5_copy_etypes crashes on null enctype lists. Subsequent
+ additions to the context structure were not reflected in
+ krb5_copy_context, creating double-free bugs. Make k5_copy_etypes
+ handle null input and account for all new fields in krb5_copy_context.
+ Reported by Arran Cudbard-Bell.
+
+ ticket: 7807 (new)
+ target_version: 1.12.1
+ tags: pullup
+
+diff --git a/src/lib/krb5/krb/copy_ctx.c b/src/lib/krb5/krb/copy_ctx.c
+index 0bc92f8..4237023 100644
+--- a/src/lib/krb5/krb/copy_ctx.c
++++ b/src/lib/krb5/krb/copy_ctx.c
+@@ -77,13 +77,22 @@ krb5_copy_context(krb5_context ctx, krb5_context *nctx_out)
+ nctx->ser_ctx_count = 0;
+ nctx->ser_ctx = NULL;
+ nctx->prompt_types = NULL;
++ nctx->preauth_context = NULL;
++ nctx->ccselect_handles = NULL;
++ nctx->localauth_handles = NULL;
++ nctx->hostrealm_handles = NULL;
++ nctx->kdblog_context = NULL;
++ nctx->trace_callback = NULL;
++ nctx->trace_callback_data = NULL;
++ nctx->plugin_base_dir = NULL;
+ nctx->os_context.default_ccname = NULL;
+
+ memset(&nctx->libkrb5_plugins, 0, sizeof(nctx->libkrb5_plugins));
+ nctx->vtbl = NULL;
+ nctx->locate_fptrs = NULL;
+
+ memset(&nctx->err, 0, sizeof(nctx->err));
++ memset(&nctx->plugins, 0, sizeof(nctx->plugins));
+
+ ret = k5_copy_etypes(ctx->in_tkt_etypes, &nctx->in_tkt_etypes);
+ if (ret)
+@@ -101,6 +109,11 @@ krb5_copy_context(krb5_context ctx, krb5_context *nctx_out)
+ ret = krb5_get_profile(ctx, &nctx->profile);
+ if (ret)
+ goto errout;
++ nctx->plugin_base_dir = strdup(ctx->plugin_base_dir);
++ if (nctx->plugin_base_dir == NULL) {
++ ret = ENOMEM;
++ goto errout;
++ }
+
+ errout:
+ if (ret) {
+diff --git a/src/lib/krb5/krb/etype_list.c b/src/lib/krb5/krb/etype_list.c
+index 9efe2e0..71f664f 100644
+--- a/src/lib/krb5/krb/etype_list.c
++++ b/src/lib/krb5/krb/etype_list.c
+@@ -49,6 +49,8 @@ k5_copy_etypes(const krb5_enctype *old_list, krb5_enctype **new_list)
+ krb5_enctype *list;
+
+ *new_list = NULL;
++ if (old_list == NULL)
++ return 0;
+ count = k5_count_etypes(old_list);
+ list = malloc(sizeof(krb5_enctype) * (count + 1));
+ if (list == NULL)
+
+commit b78c3c8c5025aec870d20472f80d4a652062f921
+Author: Greg Hudson <ghudson at mit.edu>
+Date: Wed Dec 18 13:08:25 2013 -0500
+
+ Add a test program for krb5_copy_context
+
+ This test program isn't completely proof against the kind of mistakes
+ we've made with krb5_copy_context in the past, but it at least
+ exercises krb5_copy_context and can detect some kinds of bugs.
+
+ ticket: 7807
+
+diff --git a/src/lib/krb5/krb/Makefile.in b/src/lib/krb5/krb/Makefile.in
+index 7d1682d..3b58219 100644
+--- a/src/lib/krb5/krb/Makefile.in
++++ b/src/lib/krb5/krb/Makefile.in
+@@ -349,6 +349,7 @@ SRCS= $(srcdir)/addr_comp.c \
+ $(srcdir)/t_expire_warn.c \
+ $(srcdir)/t_authdata.c \
+ $(srcdir)/t_cc_config.c \
++ $(srcdir)/t_copy_context.c \
+ $(srcdir)/t_in_ccache.c \
+ $(srcdir)/t_response_items.c \
+ $(srcdir)/t_vfy_increds.c
+@@ -429,11 +430,14 @@ t_in_ccache: t_in_ccache.o $(KRB5_BASE_DEPLIBS)
+ t_cc_config: t_cc_config.o $(KRB5_BASE_DEPLIBS)
+ $(CC_LINK) -o $@ t_cc_config.o $(KRB5_BASE_LIBS)
+
++t_copy_context: t_copy_context.o $(KRB5_BASE_DEPLIBS)
++ $(CC_LINK) -o $@ t_copy_context.o $(KRB5_BASE_LIBS)
++
+ t_response_items: t_response_items.o response_items.o $(KRB5_BASE_DEPLIBS)
+ $(CC_LINK) -o $@ t_response_items.o response_items.o $(KRB5_BASE_LIBS)
+
+ TEST_PROGS= t_walk_rtree t_kerb t_ser t_deltat t_expand t_authdata t_pac \
+- t_in_ccache t_cc_config \
++ t_in_ccache t_cc_config t_copy_context \
+ t_princ t_etypes t_vfy_increds t_response_items
+
+ check-unix:: $(TEST_PROGS)
+@@ -473,6 +477,8 @@ check-unix:: $(TEST_PROGS)
+ $(RUN_SETUP) $(VALGRIND) ./t_princ
+ $(RUN_SETUP) $(VALGRIND) ./t_etypes
+ $(RUN_SETUP) $(VALGRIND) ./t_response_items
++ KRB5_CONFIG=$(srcdir)/t_krb5.conf ; export KRB5_CONFIG ;\
++ $(RUN_SETUP) $(VALGRIND) ./t_copy_context
+
+ check-pytests:: t_expire_warn t_vfy_increds
+ $(RUNPYTEST) $(srcdir)/t_expire_warn.py $(PYTESTFLAGS)
+@@ -491,6 +497,7 @@ clean::
+ $(OUTPRE)t_princ$(EXEEXT) $(OUTPRE)t_princ.$(OBJEXT) \
+ $(OUTPRE)t_authdata$(EXEEXT) $(OUTPRE)t_authdata.$(OBJEXT) \
+ $(OUTPRE)t_cc_config$(EXEEXT) $(OUTPRE)t_cc_config.$(OBJEXT) \
++ $(OUTPRE)t_copy_context(EXEEXT) $(OUTPRE)t_copy_context.$(OBJEXT) \
+ $(OUTPRE)t_in_ccache$(EXEEXT) $(OUTPRE)t_in_ccache.$(OBJEXT) \
+ $(OUTPRE)t_ad_fx_armor$(EXEEXT) $(OUTPRE)t_ad_fx_armor.$(OBJEXT) \
+ $(OUTPRE)t_vfy_increds$(EXEEXT) $(OUTPRE)t_vfy_increds.$(OBJEXT) \
+diff --git a/src/lib/krb5/krb/t_copy_context.c b/src/lib/krb5/krb/t_copy_context.c
+new file mode 100644
+index 0000000..522fa0c
+--- /dev/null
++++ b/src/lib/krb5/krb/t_copy_context.c
+@@ -0,0 +1,166 @@
++/* -*- mode: c; c-basic-offset: 4; indent-tabs-mode: nil -*- */
++/* lib/krb5/krb/t_copy_context.C - Test program for krb5_copy_context */
++/*
++ * Copyright (C) 2013 by the Massachusetts Institute of Technology.
++ * All rights reserved.
++ *
++ * Redistribution and use in source and binary forms, with or without
++ * modification, are permitted provided that the following conditions
++ * are met:
++ *
++ * * Redistributions of source code must retain the above copyright
++ * notice, this list of conditions and the following disclaimer.
++ *
++ * * Redistributions in binary form must reproduce the above copyright
++ * notice, this list of conditions and the following disclaimer in
++ * the documentation and/or other materials provided with the
++ * distribution.
++ *
++ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
++ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
++ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
++ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
++ * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
++ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
++ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
++ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
++ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
++ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
++ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
++ * OF THE POSSIBILITY OF SUCH DAMAGE.
++ */
++
++#include <k5-int.h>
++
++static void
++trace(krb5_context ctx, const krb5_trace_info *info, void *data)
++{
++}
++
++static void
++check(int cond)
++{
++ if (!cond)
++ abort();
++}
++
++static void
++compare_string(const char *str1, const char *str2)
++{
++ check((str1 == NULL) == (str2 == NULL));
++ if (str1 != NULL)
++ check(strcmp(str1, str2) == 0);
++}
++
++static void
++compare_etypes(krb5_enctype *list1, krb5_enctype *list2)
++{
++ check((list1 == NULL) == (list2 == NULL));
++ if (list1 == NULL)
++ return;
++ while (*list1 != ENCTYPE_NULL && *list1 == *list2)
++ list1++, list2++;
++ check(*list1 == *list2);
++}
++
++/* Check that the context c is a valid copy of the reference context r. */
++static void
++check_context(krb5_context c, krb5_context r)
++{
++ int i;
++
++ /* Check fields which should have been propagated from r. */
++ compare_etypes(c->in_tkt_etypes, r->in_tkt_etypes);
++ compare_etypes(c->tgs_etypes, r->tgs_etypes);
++ check(c->os_context.time_offset == r->os_context.time_offset);
++ check(c->os_context.usec_offset == r->os_context.usec_offset);
++ check(c->os_context.os_flags == r->os_context.os_flags);
++ compare_string(c->os_context.default_ccname, r->os_context.default_ccname);
++ check(c->clockskew == r->clockskew);
++ check(c->kdc_req_sumtype == r->kdc_req_sumtype);
++ check(c->default_ap_req_sumtype == r->default_ap_req_sumtype);
++ check(c->default_safe_sumtype == r->default_safe_sumtype);
++ check(c->kdc_default_options == r->kdc_default_options);
++ check(c->library_options == r->library_options);
++ check(c->profile_secure == r->profile_secure);
++ check(c->fcc_default_format == r->fcc_default_format);
++ check(c->udp_pref_limit == r->udp_pref_limit);
++ check(c->use_conf_ktypes == r->use_conf_ktypes);
++ check(c->allow_weak_crypto == r->allow_weak_crypto);
++ check(c->ignore_acceptor_hostname == r->ignore_acceptor_hostname);
++ check(c->dns_canonicalize_hostname == r->dns_canonicalize_hostname);
++ compare_string(c->plugin_base_dir, r->plugin_base_dir);
++
++ /* Check fields which don't propagate. */
++ check(c->dal_handle == NULL);
++ check(c->ser_ctx_count == 0);
++ check(c->ser_ctx == NULL);
++ check(c->prompt_types == NULL);
++ check(c->libkrb5_plugins.files == NULL);
++ check(c->preauth_context == NULL);
++ check(c->ccselect_handles == NULL);
++ check(c->localauth_handles == NULL);
++ check(c->hostrealm_handles == NULL);
++ check(c->err.code == 0);
++ check(c->err.msg == NULL);
++ check(c->kdblog_context == NULL);
++ check(c->trace_callback == NULL);
++ check(c->trace_callback_data == NULL);
++ for (i = 0; i < PLUGIN_NUM_INTERFACES; i++) {
++ check(c->plugins[i].modules == NULL);
++ check(!c->plugins[i].configured);
++ }
++}
++
++int
++main(int argc, char **argv)
++{
++ krb5_context ctx, ctx2;
++ krb5_plugin_initvt_fn *mods;
++ const krb5_enctype etypes1[] = { ENCTYPE_DES3_CBC_SHA1, 0 };
++ const krb5_enctype etypes2[] = { ENCTYPE_AES128_CTS_HMAC_SHA1_96,
++ ENCTYPE_AES256_CTS_HMAC_SHA1_96, 0 };
++ krb5_prompt_type ptypes[] = { KRB5_PROMPT_TYPE_PASSWORD };
++
++ /* Copy a default context and verify the result. */
++ check(krb5_init_context(&ctx) == 0);
++ check(krb5_copy_context(ctx, &ctx2) == 0);
++ check_context(ctx2, ctx);
++ krb5_free_context(ctx2);
++
++ /* Set non-default values for all of the propagated fields in ctx. */
++ ctx->allow_weak_crypto = TRUE;
++ check(krb5_set_default_in_tkt_ktypes(ctx, etypes1) == 0);
++ check(krb5_set_default_tgs_enctypes(ctx, etypes2) == 0);
++ check(krb5_set_debugging_time(ctx, 1234, 5678) == 0);
++ check(krb5_cc_set_default_name(ctx, "defccname") == 0);
++ check(krb5_set_default_realm(ctx, "defrealm") == 0);
++ ctx->clockskew = 18;
++ ctx->kdc_req_sumtype = CKSUMTYPE_NIST_SHA;
++ ctx->default_ap_req_sumtype = CKSUMTYPE_HMAC_SHA1_96_AES128;
++ ctx->default_safe_sumtype = CKSUMTYPE_HMAC_SHA1_96_AES256;
++ ctx->kdc_default_options = KDC_OPT_FORWARDABLE;
++ ctx->library_options = 0;
++ ctx->profile_secure = TRUE;
++ ctx->udp_pref_limit = 2345;
++ ctx->use_conf_ktypes = TRUE;
++ ctx->ignore_acceptor_hostname = TRUE;
++ ctx->dns_canonicalize_hostname = FALSE;
++ free(ctx->plugin_base_dir);
++ check((ctx->plugin_base_dir = strdup("/a/b/c/d")) != NULL);
++
++ /* Also set some of the non-propagated fields. */
++ ctx->prompt_types = ptypes;
++ check(k5_plugin_load_all(ctx, PLUGIN_INTERFACE_PWQUAL, &mods) == 0);
++ k5_plugin_free_modules(ctx, mods);
++ krb5_set_error_message(ctx, ENOMEM, "nooooooooo");
++ krb5_set_trace_callback(ctx, trace, ctx);
++
++ /* Copy the intentionally messy context and verify the result. */
++ check(krb5_copy_context(ctx, &ctx2) == 0);
++ check_context(ctx2, ctx);
++ krb5_free_context(ctx2);
++
++ krb5_free_context(ctx);
++ return 0;
++}
diff --git a/krb5.spec b/krb5.spec
index dbf89a7..16efbd7 100644
--- a/krb5.spec
+++ b/krb5.spec
@@ -41,7 +41,7 @@
Summary: The Kerberos network authentication system
Name: krb5
Version: 1.12
-Release: 2%{?dist}
+Release: 3%{?dist}
# Maybe we should explode from the now-available-to-everybody tarball instead?
# http://web.mit.edu/kerberos/dist/krb5/1.12/krb5-1.12-signed.tar
Source0: krb5-%{version}.tar.gz
@@ -94,6 +94,7 @@ Patch135: krb5-master-no-malloc0.patch
Patch136: krb5-master-ignore-empty-unnecessary-final-token.patch
Patch137: krb5-master-gss_oid_leak.patch
Patch138: krb5-master-keytab_close.patch
+Patch139: krb5-1.12-copy_context.patch
License: MIT
URL: http://web.mit.edu/kerberos/www/
@@ -308,6 +309,7 @@ ln -s NOTICE LICENSE
%patch136 -p1 -b .ignore-empty-unnecessary-final-token
%patch137 -p1 -b .gss_oid_leak
%patch138 -p1 -b .keytab_close
+%patch139 -p1 -b .copy_context
# Apply when the hard-wired or configured default location is
# DIR:/run/user/%%{uid}/krb5cc.
@@ -962,6 +964,9 @@ exit 0
%{_sbindir}/uuserver
%changelog
+* Wed Dec 18 2013 Nalin Dahyabhai <nalin at redhat.com> - 1.12-3
+- backport fixes to krb5_copy_context (RT#7807, #1044735/#1044739)
+
* Wed Dec 18 2013 Nalin Dahyabhai <nalin at redhat.com> - 1.12-2
- pull in fix from master to return a NULL pointer rather than allocating
zero bytes of memory if we read a zero-length input token (RT#7794, part of
More information about the scm-commits
mailing list