From bdcb3741db27d6b773ce7cdf05aab063a70ea100 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Sun, 27 May 2018 20:59:59 -0700 Subject: Update to rra-c-util 7.2 and C TAP Harness 4.3 Update to rra-c-util 7.2: * Improve configure output for krb5-config testing. * Define UINT32_MAX for systems that don't have it. * Add SPDX-License-Identifier headers to all substantial source files. * Fix new warnings from GCC 7 and Clang warnings. * Require Test::Strict 0.25 or later to run those tests. * Fix off-by-one error in return-value checks for snprintf. * Use Autoconf to probe for supported warning flags. * Fix running module-version-t -u with current versions of Perl. * Use C_TAP_SOURCE and C_TAP_BUILD instead of SOURCE and BUILD. Update to C TAP Harness 4.3: * Add support for valgrind and libtool in test lists. * Report test failures as left and right, not wanted and expected. * Fix string comparisons with NULL pointers and the string "(null)". * Add SPDX-License-Identifier headers to all substantial source files. * Avoid zero-length realloc allocations in breallocarray. * Fix new warnings from GCC 7 and Clang warnings. * Use C_TAP_SOURCE and C_TAP_BUILD instead of SOURCE and BUILD. --- portable/krb5.h | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) (limited to 'portable/krb5.h') diff --git a/portable/krb5.h b/portable/krb5.h index 34f960e..63a2e9f 100644 --- a/portable/krb5.h +++ b/portable/krb5.h @@ -17,17 +17,19 @@ * krb5_free_unparsed_name() for both APIs since it's the most specific call. * * The canonical version of this file is maintained in the rra-c-util package, - * which can be found at . + * which can be found at . * * Written by Russ Allbery + * Copyright 2015, 2017 Russ Allbery + * Copyright 2010-2014 + * The Board of Trustees of the Leland Stanford Junior University * - * The authors hereby relinquish any claim to any copyright that they may have - * in this work, whether granted under contract or by operation of law or - * international treaty, and hereby commit to the public, at large, that they - * shall not, at any time in the future, seek to enforce any copyright in this - * work against any person or entity, or prevent any person or entity from - * copying, publishing, distributing or creating derivative works of this - * work. + * Copying and distribution of this file, with or without modification, are + * permitted in any medium without royalty provided the copyright notice and + * this notice are preserved. This file is offered as-is, without any + * warranty. + * + * SPDX-License-Identifier: FSFAP */ #ifndef PORTABLE_KRB5_H @@ -56,6 +58,18 @@ BEGIN_DECLS /* Default to a hidden visibility for all portability functions. */ #pragma GCC visibility push(hidden) +/* + * AIX included Kerberos includes the profile library but not the + * krb5_appdefault functions, so we provide replacements that we have to + * prototype. + */ +#ifndef HAVE_KRB5_APPDEFAULT_STRING +void krb5_appdefault_boolean(krb5_context, const char *, const krb5_data *, + const char *, int, int *); +void krb5_appdefault_string(krb5_context, const char *, const krb5_data *, + const char *, const char *, char **); +#endif + /* * krb5_{get,free}_error_message are the preferred APIs for both current MIT * and current Heimdal, but there are tons of older APIs we may have to fall -- cgit v1.2.3 From f3f09aab44117d8eb97811421b2685c295d3d585 Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Mon, 28 May 2018 15:06:46 -0700 Subject: Pass realm to krb5_appdefault_* functions When getting configuration values from krb5.conf, pass the default local realm into the Kerberos appdefault functions. This will produce more correct results with krb5.conf files that specify wallet configuration for multiple realms. --- NEWS | 5 ++ client/options.c | 126 ++++++++++++++++++++++++++++++++++++++------------ client/wallet-rekey.c | 1 + client/wallet.c | 1 + configure.ac | 4 +- portable/krb5.h | 9 ++++ 6 files changed, 116 insertions(+), 30 deletions(-) (limited to 'portable/krb5.h') diff --git a/NEWS b/NEWS index 23137e2..1af55fc 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,11 @@ wallet 1.4 (unreleased) service principal, and truncate and make unique long names in AD if necessary. This support should still be considered experimental. + When getting configuration values from krb5.conf, pass the default + local realm into the Kerberos appdefault functions. This will produce + more correct results with krb5.conf files that specify wallet + configuration for multiple realms. + Remove stray references to strlcpy and strlcat that broke builds on platforms where those functions are part of libc. Thanks to Karl Kornel for the report. diff --git a/client/options.c b/client/options.c index 3c68cc7..f011b79 100644 --- a/client/options.c +++ b/client/options.c @@ -16,43 +16,98 @@ #include #include +#include + #include +#include /* - * Load a string option from Kerberos appdefaults. This requires an annoying - * workaround because one cannot specify a default value of NULL. + * Load a number option from Kerberos appdefaults. Takes the Kerberos + * context, the realm, the option, and the result location. The native + * interface doesn't support numbers, so we actually read a string and then + * convert. */ static void -default_string(krb5_context ctx, const char *opt, const char *defval, - char **result) +default_number(krb5_context ctx, const char *realm, const char *opt, + long defval, long *result) { - if (defval == NULL) - defval = ""; - krb5_appdefault_string(ctx, "wallet", NULL, opt, defval, result); - if (*result != NULL && (*result)[0] == '\0') { - free(*result); - *result = NULL; + char *tmp = NULL; + char *end; + long value; +#ifdef HAVE_KRB5_REALM + krb5_const_realm rdata = realm; +#else + krb5_data realm_struct; + const krb5_data *rdata; + + if (realm == NULL) + rdata = NULL; + else { + rdata = &realm_struct; + realm_struct.magic = KV5M_DATA; + realm_struct.data = (void *) realm; + realm_struct.length = (unsigned int) strlen(realm); } +#endif + + *result = defval; + krb5_appdefault_string(ctx, "wallet", rdata, opt, "", &tmp); + if (tmp != NULL && tmp[0] != '\0') { + errno = 0; + value = strtol(tmp, &end, 10); + if (errno != 0 || *end != '\0') + warn("invalid number in krb5.conf setting for %s: %s", opt, tmp); + else + *result = value; + } + free(tmp); } /* - * Load a number option from Kerberos appdefaults. The native interface - * doesn't support numbers, so we actually read a string and then convert. + * Load a string option from Kerberos appdefaults. Takes the Kerberos + * context, the realm, the option, and the result location. + * + * This requires an annoying workaround because one cannot specify a default + * value of NULL with MIT Kerberos, since MIT Kerberos unconditionally calls + * strdup on the default value. There's also no way to determine if memory + * allocation failed while parsing or while setting the default value, so we + * don't return an error code. */ static void -default_number(krb5_context ctx, const char *opt, int defval, int *result) +default_string(krb5_context ctx, const char *realm, const char *opt, + const char *defval, char **result) { - char *tmp = NULL; + char *value = NULL; +#ifdef HAVE_KRB5_REALM + krb5_const_realm rdata = realm; +#else + krb5_data realm_struct; + const krb5_data *rdata; - krb5_appdefault_string(ctx, "wallet", NULL, opt, "", &tmp); - if (tmp != NULL && tmp[0] != '\0') - *result = atoi(tmp); - else - *result = defval; - if (tmp != NULL) - free(tmp); + if (realm == NULL) + rdata = NULL; + else { + rdata = &realm_struct; + realm_struct.magic = KV5M_DATA; + realm_struct.data = (void *) realm; + realm_struct.length = (unsigned int) strlen(realm); + } +#endif + + if (defval == NULL) + defval = ""; + krb5_appdefault_string(ctx, "wallet", rdata, opt, defval, &value); + if (value != NULL) { + if (value[0] == '\0') + free(value); + else { + if (*result != NULL) + free(*result); + *result = value; + } + } } @@ -64,15 +119,28 @@ default_number(krb5_context ctx, const char *opt, int defval, int *result) void default_options(krb5_context ctx, struct options *options) { - int port; + long port; + char *realm = NULL; + + /* Having no local realm may be intentional, so don't report an error. */ + krb5_get_default_realm(ctx, &realm); + + /* Load the options. */ + default_string(ctx, realm, "wallet_type", "wallet", &options->type); + default_string(ctx, realm, "wallet_server", WALLET_SERVER, + &options->server); + default_string(ctx, realm, "wallet_principal", NULL, &options->principal); + default_number(ctx, realm, "wallet_port", WALLET_PORT, &port); - default_string(ctx, "wallet_type", "wallet", &options->type); - default_string(ctx, "wallet_server", WALLET_SERVER, &options->server); - default_string(ctx, "wallet_principal", NULL, &options->principal); - default_number(ctx, "wallet_port", WALLET_PORT, &port); - if (port <= 0 || port > 65535) + /* Additional checks on the option values. */ + if (port != WALLET_PORT && (port <= 0 || port > 65535)) { + warn("invalid number in krb5.conf setting for wallet_port: %ld", port); options->port = WALLET_PORT; - else + } else { options->port = (unsigned short) port; - options->user = NULL; + } + + /* Clean up. */ + if (realm != NULL) + krb5_free_default_realm(ctx, realm); } diff --git a/client/wallet-rekey.c b/client/wallet-rekey.c index 2809efc..caab130 100644 --- a/client/wallet-rekey.c +++ b/client/wallet-rekey.c @@ -69,6 +69,7 @@ main(int argc, char *argv[]) message_program_name = "wallet"; /* Initialize default configuration. */ + memset(&options, 0, sizeof(options)); retval = krb5_init_context(&ctx); if (retval != 0) die_krb5(ctx, retval, "cannot initialize Kerberos"); diff --git a/client/wallet.c b/client/wallet.c index ed6c9b8..5a80876 100644 --- a/client/wallet.c +++ b/client/wallet.c @@ -76,6 +76,7 @@ main(int argc, char *argv[]) message_program_name = "wallet"; /* Initialize default configuration. */ + memset(&options, 0, sizeof(options)); retval = krb5_init_context(&ctx); if (retval != 0) die_krb5(ctx, retval, "cannot initialize Kerberos"); diff --git a/configure.ac b/configure.ac index 12b93f8..41ae0cf 100644 --- a/configure.ac +++ b/configure.ac @@ -57,7 +57,9 @@ dnl Probe for required libraries. RRA_LIB_REMCTL RRA_LIB_KRB5 RRA_LIB_KRB5_SWITCH -AC_CHECK_FUNCS([krb5_get_init_creds_opt_alloc \ +AC_CHECK_TYPES([krb5_realm], [], [], [RRA_INCLUDES_KRB5]) +AC_CHECK_FUNCS([krb5_free_default_realm \ + krb5_get_init_creds_opt_alloc \ krb5_get_init_creds_opt_set_default_flags \ krb5_principal_get_realm]) AC_CHECK_FUNCS([krb5_get_init_creds_opt_free], diff --git a/portable/krb5.h b/portable/krb5.h index 63a2e9f..d8884a7 100644 --- a/portable/krb5.h +++ b/portable/krb5.h @@ -70,6 +70,15 @@ void krb5_appdefault_string(krb5_context, const char *, const krb5_data *, const char *, const char *, char **); #endif +/* + * MIT-specific. The Heimdal documentation says to use free(), but that + * doesn't actually make sense since the memory is allocated inside the + * Kerberos library. Use krb5_xfree instead. + */ +#ifndef HAVE_KRB5_FREE_DEFAULT_REALM +# define krb5_free_default_realm(c, r) krb5_xfree(r) +#endif + /* * krb5_{get,free}_error_message are the preferred APIs for both current MIT * and current Heimdal, but there are tons of older APIs we may have to fall -- cgit v1.2.3