Re: [PATCH spice-server v3 01/12] red-stream: Simplify mechname matching

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi


On 12/22/2017 12:07 PM, Frediano Ziglio wrote:
Avoid over complicated matching using quoting and a simple strstr
operation.
The mech names are separated and quoted with the same chararacter (',')
making possible to search for ",MECHNAME," instead of manually check for
prefix and suffix after the search for "MECHNAME".

Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
Changes since v1:
- use strstr == NULL instead of !strstr;
- improve commit message.
---
 server/red-stream.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/server/red-stream.c b/server/red-stream.c
index 55e2c3e02..f4637808c 100644
--- a/server/red-stream.c
+++ b/server/red-stream.c
@@ -744,6 +744,7 @@ static int auth_sasl_check_ssf(RedSASL *sasl, int *runSSF)
  * u8-array serverout-strin
  * u8 continue
  */
+#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)
@@ -981,24 +982,11 @@ bool red_sasl_handle_auth_mechname(RedStream *stream, AsyncReadDone read_cb, voi
     spice_debug("Got client mechname '%s' check against '%s'",
                sasl->mechname, sasl->mechlist);
 
-    if (strncmp(sasl->mechlist, sasl->mechname, sasl->len) == 0) {
-        if (sasl->mechlist[sasl->len] != '\0' &&
-            sasl->mechlist[sasl->len] != ',') {
-            spice_debug("One %d", sasl->mechlist[sasl->len]);
-            return false;
-        }
-    } else {
-        char *offset = strstr(sasl->mechlist, sasl->mechname);
-        spice_debug("Two %p", offset);
-        if (!offset) {
-            return false;
-        }
-        spice_debug("Two '%s'", offset);
-        if (offset[-1] != ',' ||
-            (offset[sasl->len] != '\0'&&
-             offset[sasl->len] != ',')) {
-            return false;
-        }
+    char quoted_mechname[SASL_MAX_MECHNAME_LEN + 4];

Could it be + 2 ?

+    sprintf(quoted_mechname, ",%s,", sasl->mechname);
+
+    if (strstr(sasl->mechlist, quoted_mechname) == NULL) {
+        return false;
     }
 
     spice_debug("Validated mechname '%s'", sasl->mechname);
@@ -1013,7 +1001,7 @@ bool red_sasl_handle_auth_mechlen(RedStream *stream, AsyncReadDone read_cb, void
 {
     RedSASL *sasl = &stream->priv->sasl;
 
-    if (sasl->len < 1 || sasl->len > 100) {
+    if (sasl->len < 1 || sasl->len > SASL_MAX_MECHNAME_LEN) {
         spice_warning("Got bad client mechname len %d", sasl->len);
         return false;
     }
@@ -1106,9 +1094,9 @@ bool red_sasl_start_auth(RedStream *stream, AsyncReadDone read_cb, void *opaque)
 
     err = sasl_listmech(sasl->conn,
                         NULL, /* Don't need to set user */
-                        "", /* Prefix */
+                        ",", /* Prefix */
                         ",", /* Separator */
-                        "", /* Suffix */
+                        ",", /* Suffix */
                         &mechlist,
                         NULL,
                         NULL);
Acked-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]