From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> SASL may prompt for credentials after either a 'start' or 'step' invocation. In both cases the code to handle this is the same. Refactor this code into a separate method to reduce the duplication, since the complexity is about to grow * src/remote/remote_driver.c: Refactor interaction with SASL --- src/remote/remote_driver.c | 135 +++++++++++++++++++++++++------------------ 1 files changed, 78 insertions(+), 57 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index bc6fea2..1faaf9e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2863,7 +2863,8 @@ static sasl_callback_t *remoteAuthMakeCallbacks(int *credtype, int ncredtype) * are basically a 1-to-1 copy of each other. */ static int remoteAuthMakeCredentials(sasl_interact_t *interact, - virConnectCredentialPtr *cred) + virConnectCredentialPtr *cred, + size_t *ncred) { int ninteract; if (!cred) @@ -2889,16 +2890,8 @@ static int remoteAuthMakeCredentials(sasl_interact_t *interact, (*cred)[ninteract].result = NULL; } - return ninteract; -} - -static void remoteAuthFreeCredentials(virConnectCredentialPtr cred, - int ncred) -{ - int i; - for (i = 0 ; i < ncred ; i++) - VIR_FREE(cred[i].result); - VIR_FREE(cred); + *ncred = ninteract; + return 0; } @@ -2919,6 +2912,69 @@ static void remoteAuthFillInteract(virConnectCredentialPtr cred, } } + +struct remoteAuthInteractState { + sasl_interact_t *interact; + virConnectCredentialPtr cred; + size_t ncred; +}; + + +static void remoteAuthInteractStateClear(struct remoteAuthInteractState *state) +{ + size_t i; + if (!state) + return; + + for (i = 0 ; i < state->ncred ; i++) + VIR_FREE(state->cred[i].result); + VIR_FREE(state->cred); + state->ncred = 0; +} + + +static int remoteAuthInteract(struct remoteAuthInteractState *state, + virConnectAuthPtr auth) +{ + int ret = -1; + + remoteAuthInteractStateClear(state); + + if (remoteAuthMakeCredentials(state->interact, &state->cred, &state->ncred) < 0) { + remoteError(VIR_ERR_AUTH_FAILED, "%s", + _("Failed to make auth credentials")); + goto cleanup; + } + + /* Run the authentication callback */ + if (!auth || !auth->cb) { + remoteError(VIR_ERR_AUTH_FAILED, "%s", + _("No authentication callback available")); + goto cleanup; + } + + if ((*(auth->cb))(state->cred, state->ncred, auth->cbdata) < 0) { + remoteError(VIR_ERR_AUTH_FAILED, "%s", + _("Failed to collect auth credentials")); + goto cleanup; + } + + remoteAuthFillInteract(state->cred, state->interact); + /* + * 'interact' now has pointers to strings in 'state->cred' + * so we must not free state->cred until the *next* + * sasl_start/step function is complete. Hence we + * call remoteAuthInteractStateClear() at the *start* + * of this method, rather than the end. + */ + + ret = 0; + +cleanup: + return ret; +} + + /* Perform the SASL authentication process */ static int @@ -2937,13 +2993,13 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, int err, complete; int ssf; sasl_callback_t *saslcb = NULL; - sasl_interact_t *interact = NULL; - virConnectCredentialPtr cred = NULL; - int ncred = 0; int ret = -1; const char *mechlist; virNetSASLContextPtr saslCtxt; virNetSASLSessionPtr sasl = NULL; + struct remoteAuthInteractState state; + + memset(&state, 0, sizeof(state)); VIR_DEBUG("Client initialize SASL authentication"); @@ -3010,7 +3066,7 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, VIR_DEBUG("Client start negotiation mechlist '%s'", mechlist); if ((err = virNetSASLSessionClientStart(sasl, mechlist, - &interact, + &state.interact, &clientout, &clientoutlen, &mech)) < 0) @@ -3018,29 +3074,11 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, /* Need to gather some credentials from the client */ if (err == VIR_NET_SASL_INTERACT) { - const char *msg; - if (cred) { - remoteAuthFreeCredentials(cred, ncred); - cred = NULL; - } - if ((ncred = remoteAuthMakeCredentials(interact, &cred)) < 0) { - remoteError(VIR_ERR_AUTH_FAILED, "%s", - _("Failed to make auth credentials")); + if (remoteAuthInteract(&state, auth) < 0) { VIR_FREE(iret.mechlist); goto cleanup; } - /* Run the authentication callback */ - if (auth && auth->cb) { - if ((*(auth->cb))(cred, ncred, auth->cbdata) >= 0) { - remoteAuthFillInteract(cred, interact); - goto restart; - } - msg = "Failed to collect auth credentials"; - } else { - msg = "No authentication callback available"; - } - remoteError(VIR_ERR_AUTH_FAILED, "%s", msg); - goto cleanup; + goto restart; } VIR_FREE(iret.mechlist); @@ -3081,35 +3119,18 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, if ((err = virNetSASLSessionClientStep(sasl, serverin, serverinlen, - &interact, + &state.interact, &clientout, &clientoutlen)) < 0) goto cleanup; /* Need to gather some credentials from the client */ if (err == VIR_NET_SASL_INTERACT) { - const char *msg; - if (cred) { - remoteAuthFreeCredentials(cred, ncred); - cred = NULL; - } - if ((ncred = remoteAuthMakeCredentials(interact, &cred)) < 0) { - remoteError(VIR_ERR_AUTH_FAILED, "%s", - _("Failed to make auth credentials")); + if (remoteAuthInteract(&state, auth) < 0) { + VIR_FREE(iret.mechlist); goto cleanup; } - /* Run the authentication callback */ - if (auth && auth->cb) { - if ((*(auth->cb))(cred, ncred, auth->cbdata) >= 0) { - remoteAuthFillInteract(cred, interact); - goto restep; - } - msg = _("Failed to collect auth credentials"); - } else { - msg = _("No authentication callback available"); - } - remoteError(VIR_ERR_AUTH_FAILED, "%s", msg); - goto cleanup; + goto restep; } VIR_FREE(serverin); @@ -3171,8 +3192,8 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, cleanup: VIR_FREE(serverin); + remoteAuthInteractStateClear(&state); VIR_FREE(saslcb); - remoteAuthFreeCredentials(cred, ncred); virNetSASLSessionFree(sasl); virNetSASLContextFree(saslCtxt); -- 1.7.7.6 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list