[Spice-devel] [PATCH spice-server 13/16] Handle SASL initialisation mainly in red-stream.c
Frediano Ziglio
fziglio at redhat.com
Thu Dec 14 10:07:45 UTC 2017
Asynchronous code jumping from a file to another is tedious to read
also having code handling the same stuff in two files does not look
a good design.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
server/red-stream.c | 118 +++++++++++++++++++++++++++++++++-------------------
server/red-stream.h | 11 +----
server/reds.c | 118 ++++++----------------------------------------------
3 files changed, 89 insertions(+), 158 deletions(-)
diff --git a/server/red-stream.c b/server/red-stream.c
index ab4ae02cf..7cc24faa1 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -730,6 +730,33 @@ static int auth_sasl_check_ssf(RedSASL *sasl, int *runSSF)
return 1;
}
+typedef struct RedSASLAuth {
+ RedStream *stream;
+ RedSaslResult result_cb;
+ void *result_opaque;
+ AsyncReadError saved_error;
+} RedSASLAuth;
+
+// handle SASL termination, either success or error
+// NOTE: After this function is called usually there should be a
+// return or the function should exit
+static void red_sasl_async_result(RedSASLAuth *auth, RedSaslError err)
+{
+ auth->stream->priv->async_read.error = auth->saved_error;
+ auth->result_cb(auth->result_opaque, err);
+ g_free(auth);
+}
+
+static void red_sasl_error(void *opaque, int err)
+{
+ RedSASLAuth *auth = opaque;
+ auth->stream->priv->async_read.error = auth->saved_error;
+ if (auth->saved_error) {
+ auth->saved_error(auth->result_opaque, err);
+ }
+ g_free(auth);
+}
+
/*
* Step Msg
*
@@ -747,8 +774,11 @@ static int auth_sasl_check_ssf(RedSASL *sasl, int *runSSF)
#define SASL_MAX_MECHNAME_LEN 100
#define SASL_DATA_MAX_LEN (1024 * 1024)
-RedSaslError red_sasl_handle_auth_step(RedStream *stream, AsyncReadDone read_cb, void *opaque)
+static void red_sasl_handle_auth_steplen(void *opaque);
+
+static void red_sasl_handle_auth_step(void *opaque)
{
+ RedStream *stream = ((RedSASLAuth *)opaque)->stream;
const char *serverout;
unsigned int serveroutlen;
int err;
@@ -774,13 +804,13 @@ RedSaslError red_sasl_handle_auth_step(RedStream *stream, AsyncReadDone read_cb,
err != SASL_CONTINUE) {
spice_warning("sasl step failed %d (%s)",
err, sasl_errdetail(sasl->conn));
- return RED_SASL_ERROR_GENERIC;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_GENERIC);
}
if (serveroutlen > SASL_DATA_MAX_LEN) {
spice_warning("sasl step reply data too long %d",
serveroutlen);
- return RED_SASL_ERROR_INVALID_DATA;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_INVALID_DATA);
}
spice_debug("SASL return data %d bytes, %p", serveroutlen, serverout);
@@ -800,8 +830,8 @@ RedSaslError red_sasl_handle_auth_step(RedStream *stream, AsyncReadDone read_cb,
spice_debug("%s", "Authentication must continue (step)");
/* Wait for step length */
red_stream_async_read(stream, (uint8_t *)&sasl->len, sizeof(uint32_t),
- read_cb, opaque);
- return RED_SASL_ERROR_CONTINUE;
+ red_sasl_handle_auth_steplen, opaque);
+ return;
} else {
int ssf;
@@ -819,7 +849,7 @@ RedSaslError red_sasl_handle_auth_step(RedStream *stream, AsyncReadDone read_cb,
sasl->runSSF = ssf;
red_stream_disable_writev(stream); /* make sure writev isn't called directly anymore */
- return RED_SASL_ERROR_OK;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_OK);
}
authreject:
@@ -827,31 +857,26 @@ authreject:
red_stream_write_u32(stream, sizeof("Authentication failed"));
red_stream_write_all(stream, "Authentication failed", sizeof("Authentication failed"));
- return RED_SASL_ERROR_AUTH_FAILED;
+ red_sasl_async_result(opaque, RED_SASL_ERROR_AUTH_FAILED);
}
-RedSaslError red_sasl_handle_auth_steplen(RedStream *stream, AsyncReadDone read_cb, void *opaque)
+static void red_sasl_handle_auth_steplen(void *opaque)
{
+ RedStream *stream = ((RedSASLAuth *)opaque)->stream;
RedSASL *sasl = &stream->priv->sasl;
spice_debug("Got steplen %d", sasl->len);
if (sasl->len > SASL_DATA_MAX_LEN) {
spice_warning("Too much SASL data %d", sasl->len);
- return RED_SASL_ERROR_INVALID_DATA;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_INVALID_DATA);
}
if (sasl->len == 0) {
- read_cb(opaque);
- /* FIXME: can't report potential errors correctly here,
- * but read_cb() will have done the needed RedLinkInfo cleanups
- * if an error occurs, so the caller should not need to do more
- * treatment */
- return RED_SASL_ERROR_OK;
+ red_sasl_handle_auth_step(opaque);
} else {
sasl->data = g_realloc(sasl->data, sasl->len);
red_stream_async_read(stream, (uint8_t *)sasl->data, sasl->len,
- read_cb, opaque);
- return RED_SASL_ERROR_OK;
+ red_sasl_handle_auth_step, opaque);
}
}
@@ -870,8 +895,9 @@ RedSaslError red_sasl_handle_auth_steplen(RedStream *stream, AsyncReadDone read_
* u8 continue
*/
-RedSaslError red_sasl_handle_auth_start(RedStream *stream, AsyncReadDone read_cb, void *opaque)
+static void red_sasl_handle_auth_start(void *opaque)
{
+ RedStream *stream = ((RedSASLAuth *)opaque)->stream;
const char *serverout;
unsigned int serveroutlen;
int err;
@@ -898,13 +924,13 @@ RedSaslError red_sasl_handle_auth_start(RedStream *stream, AsyncReadDone read_cb
err != SASL_CONTINUE) {
spice_warning("sasl start failed %d (%s)",
err, sasl_errdetail(sasl->conn));
- return RED_SASL_ERROR_INVALID_DATA;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_INVALID_DATA);
}
if (serveroutlen > SASL_DATA_MAX_LEN) {
spice_warning("sasl start reply data too long %d",
- serveroutlen);
- return RED_SASL_ERROR_INVALID_DATA;
+ serveroutlen);
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_INVALID_DATA);
}
spice_debug("SASL return data %d bytes, %p", serveroutlen, serverout);
@@ -924,8 +950,8 @@ RedSaslError red_sasl_handle_auth_start(RedStream *stream, AsyncReadDone read_cb
spice_debug("%s", "Authentication must continue (start)");
/* Wait for step length */
red_stream_async_read(stream, (uint8_t *)&sasl->len, sizeof(uint32_t),
- read_cb, opaque);
- return RED_SASL_ERROR_CONTINUE;
+ red_sasl_handle_auth_steplen, opaque);
+ return;
} else {
int ssf;
@@ -942,7 +968,8 @@ RedSaslError red_sasl_handle_auth_start(RedStream *stream, AsyncReadDone read_cb
*/
sasl->runSSF = ssf;
red_stream_disable_writev(stream); /* make sure writev isn't called directly anymore */
- return RED_SASL_ERROR_OK;
+
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_OK);
}
authreject:
@@ -950,32 +977,32 @@ authreject:
red_stream_write_u32(stream, sizeof("Authentication failed"));
red_stream_write_all(stream, "Authentication failed", sizeof("Authentication failed"));
- return RED_SASL_ERROR_AUTH_FAILED;
+ red_sasl_async_result(opaque, RED_SASL_ERROR_AUTH_FAILED);
}
-RedSaslError red_sasl_handle_auth_startlen(RedStream *stream, AsyncReadDone read_cb, void *opaque)
+static void red_sasl_handle_auth_startlen(void *opaque)
{
+ RedStream *stream = ((RedSASLAuth *)opaque)->stream;
RedSASL *sasl = &stream->priv->sasl;
spice_debug("Got client start len %d", sasl->len);
if (sasl->len > SASL_DATA_MAX_LEN) {
spice_warning("Too much SASL data %d", sasl->len);
- return RED_SASL_ERROR_INVALID_DATA;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_INVALID_DATA);
}
if (sasl->len == 0) {
- return RED_SASL_ERROR_RETRY;
+ return red_sasl_handle_auth_start(opaque);
}
sasl->data = g_realloc(sasl->data, sasl->len);
red_stream_async_read(stream, (uint8_t *)sasl->data, sasl->len,
- read_cb, opaque);
-
- return RED_SASL_ERROR_OK;
+ red_sasl_handle_auth_start, opaque);
}
-bool red_sasl_handle_auth_mechname(RedStream *stream, AsyncReadDone read_cb, void *opaque)
+static void red_sasl_handle_auth_mechname(void *opaque)
{
+ RedStream *stream = ((RedSASLAuth *)opaque)->stream;
RedSASL *sasl = &stream->priv->sasl;
sasl->mechname[sasl->len] = '\0';
@@ -986,36 +1013,33 @@ bool red_sasl_handle_auth_mechname(RedStream *stream, AsyncReadDone read_cb, voi
sprintf(quoted_mechname, ",%s,", sasl->mechname);
if (strchr(sasl->mechname, ',') || !strstr(sasl->mechlist, quoted_mechname)) {
- return false;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_INVALID_DATA);
}
spice_debug("Validated mechname '%s'", sasl->mechname);
red_stream_async_read(stream, (uint8_t *)&sasl->len, sizeof(uint32_t),
- read_cb, opaque);
-
- return true;
+ red_sasl_handle_auth_startlen, opaque);
}
-bool red_sasl_handle_auth_mechlen(RedStream *stream, AsyncReadDone read_cb, void *opaque)
+static void red_sasl_handle_auth_mechlen(void *opaque)
{
+ RedStream *stream = ((RedSASLAuth *)opaque)->stream;
RedSASL *sasl = &stream->priv->sasl;
if (sasl->len < 1 || sasl->len > SASL_MAX_MECHNAME_LEN) {
spice_warning("Got bad client mechname len %d", sasl->len);
- return false;
+ return red_sasl_async_result(opaque, RED_SASL_ERROR_INVALID_DATA);
}
sasl->mechname = g_malloc(sasl->len + 1);
spice_debug("Wait for client mechname");
red_stream_async_read(stream, (uint8_t *)sasl->mechname, sasl->len,
- read_cb, opaque);
-
- return true;
+ red_sasl_handle_auth_mechname, opaque);
}
-bool red_sasl_start_auth(RedStream *stream, AsyncReadDone read_cb, void *opaque)
+bool red_sasl_start_auth(RedStream *stream, RedSaslResult result_cb, void *opaque)
{
const char *mechlist = NULL;
sasl_security_properties_t secprops;
@@ -1023,6 +1047,7 @@ bool red_sasl_start_auth(RedStream *stream, AsyncReadDone read_cb, void *opaque)
char *localAddr, *remoteAddr;
int mechlistlen;
RedSASL *sasl = &stream->priv->sasl;
+ RedSASLAuth *auth;
if (!(localAddr = red_stream_get_local_address(stream))) {
goto error;
@@ -1117,9 +1142,16 @@ bool red_sasl_start_auth(RedStream *stream, AsyncReadDone read_cb, void *opaque)
goto error;
}
+ auth = g_new0(RedSASLAuth, 1);
+ auth->stream = stream;
+ auth->result_cb = result_cb;
+ auth->result_opaque = opaque;
+ auth->saved_error = stream->priv->async_read.error;
+ stream->priv->async_read.error = red_sasl_error;
+
spice_debug("Wait for client mechname length");
red_stream_async_read(stream, (uint8_t *)&sasl->len, sizeof(uint32_t),
- read_cb, opaque);
+ red_sasl_handle_auth_mechlen, auth);
return true;
diff --git a/server/red-stream.h b/server/red-stream.h
index a8d855c2d..4d5075ed1 100644
--- a/server/red-stream.h
+++ b/server/red-stream.h
@@ -73,17 +73,10 @@ typedef enum {
RED_SASL_ERROR_OK,
RED_SASL_ERROR_GENERIC,
RED_SASL_ERROR_INVALID_DATA,
- RED_SASL_ERROR_RETRY,
- RED_SASL_ERROR_CONTINUE,
RED_SASL_ERROR_AUTH_FAILED
} RedSaslError;
-RedSaslError red_sasl_handle_auth_step(RedStream *stream, AsyncReadDone read_cb, void *opaque);
-RedSaslError red_sasl_handle_auth_steplen(RedStream *stream, AsyncReadDone read_cb, void *opaque);
-RedSaslError red_sasl_handle_auth_start(RedStream *stream, AsyncReadDone read_cb, void *opaque);
-RedSaslError red_sasl_handle_auth_startlen(RedStream *stream, AsyncReadDone read_cb, void *opaque);
-bool red_sasl_handle_auth_mechname(RedStream *stream, AsyncReadDone read_cb, void *opaque);
-bool red_sasl_handle_auth_mechlen(RedStream *stream, AsyncReadDone read_cb, void *opaque);
-bool red_sasl_start_auth(RedStream *stream, AsyncReadDone read_cb, void *opaque);
+typedef void (*RedSaslResult)(void *opaque, RedSaslError err);
+bool red_sasl_start_auth(RedStream *stream, RedSaslResult result_cb, void *opaque);
#endif /* RED_STREAM_H_ */
diff --git a/server/reds.c b/server/reds.c
index 761afa772..ab3e21ffc 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2100,123 +2100,29 @@ static void reds_get_spice_ticket(RedLinkInfo *link)
}
#if HAVE_SASL
-/*
- * Step Msg
- *
- * Input from client:
- *
- * u32 clientin-length
- * u8-array clientin-string
- *
- * Output to client:
- *
- * u32 serverout-length
- * u8-array serverout-strin
- * u8 continue
- */
-
-static void reds_handle_auth_sasl_steplen(void *opaque);
-
-static void reds_handle_auth_sasl_step(void *opaque)
-{
- RedLinkInfo *link = (RedLinkInfo *)opaque;
- RedSaslError status;
-
- status = red_sasl_handle_auth_step(link->stream, reds_handle_auth_sasl_steplen, link);
- if (status == RED_SASL_ERROR_OK) {
- reds_handle_link(link);
- } else if (status != RED_SASL_ERROR_CONTINUE) {
- reds_link_free(link);
- }
-}
-
-static void reds_handle_auth_sasl_steplen(void *opaque)
-{
- RedLinkInfo *link = (RedLinkInfo *)opaque;
- RedSaslError status;
-
- status = red_sasl_handle_auth_steplen(link->stream, reds_handle_auth_sasl_step, link);
- if (status != RED_SASL_ERROR_OK) {
- reds_link_free(link);
- }
-}
-
-/*
- * Start Msg
- *
- * Input from client:
- *
- * u32 clientin-length
- * u8-array clientin-string
- *
- * Output to client:
- *
- * u32 serverout-length
- * u8-array serverout-strin
- * u8 continue
- */
-
-
-static void reds_handle_auth_sasl_start(void *opaque)
+static void reds_handle_sasl_result(void *opaque, RedSaslError status)
{
RedLinkInfo *link = (RedLinkInfo *)opaque;
- RedSaslError status;
-
- status = red_sasl_handle_auth_start(link->stream, reds_handle_auth_sasl_steplen, link);
- if (status == RED_SASL_ERROR_OK) {
- reds_handle_link(link);
- } else if (status != RED_SASL_ERROR_CONTINUE) {
- reds_link_free(link);
- }
-}
-static void reds_handle_auth_startlen(void *opaque)
-{
- RedLinkInfo *link = (RedLinkInfo *)opaque;
- RedSaslError status;
-
- status = red_sasl_handle_auth_startlen(link->stream, reds_handle_auth_sasl_start, link);
switch (status) {
- case RED_SASL_ERROR_OK:
- break;
- case RED_SASL_ERROR_RETRY:
- reds_handle_auth_sasl_start(opaque);
- break;
- case RED_SASL_ERROR_GENERIC:
- case RED_SASL_ERROR_INVALID_DATA:
- reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
- reds_link_free(link);
- break;
- default:
- g_warn_if_reached();
- reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
- reds_link_free(link);
- break;
- }
-}
-
-static void reds_handle_auth_mechname(void *opaque)
-{
- RedLinkInfo *link = (RedLinkInfo *)opaque;
-
- if (!red_sasl_handle_auth_mechname(link->stream, reds_handle_auth_startlen, link)) {
- reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
+ case RED_SASL_ERROR_OK:
+ reds_handle_link(link);
+ break;
+ case RED_SASL_ERROR_GENERIC:
+ case RED_SASL_ERROR_INVALID_DATA:
+ reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
reds_link_free(link);
- }
-}
-
-static void reds_handle_auth_mechlen(void *opaque)
-{
- RedLinkInfo *link = (RedLinkInfo *)opaque;
-
- if (!red_sasl_handle_auth_mechlen(link->stream, reds_handle_auth_mechname, link)) {
+ break;
+ default:
+ // in these cases error was reported using SASL protocol
reds_link_free(link);
+ break;
}
}
static void reds_start_auth_sasl(RedLinkInfo *link)
{
- if (!red_sasl_start_auth(link->stream, reds_handle_auth_mechlen, link)) {
+ if (!red_sasl_start_auth(link->stream, reds_handle_sasl_result, link)) {
reds_link_free(link);
}
}
--
2.14.3
More information about the Spice-devel
mailing list