Fix krb5_cc_move() issues

Move init/copy/destroy fallback sequence from fcc_move() to
krb5_cc_move().

Make sure all backends's move() method calls krb5_cc_destroy() on the
source on success (and only on success).

In text_cc make sure that we can find in the destination the cred
stored into the source.
This commit is contained in:
Nicolas Williams
2019-09-09 19:19:53 -05:00
parent cf16e60f3b
commit ec84667763
8 changed files with 73 additions and 107 deletions

View File

@@ -992,8 +992,7 @@ acc_move(krb5_context context, krb5_ccache from, krb5_ccache to)
error = (*ato->ccache->func->move)(afrom->ccache, ato->ccache);
acc_destroy(context, from);
krb5_cc_destroy(context, from);
return translate_cc_error(context, error);
}

View File

@@ -1295,8 +1295,9 @@ krb5_cc_cache_match (krb5_context context,
* @param from the credential cache to move the content from
* @param to the credential cache to move the content to
* @return On sucess, from is freed. On failure, error code is
* returned and from and to are both still allocated, see krb5_get_error_message().
* @return On sucess, from is destroyed and closed. On failure, error code is
* returned and from and to are both still allocated; see
* krb5_get_error_message().
*
* @ingroup krb5_ccache
*/
@@ -1304,20 +1305,38 @@ krb5_cc_cache_match (krb5_context context,
KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
krb5_cc_move(krb5_context context, krb5_ccache from, krb5_ccache to)
{
krb5_error_code ret;
krb5_error_code ret = ENOTSUP;
krb5_principal princ = NULL;
if (strcmp(from->ops->prefix, to->ops->prefix) != 0) {
krb5_set_error_message(context, KRB5_CC_NOSUPP,
N_("Moving credentials between diffrent "
"types not yet supported", ""));
return KRB5_CC_NOSUPP;
}
if (to->ops->move &&
strcmp(from->ops->prefix, to->ops->prefix) == 0) {
/*
* NOTE: to->ops->move() is expected to call
* krb5_cc_destroy(context, from) on success.
*/
ret = (*to->ops->move)(context, from, to);
if (ret == 0)
return 0;
if (ret != EXDEV && ret != ENOTSUP)
return ret;
/* Fallback to high-level copy */
} /* Else high-level copy */
ret = (*to->ops->move)(context, from, to);
if (ret == 0) {
memset(from, 0, sizeof(*from));
free(from);
}
/*
* Initialize destination, copy the source's contents to the destination,
* then destroy the source on success.
*
* It'd be nice if we could destroy any half-built destination if the copy
* fails, but the interface is not documented as doing so.
*/
ret = krb5_cc_get_principal(context, from, &princ);
if (ret == 0)
ret = krb5_cc_initialize(context, to, princ);
krb5_free_principal(context, princ);
if (ret == 0)
ret = krb5_cc_copy_cache(context, from, to);
if (ret == 0)
krb5_cc_destroy(context, from);
return ret;
}

View File

@@ -1160,78 +1160,14 @@ fcc_move(krb5_context context, krb5_ccache from, krb5_ccache to)
{
krb5_error_code ret = 0;
ret = rk_rename(FILENAME(from), FILENAME(to));
if (ret && errno != EXDEV) {
char buf[128];
ret = errno;
rk_strerror_r(ret, buf, sizeof(buf));
krb5_set_error_message(context, ret,
N_("Rename of file from %s "
"to %s failed: %s", ""),
FILENAME(from), FILENAME(to), buf);
return ret;
} else if (ret && errno == EXDEV) {
/* make a copy and delete the orignal */
krb5_ssize_t sz1, sz2;
int fd1, fd2;
char buf[BUFSIZ];
ret = fcc_open(context, from, "move/from", &fd1, O_RDONLY, 0);
if(ret)
return ret;
unlink(FILENAME(to));
ret = fcc_open(context, to, "move/to", &fd2,
O_WRONLY | O_CREAT | O_EXCL, 0600);
if(ret)
goto out1;
while((sz1 = read(fd1, buf, sizeof(buf))) > 0) {
sz2 = write(fd2, buf, sz1);
if (sz1 != sz2) {
ret = EIO;
krb5_set_error_message(context, ret,
N_("Failed to write data from one file "
"credential cache to the other", ""));
goto out2;
}
}
if (sz1 < 0) {
ret = EIO;
krb5_set_error_message(context, ret,
N_("Failed to read data from one file "
"credential cache to the other", ""));
goto out2;
}
out2:
close(fd2);
out1:
close(fd1);
_krb5_erase_file(context, FILENAME(from));
if (ret) {
_krb5_erase_file(context, FILENAME(to));
return ret;
}
}
/* make sure ->version is uptodate */
{
krb5_storage *sp;
int fd;
if ((ret = init_fcc (context, to, "move", &sp, &fd, NULL)) == 0) {
if (sp)
krb5_storage_free(sp);
close(fd);
}
}
fcc_close(context, from);
if ((ret = rk_rename(FILENAME(from), FILENAME(to))))
ret = errno;
/*
* We need only close from -- we can't destroy it since the rename
* succeeded, which "destroyed" it at its old name.
*/
if (ret == 0)
krb5_cc_close(context, from);
return ret;
}

View File

@@ -944,6 +944,9 @@ kcm_move(krb5_context context, krb5_ccache from, krb5_ccache to)
ret = krb5_kcm_call(context, request, NULL, NULL);
krb5_storage_free(request);
if (ret == 0)
krb5_cc_destroy(context, from);
return ret;
}

View File

@@ -1977,10 +1977,7 @@ krcc_move(krb5_context context, krb5_ccache from, krb5_ccache to)
}
update_change_time(context, now, krto);
krcc_destroy(context, from);
krcc_close(context, from);
krb5_cc_destroy(context, from);
return 0;
}

View File

@@ -519,8 +519,8 @@ mcc_move(krb5_context context, krb5_ccache from, krb5_ccache to)
HEIMDAL_MUTEX_unlock(&(mfrom->mutex));
HEIMDAL_MUTEX_unlock(&(mto->mutex));
HEIMDAL_MUTEX_unlock(&mcc_mutex);
mcc_destroy(context, from);
krb5_cc_destroy(context, from);
return 0;
}

View File

@@ -1276,11 +1276,8 @@ scc_move(krb5_context context, krb5_ccache from, krb5_ccache to)
krb5_error_code ret;
if (strcmp(sfrom->file, sto->file) != 0) {
krb5_set_error_message(context, KRB5_CC_BADNAME,
N_("Can't handle cross database "
"credential move: %s -> %s", ""),
sfrom->file, sto->file);
return KRB5_CC_BADNAME;
/* Let upstairs handle the move */
return EXDEV;
}
ret = make_database(context, sfrom);
@@ -1326,14 +1323,11 @@ scc_move(krb5_context context, krb5_ccache from, krb5_ccache to)
ret = exec_stmt(context, sfrom->db, "COMMIT", KRB5_CC_IO);
if (ret) return ret;
scc_free(sfrom);
krb5_cc_close(context, from);
return 0;
rollback:
exec_stmt(context, sfrom->db, "ROLLBACK", 0);
scc_free(sfrom);
return KRB5_CC_IO;
}

View File

@@ -249,6 +249,8 @@ test_cache_remove(krb5_context context, const char *type)
if (ret)
krb5_err(context, 1, ret, "krb5_cc_remove_cred");
/* XXX Search for it */
ret = krb5_cc_destroy(context, id);
if (ret)
krb5_err(context, 1, ret, "krb5_cc_destroy");
@@ -506,6 +508,7 @@ test_move(krb5_context context, const char *type)
krb5_ccache fromid, toid;
krb5_error_code ret;
krb5_principal p, p2;
krb5_creds cred, tocred;
ops = krb5_cc_get_prefix_ops(context, type);
if (ops == NULL)
@@ -525,13 +528,24 @@ test_move(krb5_context context, const char *type)
if (ret)
krb5_err(context, 1, ret, "krb5_cc_initialize");
memset(&cred, 0, sizeof(cred));
ret = krb5_parse_name(context, "krbtgt/SU.SE@SU.SE", &cred.server);
if (ret)
krb5_err(context, 1, ret, "krb5_parse_name");
ret = krb5_parse_name(context, "lha@SU.SE", &cred.client);
if (ret)
krb5_err(context, 1, ret, "krb5_parse_name");
ret = krb5_cc_store_cred(context, fromid, &cred);
if (ret)
krb5_err(context, 1, ret, "krb5_cc_store_cred");
ret = krb5_cc_new_unique(context, type, NULL, &toid);
if (ret)
krb5_err(context, 1, ret, "krb5_cc_new_unique");
ret = krb5_cc_initialize(context, toid, p);
if (ret)
krb5_err(context, 1, ret, "krb5_cc_initialize");
ret = krb5_cc_move(context, fromid, toid);
ret = krb5_cc_get_principal(context, toid, &p2);
if (ret)
@@ -540,11 +554,15 @@ test_move(krb5_context context, const char *type)
if (krb5_principal_compare(context, p, p2) == FALSE)
krb5_errx(context, 1, "p != p2");
ret = krb5_cc_retrieve_cred(context, toid, 0, &cred, &tocred);
if (ret)
krb5_errx(context, 1, "move failed");
krb5_free_cred_contents(context, &cred);
krb5_free_cred_contents(context, &tocred);
krb5_free_principal(context, p);
krb5_free_principal(context, p2);
krb5_cc_destroy(context, toid);
krb5_cc_destroy(context, fromid);
}