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(-) 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