Add min lifetime option #35
Conversation
Currently gp_debug_toggle() is called at the end of load_config(), and as a result any GPDEBUG() calls that happen during config processing are no-ops. Signed-off-by: Scott Mayhew <smayhew@redhat.com>
src/gp_config.c
Outdated
| @@ -538,6 +538,17 @@ static int load_services(struct gp_config *cfg, struct gp_ini_context *ctx) | |||
| goto done; | |||
| } | |||
| } | |||
|
|
|||
| cfg->svcs[n]->min_lifetime = 0; | |||
There was a problem hiding this comment.
Can you please ad a DEFAULT_MIN_LIFETIME macro for this?
We might actually set a default min_lifetime that is not 0, thinking about it.
There was a problem hiding this comment.
Okay. Have you thought about whether you want the default to be non-zero?
src/gp_creds.c
Outdated
| ret_maj = GSS_S_COMPLETE; | ||
| if (svc->min_lifetime && lifetime < svc->min_lifetime) { | ||
| GPDEBUG("%s: lifetime (%u) less than min_lifetime (%u) " | ||
| "for service \"%s\" - returning GSS_S_CREDENTIALS_EXPIRED\n", |
There was a problem hiding this comment.
Please drop the " - returning GSS_S_CREDENTIALS_EXPIRED" part, as that will be logged already as the return error of the function in debug mode. Replace with just "returning".
src/gp_creds.c
Outdated
| @@ -492,6 +492,7 @@ static int gp_get_cred_environment(struct gp_call_ctx *gpcall, | |||
| } | |||
|
|
|||
| static uint32_t gp_check_cred(uint32_t *min, | |||
| struct gp_call_ctx *gpcall, | |||
There was a problem hiding this comment.
You are not using gpcall, please pass in a pointer to gp_service directly instead.
|
We should probably have a test for this, I wonder if we can add one easily ? |
I would think a test program would need to use of gss_inquire_cred() to check the lifetime, and from what I've seen with rpc.gssd, when we call gss_inquire_cred() we don't get the correct lifetime back from gssproxy (it sort of sounds like the same issue being described in #33). |
|
I think we want to set a DEFAULT_MIN_LIFETIME of 15, it's not a lot, but should allow at least a couple of roundtrips before creds become useless. |
It's possible for gssproxy to return a cached credential with a very small remaining lifetime. This can be problematic for NFS clients since it requires a round trip to the NFS server to establish a GSS context. Add a min_lifetime option that represents the lowest value that the lifetime of the cached credential can be. Any lower than that, and gp_check_cred() returns GSS_S_CREDENTIALS_EXPIRED, so that gp_add_krb5_creds() is forced to try to obtain a new credential. Signed-off-by: Scott Mayhew <smayhew@redhat.com>
scottmayhew
force-pushed
the
add-min-lifetime-option
branch
from
d3ce1f9 to
e420f95
Compare
|
Thanks Scott, I think this is good enough to be merged, I will think later about how to test, given I will need to deal with time manipulation in order to not have to just slow down tests. |
It's possible for gssproxy to return a cached credential with a very small remaining lifetime. This can be problematic for NFS clients since it requires a round trip to the NFS server to establish a GSS context. Add a min_lifetime option that represents the lowest value that the lifetime of the cached credential can be. Any lower than that, and gp_check_cred() returns GSS_S_CREDENTIALS_EXPIRED, so that gp_add_krb5_creds() is forced to try to obtain a new credential.
This fixes an issue where NFS clients (particularly ones using the interposer mechanism) can receive EKEYEXPIRED/EACCES/EIO at the time of Kerberos ticket expiration.
Also, fix an issue where debug messages aren't getting logged during config parsing.