2023-10-04 17:32:52

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH v2 0/3] nfs-utils: gssd support for KRB5_AP_ERR_BAD_INTEGRITY

From: Olga Kornievskaia <[email protected]>

Together with libtirpc patch this series attempts to provide
support for handling KRB5_AP_ERR_BAD_INTEGRITY.

Such error can be returned by the server when it has changed
its key material and the client is still using the service
ticket that was issues prior to the change.

Upon calling authgss_create_default() and receiving a NULL
context, we can inspect the returned structure to see
if gss major/minor error code was set. If the client
determines that it received KRB5_AP_ERR_BAD_INTEGRITY error,
it will proceed to handle it based on what type of credentials
were used for context establishement. If machine credentials
were used, the client can call into a routine and force
credential renewal. If user credentials were used, the client
needs to remove the existing service ticket and then retry
the request.

-- fix compile warning in libtirpc patch

Olga Kornievskaia (3):
gssd: enable forcing cred renewal using the keytab
gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for machine credentials
gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for user credentials

utils/gssd/gssd_proc.c | 20 ++++++++++++--
utils/gssd/krb5_util.c | 62 ++++++++++++++++++++++++++++++++++++------
utils/gssd/krb5_util.h | 4 ++-
3 files changed, 75 insertions(+), 11 deletions(-)

--
2.39.1


2023-10-04 17:32:57

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 1/1] gssd: fix handling DNS lookup failure

From: Olga Kornievskaia <[email protected]>

When the kernel does its first ever lookup for a given server ip it
sends down info for server, protocol, etc. On the gssd side as it
scans the pipefs structure and sees a new entry it reads that info
and creates a clp_info structure. At that time it also does
a DNS lookup of the provided ip to name using getnameinfo(),
this is saved in clp->servername for all other upcalls that is
down under that directory.

IF this 1st getnameinfo() results in a failed resolution for
whatever reason (a temporary DNS resolution problem), this cause
of all other future upcalls to fail.

As a fix, this patch proposed to (1) save the server info that's
passed only in the initial pipefs new entry creation in the
clp_info structure, then (2) for the upcalls, if clp->servername
is NULL, then do the DNS lookup again and set all the needed
clp_info fields upon successful resolution.

Signed-off-by: Olga Kornievskaia <[email protected]>
---
utils/gssd/gssd.c | 41 +++++++++++++++++++++++++++++++++++++++++
utils/gssd/gssd.h | 6 ++++++
2 files changed, 47 insertions(+)

diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 833d8e01..ca9b3267 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -365,6 +365,12 @@ gssd_read_service_info(int dirfd, struct clnt_info *clp)

fail:
printerr(0, "ERROR: failed to parse %s/info\n", clp->relpath);
+ clp->upcall_address = strdup(address);
+ clp->upcall_port = strdup(port);
+ clp->upcall_program = program;
+ clp->upcall_vers = version;
+ clp->upcall_protoname = strdup(protoname);
+ clp->upcall_service = strdup(service);
free(servername);
free(protoname);
clp->servicename = NULL;
@@ -408,6 +414,16 @@ gssd_free_client(struct clnt_info *clp)
free(clp->servicename);
free(clp->servername);
free(clp->protocol);
+ if (!clp->servername) {
+ if (clp->upcall_address)
+ free(clp->upcall_address);
+ if (clp->upcall_port)
+ free(clp->upcall_port);
+ if (clp->upcall_protoname)
+ free(clp->upcall_protoname);
+ if (clp->upcall_service)
+ free(clp->upcall_service);
+ }
free(clp);
}

@@ -446,6 +462,31 @@ gssd_clnt_gssd_cb(int UNUSED(fd), short UNUSED(which), void *data)
{
struct clnt_info *clp = data;

+ /* if there was a failure to translate IP to name for this server,
+ * try again
+ */
+ if (!clp->servername) {
+ if (!gssd_addrstr_to_sockaddr((struct sockaddr *)&clp->addr,
+ clp->upcall_address, clp->upcall_port ?
+ clp->upcall_port : "")) {
+ goto do_upcall;
+ }
+ clp->servername = gssd_get_servername(clp->upcall_address,
+ (struct sockaddr *)&clp->addr, clp->upcall_address);
+ if (!clp->servername)
+ goto do_upcall;
+
+ if (asprintf(&clp->servicename, "%s@%s", clp->upcall_service,
+ clp->servername) < 0) {
+ free(clp->servername);
+ clp->servername = NULL;
+ goto do_upcall;
+ }
+ clp->prog = clp->upcall_program;
+ clp->vers = clp->upcall_vers;
+ clp->protocol = strdup(clp->upcall_protoname);
+ }
+do_upcall:
handle_gssd_upcall(clp);
}

diff --git a/utils/gssd/gssd.h b/utils/gssd/gssd.h
index 519dc431..4e070ed6 100644
--- a/utils/gssd/gssd.h
+++ b/utils/gssd/gssd.h
@@ -86,6 +86,12 @@ struct clnt_info {
int gssd_fd;
struct event *gssd_ev;
struct sockaddr_storage addr;
+ char *upcall_address;
+ char *upcall_port;
+ int upcall_program;
+ int upcall_vers;
+ char *upcall_protoname;
+ char *upcall_service;
};

struct clnt_upcall_info {
--
2.39.1

2023-10-04 17:33:00

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 1/3] nfs-utils: gssd: enable forcing cred renewal using the keytab

From: Olga Kornievskaia <[email protected]>

Add a new function parameter "force_renewal" that callers could
set to force service ticket renewal even if one exists already.

This is needed in preparation for handling
KRB5_AP_ERR_BAD_INTEGRITY when service's keytab changes while
the client holds valid service ticket in the cache.

Signed-off-by: Olga Kornievskaia <[email protected]>
---
utils/gssd/gssd_proc.c | 2 +-
utils/gssd/krb5_util.c | 20 ++++++++++++--------
utils/gssd/krb5_util.h | 3 ++-
3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index ae568f15..4fb6b72d 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -571,7 +571,7 @@ krb5_use_machine_creds(struct clnt_info *clp, uid_t uid,

do {
gssd_refresh_krb5_machine_credential(clp->servername,
- service, srchost);
+ service, srchost, 0);
/*
* Get a list of credential cache names and try each
* of them until one works or we've tried them all
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index e3f270e9..f6ce1fec 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -165,7 +165,7 @@ static int select_krb5_ccache(const struct dirent *d);
static int gssd_find_existing_krb5_ccache(uid_t uid, char *dirname,
const char **cctype, struct dirent **d);
static int gssd_get_single_krb5_cred(krb5_context context,
- krb5_keytab kt, struct gssd_k5_kt_princ *ple);
+ krb5_keytab kt, struct gssd_k5_kt_princ *ple, int force_renew);
static int query_krb5_ccache(const char* cred_cache, char **ret_princname,
char **ret_realm);

@@ -391,7 +391,8 @@ gssd_check_if_cc_exists(struct gssd_k5_kt_princ *ple)
static int
gssd_get_single_krb5_cred(krb5_context context,
krb5_keytab kt,
- struct gssd_k5_kt_princ *ple)
+ struct gssd_k5_kt_princ *ple,
+ int force_renew)
{
#ifdef HAVE_KRB5_GET_INIT_CREDS_OPT_SET_ADDRESSLESS
krb5_get_init_creds_opt *init_opts = NULL;
@@ -421,7 +422,7 @@ gssd_get_single_krb5_cred(krb5_context context,
*/
now += 300;
pthread_mutex_lock(&ple_lock);
- if (ple->ccname && ple->endtime > now && !nocache) {
+ if (ple->ccname && ple->endtime > now && !nocache && !force_renew) {
printerr(3, "%s(0x%lx): Credentials in CC '%s' are good until %s",
__func__, tid, ple->ccname, ctime((time_t *)&ple->endtime));
code = 0;
@@ -1155,7 +1156,8 @@ err_cache:
static int
gssd_refresh_krb5_machine_credential_internal(char *hostname,
struct gssd_k5_kt_princ *ple,
- char *service, char *srchost)
+ char *service, char *srchost,
+ int force_renew)
{
krb5_error_code code = 0;
krb5_context context;
@@ -1221,7 +1223,7 @@ gssd_refresh_krb5_machine_credential_internal(char *hostname,
goto out_free_kt;
}
}
- retval = gssd_get_single_krb5_cred(context, kt, ple);
+ retval = gssd_get_single_krb5_cred(context, kt, ple, force_renew);
out_free_kt:
krb5_kt_close(context, kt);
out_free_context:
@@ -1344,7 +1346,7 @@ gssd_get_krb5_machine_cred_list(char ***list)
pthread_mutex_unlock(&ple_lock);
/* Make sure cred is up-to-date before returning it */
retval = gssd_refresh_krb5_machine_credential_internal(NULL, ple,
- NULL, NULL);
+ NULL, NULL, 0);
pthread_mutex_lock(&ple_lock);
if (gssd_k5_kt_princ_list == NULL) {
/* Looks like we did shutdown... abort */
@@ -1456,10 +1458,12 @@ gssd_destroy_krb5_principals(int destroy_machine_creds)
*/
int
gssd_refresh_krb5_machine_credential(char *hostname,
- char *service, char *srchost)
+ char *service, char *srchost,
+ int force_renew)
{
return gssd_refresh_krb5_machine_credential_internal(hostname, NULL,
- service, srchost);
+ service, srchost,
+ force_renew);
}

/*
diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
index 2415205a..62c91a0e 100644
--- a/utils/gssd/krb5_util.h
+++ b/utils/gssd/krb5_util.h
@@ -16,7 +16,8 @@ int gssd_get_krb5_machine_cred_list(char ***list);
void gssd_free_krb5_machine_cred_list(char **list);
void gssd_destroy_krb5_principals(int destroy_machine_creds);
int gssd_refresh_krb5_machine_credential(char *hostname,
- char *service, char *srchost);
+ char *service, char *srchost,
+ int force_renew);
char *gssd_k5_err_msg(krb5_context context, krb5_error_code code);
void gssd_k5_get_default_realm(char **def_realm);

--
2.39.1

2023-10-04 17:33:02

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 2/3] nfs-utils: gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for machine credentials

From: Olga Kornievskaia <[email protected]>

During context establishment, when the client received
KRB5_AP_ERR_BAD_INTEGRITY error, it might be due to the server
updating its key material. To handle such error, get a new
service ticket and re-try the AP_REQ.

Signed-off-by: Olga Kornievskaia <[email protected]>
---
utils/gssd/gssd_proc.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index 4fb6b72d..e5cc1d98 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -412,13 +412,27 @@ create_auth_rpc_client(struct clnt_info *clp,
tid, tgtname);
auth = authgss_create_default(rpc_clnt, tgtname, &sec);
if (!auth) {
+ if (sec.minor_status == KRB5KRB_AP_ERR_BAD_INTEGRITY) {
+ printerr(2, "WARNING: server=%s failed context "
+ "creation with KRB5_AP_ERR_BAD_INTEGRITY\n",
+ clp->servername);
+ if (cred == GSS_C_NO_CREDENTIAL)
+ retval = gssd_refresh_krb5_machine_credential(clp->servername,
+ "*", NULL, 1);
+ if (!retval) {
+ auth = authgss_create_default(rpc_clnt, tgtname,
+ &sec);
+ if (auth)
+ goto success;
+ }
+ }
/* Our caller should print appropriate message */
printerr(2, "WARNING: Failed to create krb5 context for "
"user with uid %d for server %s\n",
uid, tgtname);
goto out_fail;
}
-
+success:
/* Success !!! */
rpc_clnt->cl_auth = auth;
*clnt_return = rpc_clnt;
--
2.39.1

2023-10-04 17:33:05

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH 3/3] nfs-utils: gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for user credentials

From: Olga Kornievskaia <[email protected]>

Unlike the machine credential case, we can't throw away the ticket
cache and use the keytab to renew the credentials. Instead, we
need to remove the service ticket for the server that returned
KRB5_AP_ERR_BAD_INTEGRITY and try again.

Signed-off-by: Olga Kornievskaia <[email protected]>
---
utils/gssd/gssd_proc.c | 2 ++
utils/gssd/krb5_util.c | 42 ++++++++++++++++++++++++++++++++++++++++++
utils/gssd/krb5_util.h | 1 +
3 files changed, 45 insertions(+)

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index e5cc1d98..a96647df 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -419,6 +419,8 @@ create_auth_rpc_client(struct clnt_info *clp,
if (cred == GSS_C_NO_CREDENTIAL)
retval = gssd_refresh_krb5_machine_credential(clp->servername,
"*", NULL, 1);
+ else
+ retval = gssd_k5_remove_bad_service_cred(clp->servername);
if (!retval) {
auth = authgss_create_default(rpc_clnt, tgtname,
&sec);
diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
index f6ce1fec..6f66ef4f 100644
--- a/utils/gssd/krb5_util.c
+++ b/utils/gssd/krb5_util.c
@@ -1553,6 +1553,48 @@ gssd_acquire_user_cred(gss_cred_id_t *gss_cred)
return ret;
}

+/* Removed a service ticket for nfs/<name> from the ticket cache
+ */
+int
+gssd_k5_remove_bad_service_cred(char *name)
+{
+ krb5_creds in_creds, out_creds;
+ krb5_error_code ret;
+ krb5_context context;
+ krb5_ccache cache;
+ krb5_principal principal;
+ int retflags = KRB5_TC_MATCH_SRV_NAMEONLY;
+ char srvname[1024];
+
+ ret = krb5_init_context(&context);
+ if (ret)
+ goto out_cred;
+ ret = krb5_cc_default(context, &cache);
+ if (ret)
+ goto out_free_context;
+ ret = krb5_cc_get_principal(context, cache, &principal);
+ if (ret)
+ goto out_close_cache;
+ memset(&in_creds, 0, sizeof(in_creds));
+ in_creds.client = principal;
+ sprintf(srvname, "nfs/%s", name);
+ ret = krb5_parse_name(context, srvname, &in_creds.server);
+ if (ret)
+ goto out_free_principal;
+ ret = krb5_cc_retrieve_cred(context, cache, retflags, &in_creds, &out_creds);
+ if (ret)
+ goto out_free_principal;
+ ret = krb5_cc_remove_cred(context, cache, 0, &out_creds);
+out_free_principal:
+ krb5_free_principal(context, principal);
+out_close_cache:
+ krb5_cc_close(context, cache);
+out_free_context:
+ krb5_free_context(context);
+out_cred:
+ return ret;
+}
+
#ifdef HAVE_SET_ALLOWABLE_ENCTYPES
/*
* this routine obtains a credentials handle via gss_acquire_cred()
diff --git a/utils/gssd/krb5_util.h b/utils/gssd/krb5_util.h
index 62c91a0e..7ef87018 100644
--- a/utils/gssd/krb5_util.h
+++ b/utils/gssd/krb5_util.h
@@ -22,6 +22,7 @@ char *gssd_k5_err_msg(krb5_context context, krb5_error_code code);
void gssd_k5_get_default_realm(char **def_realm);

int gssd_acquire_user_cred(gss_cred_id_t *gss_cred);
+int gssd_k5_remove_bad_service_cred(char *srvname);

#ifdef HAVE_SET_ALLOWABLE_ENCTYPES
extern int limit_to_legacy_enctypes;
--
2.39.1

2023-10-16 16:16:21

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] nfs-utils: gssd support for KRB5_AP_ERR_BAD_INTEGRITY



On 10/4/23 1:32 PM, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <[email protected]>
>
> Together with libtirpc patch this series attempts to provide
> support for handling KRB5_AP_ERR_BAD_INTEGRITY.
>
> Such error can be returned by the server when it has changed
> its key material and the client is still using the service
> ticket that was issues prior to the change.
>
> Upon calling authgss_create_default() and receiving a NULL
> context, we can inspect the returned structure to see
> if gss major/minor error code was set. If the client
> determines that it received KRB5_AP_ERR_BAD_INTEGRITY error,
> it will proceed to handle it based on what type of credentials
> were used for context establishement. If machine credentials
> were used, the client can call into a routine and force
> credential renewal. If user credentials were used, the client
> needs to remove the existing service ticket and then retry
> the request.
>
> -- fix compile warning in libtirpc patch
>
> Olga Kornievskaia (3):
> gssd: enable forcing cred renewal using the keytab
> gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for machine credentials
> gssd: handle KRB5_AP_ERR_BAD_INTEGRITY for user credentials
Committed... (tag: nfs-utils-2-6-4-rc4)

steved.

>
> utils/gssd/gssd_proc.c | 20 ++++++++++++--
> utils/gssd/krb5_util.c | 62 ++++++++++++++++++++++++++++++++++++------
> utils/gssd/krb5_util.h | 4 ++-
> 3 files changed, 75 insertions(+), 11 deletions(-)
>