PR#171: Properly renew expired credentials - gssproxy

gssproxy

#171 Properly renew expired credentials

Closed 7 years ago by rharwood. Opened 7 years ago by simo.

simo/gssproxy expirefix into master

Properly renew expired credentials

Simo Sorce • 7 years ago

b54480c

proxy/src/gp_creds.c

file modified

+9 -5

		`@@ -629,8 +629,12 @@`
		`ret_maj = gp_check_cred(&ret_min, in_cred, desired_name, cred_usage);`
		`if (ret_maj == GSS_S_COMPLETE) {`
		`return GSS_S_COMPLETE;`
		`- } else if (ret_maj != GSS_S_CREDENTIALS_EXPIRED &&`
		`- ret_maj != GSS_S_NO_CRED) {`
		`+ } else if (ret_maj == GSS_S_CREDENTIALS_EXPIRED \|\|`
		`+ ret_maj == GSS_S_NO_CRED) {`
		`+ /* continue and try to obtain new creds */`
		`+ ret_maj = 0;`
		`+ ret_min = 0;`
		`+ } else {`
		`*min = ret_min;`
		`return GSS_S_CRED_UNAVAIL;`
		`}`
		`@@ -639,14 +643,14 @@`
		`if (acquire_type == ACQ_NORMAL) {`
		`ret_min = gp_get_cred_environment(gpcall, desired_name, &req_name,`
		`&cred_usage, &cred_store);`
		`+ if (ret_min) {`
		`+ ret_maj = GSS_S_CRED_UNAVAIL;`
		`+ }`
		`} else if (desired_name) {`
		`ret_maj = gp_conv_gssx_to_name(&ret_min, desired_name, &req_name);`
		`}`
		`if (ret_maj) {`
		`goto done;`
		`- } else if (ret_min) {`
		`- ret_maj = GSS_S_CRED_UNAVAIL;`
		`- goto done;`
		`}`

		`if (!try_impersonate(gpcall->service, cred_usage, acquire_type)) {`

proxy/src/mechglue/gpp_acquire_cred.c

file modified

		`@@ -186,6 +186,11 @@`
		`}`

		`if (behavior == GPP_REMOTE_FIRST) {`
		`+ if (maj != GSS_S_COMPLETE) {`
		`+ /* save errors */`
		`+ tmaj = maj;`
		`+ tmin = min;`
		`+ }`
		`/* So remote failed, but we can fallback to local, try that */`
		`maj = acquire_local(&min, NULL, name,`
		`time_req, desired_mechs, cred_usage, cred_store,`

simo commented 7 years ago

When a caller imports expired credentials, we aim to actually renew them
if we can. However due to incorrect checks and not clearing of the
maj_err variable after checks we end up returning an error instead.

https://pagure.io/gssproxy/issue/170

rebased

7 years ago

rharwood commented 7 years ago

Looks good to me. However, it is missing your "Signed-off-by:" which we currently require (and that I cannot add). Also, I think pagure will take "Resolves: #170" as the syntax for closing an issue from a commit.

rebased

7 years ago

simo commented 7 years ago

Fixed, sorry about the signed-off-by

rharwood commented 7 years ago

Pushed to master as dc46232