diff options
| -rw-r--r-- | src/lib/kdb/kdb_default.c | 37 | ||||
| -rw-r--r-- | src/slave/kprop.c | 16 | ||||
| -rw-r--r-- | src/tests/t_mkey.py | 9 |
3 files changed, 39 insertions, 23 deletions
diff --git a/src/lib/kdb/kdb_default.c b/src/lib/kdb/kdb_default.c index 82fa5c4a0..b7a2f2427 100644 --- a/src/lib/kdb/kdb_default.c +++ b/src/lib/kdb/kdb_default.c @@ -156,10 +156,6 @@ krb5_def_store_mkey_list(krb5_context context, keyfile = defkeyfile; } - /* - * XXX making the assumption that the keyfile is in a dir that requires root - * privilege to write to thus making timing attacks unlikely. - */ if ((statrc = stat(keyfile, &stb)) >= 0) { /* if keyfile exists it better be a regular file */ if (!S_ISREG(stb.st_mode)) { @@ -171,27 +167,40 @@ krb5_def_store_mkey_list(krb5_context context, } } - /* Use temp keytab file name in case creation of keytab fails */ - - /* create temp file template for use by mktemp() */ - if ((retval = asprintf(&tmp_ktname, "WRFILE:%s_XXXXXX", keyfile)) < 0) { + /* + * We assume the stash file is in a directory writable only by root. + * As such, don't worry about collisions, just do an atomic rename. + */ + retval = asprintf(&tmp_ktname, "FILE:%s_tmp", keyfile); + if (retval < 0) { krb5_set_error_message(context, retval, _("Could not create temp keytab file name.")); goto out; } /* - * Set tmp_ktpath to point to the keyfile path (skip WRFILE:). Subtracting + * Set tmp_ktpath to point to the keyfile path (skip FILE:). Subtracting * 1 to account for NULL terminator in sizeof calculation of a string * constant. Used further down. */ - tmp_ktpath = tmp_ktname + (sizeof("WRFILE:") - 1); + tmp_ktpath = tmp_ktname + (sizeof("FILE:") - 1); - if (mktemp(tmp_ktpath) == NULL) { + /* + * This time-of-check-to-time-of-access race is fine; we care only + * about an administrator running the command twice, not an attacker + * trying to beat us to creating the file. Per the above comment, we + * assume the stash file is in a directory writable only by root. + */ + statrc = stat(tmp_ktpath, &stb); + if (statrc == -1 && errno != ENOENT) { + /* ENOENT is the expected case */ retval = errno; + goto out; + } else if (statrc == 0) { + retval = EEXIST; krb5_set_error_message(context, retval, - _("Could not create temp stash file: %s"), - error_message(errno)); + _("Temporary stash file already exists: %s."), + tmp_ktpath); goto out; } @@ -215,7 +224,7 @@ krb5_def_store_mkey_list(krb5_context context, /* Clean up by deleting the tmp keyfile if it exists. */ (void)unlink(tmp_ktpath); } else { - /* rename original keyfile to original filename */ + /* Atomically rename temp keyfile to original filename. */ if (rename(tmp_ktpath, keyfile) < 0) { retval = errno; krb5_set_error_message(context, retval, diff --git a/src/slave/kprop.c b/src/slave/kprop.c index acdca0a5a..b668147dc 100644 --- a/src/slave/kprop.c +++ b/src/slave/kprop.c @@ -187,9 +187,9 @@ void PRS(argc, argv) void get_tickets(context) krb5_context context; { - char buf[BUFSIZ], *def_realm; + char const ccname[] = "MEMORY:kpropcc"; + char *def_realm; krb5_error_code retval; - static char tkstring[] = "/tmp/kproptktXXXXXX"; krb5_keytab keytab = NULL; /* @@ -230,20 +230,18 @@ void get_tickets(context) #endif /* - * Initialize cache file which we're going to be using + * Use a memory cache to avoid possible filesystem conflicts. */ - (void) mktemp(tkstring); - snprintf(buf, sizeof(buf), "FILE:%s", tkstring); - - retval = krb5_cc_resolve(context, buf, &ccache); + retval = krb5_cc_resolve(context, ccname, &ccache); if (retval) { - com_err(progname, retval, _("while opening credential cache %s"), buf); + com_err(progname, retval, _("while opening credential cache %s"), + ccname); exit(1); } retval = krb5_cc_initialize(context, ccache, my_principal); if (retval) { - com_err(progname, retval, _("when initializing cache %s"), buf); + com_err(progname, retval, _("when initializing cache %s"), ccname); exit(1); } diff --git a/src/tests/t_mkey.py b/src/tests/t_mkey.py index 35d14f71c..3cecabffa 100644 --- a/src/tests/t_mkey.py +++ b/src/tests/t_mkey.py @@ -155,6 +155,15 @@ check_master_dbent(1, (1, defetype)) check_stash((1, defetype)) check_mkvno(realm.user_princ, 1) +# Check that stash will fail if a temp stash file is already present. +collisionfile = os.path.join(realm.testdir, 'stash_tmp') +f = open(collisionfile, 'w') +f.close() +output = realm.run([kdb5_util, 'stash'], expected_code=1) +if 'Temporary stash file already exists' not in output: + fail('Did not detect temp stash file collision') +os.unlink(collisionfile) + # Add a new master key with no options. Verify that: # 1. The new key appears in list_mkeys but has no activation time and # is not active. |