12:33 p.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
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