Commit 17620150 authored by Russ Allbery's avatar Russ Allbery

Update code to take advantage of portability guarantees

Add portability code for platforms without a working snprintf or other
deficiencies and updated the code to take advantage of those
guarantees.

Add extra blank lines between functions.
parent 20263994
...@@ -20,6 +20,10 @@ krb5-sync 2.0 (unreleased) ...@@ -20,6 +20,10 @@ krb5-sync 2.0 (unreleased)
Enable Automake silent rules. For a quieter build, pass the Enable Automake silent rules. For a quieter build, pass the
--enable-silent-rules option to configure or build with make V=0. --enable-silent-rules option to configure or build with make V=0.
Add portability code for platforms without a working snprintf or other
deficiencies and updated the code to take advantage of those
guarantees.
krb5-sync 1.2 (2007-12-25) krb5-sync 1.2 (2007-12-25)
Don't call rx_Finalize after every synchronization with an AFS Don't call rx_Finalize after every synchronization with an AFS
......
...@@ -15,6 +15,7 @@ AM_INIT_AUTOMAKE([1.11 check-news silent-rules -Wall -Werror]) ...@@ -15,6 +15,7 @@ AM_INIT_AUTOMAKE([1.11 check-news silent-rules -Wall -Werror])
AM_MAINTAINER_MODE AM_MAINTAINER_MODE
AC_PROG_CC AC_PROG_CC
AC_USE_SYSTEM_EXTENSIONS
AM_PROG_CC_C_O AM_PROG_CC_C_O
AC_PROG_INSTALL AC_PROG_INSTALL
AM_DISABLE_STATIC AM_DISABLE_STATIC
...@@ -35,7 +36,7 @@ AC_CHECK_DECLS([snprintf, vsnprintf]) ...@@ -35,7 +36,7 @@ AC_CHECK_DECLS([snprintf, vsnprintf])
AC_TYPE_LONG_LONG_INT AC_TYPE_LONG_LONG_INT
RRA_FUNC_SNPRINTF RRA_FUNC_SNPRINTF
AC_CHECK_FUNCS([setrlimit]) AC_CHECK_FUNCS([setrlimit])
AC_REPLACE_FUNCS([strlcat strlcpy]) AC_REPLACE_FUNCS([asprintf strlcat strlcpy])
# Crank up the warnings if we're using GCC. # Crank up the warnings if we're using GCC.
if test "x$GCC" = "xyes" ; then if test "x$GCC" = "xyes" ; then
......
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
/* The flag value used in Active Directory to indicate a disabled account. */ /* The flag value used in Active Directory to indicate a disabled account. */
#define UF_ACCOUNTDISABLE 0x02 #define UF_ACCOUNTDISABLE 0x02
/* /*
* Given the plugin options, a Kerberos context, a pointer to krb5_ccache * Given the plugin options, a Kerberos context, a pointer to krb5_ccache
* storage, and the buffer into which to store an error message if any, * storage, and the buffer into which to store an error message if any,
...@@ -124,6 +125,7 @@ get_creds(struct plugin_config *config, krb5_context ctx, krb5_ccache *cc, ...@@ -124,6 +125,7 @@ get_creds(struct plugin_config *config, krb5_context ctx, krb5_ccache *cc,
return 0; return 0;
} }
/* /*
* Given the krb5_principal from kadmind, convert it to the corresponding * Given the krb5_principal from kadmind, convert it to the corresponding
* principal in Active Directory. Returns 0 on success and a Kerberos error * principal in Active Directory. Returns 0 on success and a Kerberos error
...@@ -147,6 +149,7 @@ get_ad_principal(krb5_context ctx, const char *realm, ...@@ -147,6 +149,7 @@ get_ad_principal(krb5_context ctx, const char *realm,
return 0; return 0;
} }
/* /*
* Push a password change to Active Directory. Takes the module * Push a password change to Active Directory. Takes the module
* configuration, a Kerberos context, the principal whose password is being * configuration, a Kerberos context, the principal whose password is being
...@@ -214,7 +217,7 @@ pwupdate_ad_change(struct plugin_config *config, krb5_context ctx, ...@@ -214,7 +217,7 @@ pwupdate_ad_change(struct plugin_config *config, krb5_context ctx,
free(result_string.data); free(result_string.data);
free(result_code_string.data); free(result_code_string.data);
syslog(LOG_INFO, "pwupdate: %s password changed", target); syslog(LOG_INFO, "pwupdate: %s password changed", target);
snprintf(errstr, errstrlen, "Password changed"); strlcpy(errstr, "Password changed", errstrlen);
done: done:
if (target != NULL) if (target != NULL)
...@@ -227,6 +230,7 @@ done: ...@@ -227,6 +230,7 @@ done:
return code; return code;
} }
/* /*
* Empty SASL callback function to satisfy the requirements of the LDAP SASL * Empty SASL callback function to satisfy the requirements of the LDAP SASL
* bind interface. Hopefully it won't need anything. * bind interface. Hopefully it won't need anything.
...@@ -238,6 +242,7 @@ ad_interact_sasl(LDAP *ld UNUSED, unsigned flags UNUSED, ...@@ -238,6 +242,7 @@ ad_interact_sasl(LDAP *ld UNUSED, unsigned flags UNUSED,
return 0; return 0;
} }
/* /*
* Change the status of an account in Active Directory. Takes the plugin * Change the status of an account in Active Directory. Takes the plugin
* configuration, a Kerberos context, the principal whose status changed (only * configuration, a Kerberos context, the principal whose status changed (only
...@@ -245,16 +250,17 @@ ad_interact_sasl(LDAP *ld UNUSED, unsigned flags UNUSED, ...@@ -245,16 +250,17 @@ ad_interact_sasl(LDAP *ld UNUSED, unsigned flags UNUSED,
* account is enabled, and a buffer into which to put error messages and its * account is enabled, and a buffer into which to put error messages and its
* length. * length.
*/ */
int pwupdate_ad_status(struct plugin_config *config, krb5_context ctx, int
krb5_principal principal, int enabled, char *errstr, pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
int errstrlen) krb5_principal principal, int enabled, char *errstr,
int errstrlen)
{ {
krb5_ccache ccache; krb5_ccache ccache;
krb5_principal ad_principal = NULL; krb5_principal ad_principal = NULL;
LDAP *ld; LDAP *ld;
LDAPMessage *res = NULL; LDAPMessage *res = NULL;
LDAPMod mod, *mod_array[2]; LDAPMod mod, *mod_array[2];
char ldapuri[256], ldapbase[256], ldapdn[256], *dname, *lb, *dn; char ldapuri[256], ldapbase[256], ldapdn[256], *dname, *lb, *end, *dn;
char *target = NULL; char *target = NULL;
char **vals = NULL; char **vals = NULL;
const char *attrs[] = { "userAccountControl", NULL }; const char *attrs[] = { "userAccountControl", NULL };
...@@ -310,21 +316,21 @@ int pwupdate_ad_status(struct plugin_config *config, krb5_context ctx, ...@@ -310,21 +316,21 @@ int pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
*/ */
memset(ldapbase, 0, sizeof(ldapbase)); memset(ldapbase, 0, sizeof(ldapbase));
if (config->ad_ldap_base == NULL) if (config->ad_ldap_base == NULL)
strcpy(ldapbase, "ou=Accounts,dc="); strlcpy(ldapbase, "ou=Accounts,dc=", sizeof(ldapbase));
else { else {
strncpy(ldapbase, config->ad_ldap_base, sizeof(ldapbase) - 5); strlcpy(ldapbase, config->ad_ldap_base, sizeof(ldapbase));
ldapbase[sizeof(ldapbase) - 5] = '\0'; strlcat(ldapbase, ",dc=", sizeof(ldapbase));
strcat(ldapbase, ",dc=");
} }
lb = ldapbase + strlen(ldapbase); lb = ldapbase + strlen(ldapbase);
for (dname = config->ad_realm; *dname != '\0'; dname++) { end = ldapbase + sizeof(ldapbase) - 1;
for (dname = config->ad_realm; lb < end && *dname != '\0'; dname++) {
if (*dname == '.') { if (*dname == '.') {
strcpy(lb, ",dc="); *lb = '\0';
strlcat(ldapbase, ",dc=", sizeof(ldapbase));
lb += 4; lb += 4;
} else } else {
*lb++ = *dname; *lb++ = *dname;
if (strlen(ldapbase) > sizeof(ldapbase) - 5) }
break;
} }
/* /*
...@@ -410,7 +416,7 @@ int pwupdate_ad_status(struct plugin_config *config, krb5_context ctx, ...@@ -410,7 +416,7 @@ int pwupdate_ad_status(struct plugin_config *config, krb5_context ctx,
done: done:
if (target != NULL) if (target != NULL)
free(target); krb5_free_unparsed_name(ctx, target);
if (res != NULL) if (res != NULL)
ldap_msgfree(res); ldap_msgfree(res);
if (vals != NULL) if (vals != NULL)
......
...@@ -36,6 +36,7 @@ ...@@ -36,6 +36,7 @@
# define krb5_principal_get_num_comp(c, p) krb5_princ_size((c), (p)) # define krb5_principal_get_num_comp(c, p) krb5_princ_size((c), (p))
#endif #endif
/* /*
* Load a string option from Kerberos appdefaults, setting the default to NULL * Load a string option from Kerberos appdefaults, setting the default to NULL
* if the setting was not found. This requires an annoying workaround because * if the setting was not found. This requires an annoying workaround because
...@@ -53,6 +54,7 @@ config_string(krb5_context ctx, const char *opt, char **result) ...@@ -53,6 +54,7 @@ config_string(krb5_context ctx, const char *opt, char **result)
} }
} }
/* /*
* Initialize the module. This consists solely of loading our configuration * Initialize the module. This consists solely of loading our configuration
* options from krb5.conf into a newly allocated struct stored in the second * options from krb5.conf into a newly allocated struct stored in the second
...@@ -78,6 +80,7 @@ pwupdate_init(krb5_context ctx, void **data) ...@@ -78,6 +80,7 @@ pwupdate_init(krb5_context ctx, void **data)
return 0; return 0;
} }
/* /*
* Shut down the module. This just means freeing our configuration struct, * Shut down the module. This just means freeing our configuration struct,
* since we don't store any other local state. * since we don't store any other local state.
...@@ -100,6 +103,7 @@ pwupdate_close(void *data) ...@@ -100,6 +103,7 @@ pwupdate_close(void *data)
free(config); free(config);
} }
/* /*
* Create a local Kerberos context and set the error appropriately if this * Create a local Kerberos context and set the error appropriately if this
* fails. Return true on success, false otherwise. Puts the error message in * fails. Return true on success, false otherwise. Puts the error message in
...@@ -119,6 +123,7 @@ create_context(krb5_context *ctx, char *errstr, int errstrlen) ...@@ -119,6 +123,7 @@ create_context(krb5_context *ctx, char *errstr, int errstrlen)
return 1; return 1;
} }
/* /*
* Given the list of allowed principals as a space-delimited string and the * Given the list of allowed principals as a space-delimited string and the
* instance of a principal, returns true if that instance is allowed and false * instance of a principal, returns true if that instance is allowed and false
...@@ -158,6 +163,7 @@ instance_allowed(const char *allowed, const char *instance, size_t length) ...@@ -158,6 +163,7 @@ instance_allowed(const char *allowed, const char *instance, size_t length)
return 0; return 0;
} }
/* /*
* Check the principal for which we're changing a password. If it contains a * Check the principal for which we're changing a password. If it contains a
* non-null instance, we don't want to propagate the change; we only want to * non-null instance, we don't want to propagate the change; we only want to
...@@ -196,12 +202,13 @@ principal_allowed(struct plugin_config *config, krb5_context ctx, ...@@ -196,12 +202,13 @@ principal_allowed(struct plugin_config *config, krb5_context ctx,
display != NULL ? display : "???", display != NULL ? display : "???",
ad ? "Active Directory" : "AFS"); ad ? "Active Directory" : "AFS");
if (display != NULL) if (display != NULL)
free(display); krb5_free_unparsed_name(ctx, display);
return 0; return 0;
} }
return 1; return 1;
} }
/* /*
* Actions to take before the password is changed in the local database. * Actions to take before the password is changed in the local database.
* *
...@@ -248,11 +255,12 @@ queue: ...@@ -248,11 +255,12 @@ queue:
if (status) if (status)
return 0; return 0;
else { else {
snprintf(errstr, errstrlen, "queueing AD password change failed"); strlcpy(errstr, "queueing AD password change failed", errstrlen);
return 1; return 1;
} }
} }
/* /*
* Actions to take after the password is changed in the local database. * Actions to take after the password is changed in the local database.
* Currently, there are none. * Currently, there are none.
...@@ -309,7 +317,7 @@ queue: ...@@ -309,7 +317,7 @@ queue:
if (status) if (status)
return 0; return 0;
else { else {
snprintf(errstr, errstrlen, "queueing AD status change failed"); strlcpy(errstr, "queueing AD status change failed", errstrlen);
return 1; return 1;
} }
} }
...@@ -41,6 +41,7 @@ ...@@ -41,6 +41,7 @@
goto fail; \ goto fail; \
} while (0) } while (0)
/* /*
* Lock the queue directory. Returns a file handle to the lock file, which * Lock the queue directory. Returns a file handle to the lock file, which
* must then be passed into unlock_queue when the queue should be unlocked, or * must then be passed into unlock_queue when the queue should be unlocked, or
...@@ -53,14 +54,10 @@ static int ...@@ -53,14 +54,10 @@ static int
lock_queue(struct plugin_config *config) lock_queue(struct plugin_config *config)
{ {
char *lockpath = NULL; char *lockpath = NULL;
size_t length;
int fd = -1; int fd = -1;
length = strlen(config->queue_dir) + 1 + strlen(".lock") + 1; if (asprintf(&lockpath, "%s/.lock", config->queue_dir) < 0)
lockpath = malloc(length);
if (lockpath == NULL)
return -1; return -1;
snprintf(lockpath, length, "%s/.lock", config->queue_dir);
fd = open(lockpath, O_RDWR | O_CREAT, 0644); fd = open(lockpath, O_RDWR | O_CREAT, 0644);
if (fd < 0) if (fd < 0)
goto fail; goto fail;
...@@ -102,7 +99,6 @@ queue_prefix(krb5_context ctx, krb5_principal principal, const char *domain, ...@@ -102,7 +99,6 @@ queue_prefix(krb5_context ctx, krb5_principal principal, const char *domain,
{ {
char *user = NULL, *prefix = NULL; char *user = NULL, *prefix = NULL;
char *p; char *p;
size_t length;
krb5_error_code retval; krb5_error_code retval;
/* Enable and disable should go into the same queue. */ /* Enable and disable should go into the same queue. */
...@@ -116,20 +112,10 @@ queue_prefix(krb5_context ctx, krb5_principal principal, const char *domain, ...@@ -116,20 +112,10 @@ queue_prefix(krb5_context ctx, krb5_principal principal, const char *domain,
*p = '\0'; *p = '\0';
while ((p = strchr(user, '/')) != NULL) while ((p = strchr(user, '/')) != NULL)
*p = '.'; *p = '.';
length = strlen(user) + strlen(domain) + strlen(operation) + 4; if (asprintf(&prefix, "%s-%s-%s-", user, domain, operation) < 0) {
prefix = malloc(length);
if (prefix == NULL) {
#ifdef HAVE_KRB5_XFREE
krb5_xfree(user);
#else
krb5_free_unparsed_name(ctx, user); krb5_free_unparsed_name(ctx, user);
#endif
return NULL; return NULL;
} }
snprintf(prefix, length, "%s-%s-%s-", user, domain, operation);
#ifdef HAVE_KRB5_XFREE
krb5_xfree(user);
#else
krb5_free_unparsed_name(ctx, user); krb5_free_unparsed_name(ctx, user);
#endif #endif
return prefix; return prefix;
...@@ -227,8 +213,8 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx, ...@@ -227,8 +213,8 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx,
{ {
char *prefix = NULL, *timestamp = NULL, *path = NULL, *user = NULL; char *prefix = NULL, *timestamp = NULL, *path = NULL, *user = NULL;
char *p; char *p;
size_t length;
unsigned int i; unsigned int i;
int status;
int lock = -1, fd = -1; int lock = -1, fd = -1;
krb5_error_code retval; krb5_error_code retval;
...@@ -248,14 +234,15 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx, ...@@ -248,14 +234,15 @@ pwupdate_queue_write(struct plugin_config *config, krb5_context ctx,
goto fail; goto fail;
/* Find a unique filename for the queue file. */ /* Find a unique filename for the queue file. */
length = strlen(config->queue_dir) + 1 + strlen(prefix)
+ strlen(timestamp) + 1 + strlen(MAX_QUEUE_STR) + 1;
path = malloc(length);
if (path == NULL)
goto fail;
for (i = 0; i < MAX_QUEUE; i++) { for (i = 0; i < MAX_QUEUE; i++) {
snprintf(path, length, "%s/%s%s-%02d", config->queue_dir, prefix, if (path != NULL) {
timestamp, i); free(path);
path = NULL;
}
status = asprintf(&path, "%s/%s%s-%02d", config->queue_dir, prefix,
timestamp, i);
if (status < 0)
goto fail;
fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0600); fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0600);
if (fd >= 0) if (fd >= 0)
break; break;
......
/*
* Replacement for a missing asprintf and vasprintf.
*
* Provides the same functionality as the standard GNU library routines
* asprintf and vasprintf for those platforms that don't have them.
*
* Written by Russ Allbery <rra@stanford.edu>
* This work is hereby placed in the public domain by its author.
*/
#include <config.h>
#include <portable/system.h>
/*
* If we're running the test suite, rename the functions to avoid conflicts
* with the system versions.
*/
#if TESTING
# define asprintf test_asprintf
# define vasprintf test_vasprintf
int test_asprintf(char **, const char *, ...)
__attribute__((__format__(printf, 2, 3)));
int test_vasprintf(char **, const char *, va_list);
#endif
int
asprintf(char **strp, const char *fmt, ...)
{
va_list args;
int status;
va_start(args, fmt);
status = vasprintf(strp, fmt, args);
va_end(args);
return status;
}
int
vasprintf(char **strp, const char *fmt, va_list args)
{
va_list args_copy;
int status, needed;
va_copy(args_copy, args);
needed = vsnprintf(NULL, 0, fmt, args_copy);
va_end(args_copy);
if (needed < 0) {
*strp = NULL;
return needed;
}
*strp = malloc(needed + 1);
if (*strp == NULL)
return -1;
status = vsnprintf(*strp, needed + 1, fmt, args);
if (status >= 0)
return status;
else {
free(*strp);
*strp = NULL;
return status;
}
}
...@@ -66,6 +66,11 @@ BEGIN_DECLS ...@@ -66,6 +66,11 @@ BEGIN_DECLS
* HAVE_DECL macros for those functions that may be prototyped but implemented * HAVE_DECL macros for those functions that may be prototyped but implemented
* incorrectly or implemented without a prototype. * incorrectly or implemented without a prototype.
*/ */
#if !HAVE_ASPRINTF
extern int asprintf(char **, const char *, ...)
__attribute__((__format__(printf, 2, 3)));
extern int vasprintf(char **, const char *, va_list);
#endif
#if !HAVE_DECL_SNPRINTF #if !HAVE_DECL_SNPRINTF
extern int snprintf(char *, size_t, const char *, ...) extern int snprintf(char *, size_t, const char *, ...)
__attribute__((__format__(printf, 3, 4))); __attribute__((__format__(printf, 3, 4)));
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include <plugin/internal.h> #include <plugin/internal.h>
/* /*
* Change a password in Active Directory. Print a success message if we were * Change a password in Active Directory. Print a success message if we were
* successful, and exit with an error message if we weren't. * successful, and exit with an error message if we weren't.
...@@ -46,6 +47,7 @@ ad_password(void *data, krb5_context ctx, krb5_principal principal, ...@@ -46,6 +47,7 @@ ad_password(void *data, krb5_context ctx, krb5_principal principal,
printf("AD password change for %s succeeded\n", user); printf("AD password change for %s succeeded\n", user);
} }
/* /*
* Change the account status in Active Directory. Print a success message if * Change the account status in Active Directory. Print a success message if
* we were successful, and exit with an error message if we weren't. * we were successful, and exit with an error message if we weren't.
...@@ -67,6 +69,7 @@ ad_status(void *data, krb5_context ctx, krb5_principal principal, int enable, ...@@ -67,6 +69,7 @@ ad_status(void *data, krb5_context ctx, krb5_principal principal, int enable,
printf("AD status change for %s succeeded\n", user); printf("AD status change for %s succeeded\n", user);
} }
/* /*
* Read a line from a queue file, making sure we got a complete line and * Read a line from a queue file, making sure we got a complete line and
* cutting off the trailing newline. Doesn't return on error. * cutting off the trailing newline. Doesn't return on error.
...@@ -86,6 +89,7 @@ read_line(FILE *file, const char *filename, char *buffer, size_t bufsiz) ...@@ -86,6 +89,7 @@ read_line(FILE *file, const char *filename, char *buffer, size_t bufsiz)
buffer[strlen(buffer) - 1] = '\0'; buffer[strlen(buffer) - 1] = '\0';
} }
/* /*
* Read a queue file and take appropriate action based on its contents. The * Read a queue file and take appropriate action based on its contents. The
* format is: * format is:
...@@ -170,6 +174,7 @@ process_queue_file(void *data, krb5_context ctx, const char *filename) ...@@ -170,6 +174,7 @@ process_queue_file(void *data, krb5_context ctx, const char *filename)
free(user); free(user);
} }
int int
main(int argc, char *argv[]) main(int argc, char *argv[])
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment