[exim-cvs] GSASL: provide $autnN for scram option expansions

Startseite
Nachricht löschen
Nachricht beantworten
Autor: Exim Git Commits Mailing List
Datum:  
To: exim-cvs
Betreff: [exim-cvs] GSASL: provide $autnN for scram option expansions
Gitweb: https://git.exim.org/exim.git/commitdiff/2b615f22d0ce78ba28a6d758d6a2a5c8cb33e10a
Commit:     2b615f22d0ce78ba28a6d758d6a2a5c8cb33e10a
Parent:     0299eb6ae2f923bd2a4ba8f82fc06e615b99c177
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Wed Jan 1 15:19:52 2020 +0000
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Wed Jan 1 15:27:41 2020 +0000


    GSASL: provide $autnN for scram option expansions
---
 doc/doc-docbook/spec.xfpt  |  22 +++++++--
 doc/doc-txt/ChangeLog      |   3 ++
 src/src/auths/gsasl_exim.c | 116 ++++++++++++++++++++++-----------------------
 test/confs/3820            |   4 +-
 4 files changed, 78 insertions(+), 67 deletions(-)


diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 560b720..4d02bdc 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -27544,16 +27544,28 @@ This specifies the SASL realm that the server claims to be in.
Some mechanisms will use this data.


-.option server_scram_iter gsasl string&!! unset
+.option server_scram_iter gsasl string&!! 4096
This option provides data for the SCRAM family of mechanisms.
-&$auth1$& is not available at evaluation time.
-(This may change, as we receive feedback on use)
+.new
+The &$auth1$&, &$auth2$& and &$auth3$& variables are available for expansion.
+
+The result of expansion should be a decimal number,
+and represents both a lower-bound on the security, and
+a compute cost factor imposed on the client
+(if it does not cache results, or the server changes
+either the iteration count or the salt).
+A minimum value of 4096 is required by the standards
+for all current CRAM mechanism variants.
+.wen


.option server_scram_salt gsasl string&!! unset
This option provides data for the SCRAM family of mechanisms.
-&$auth1$& is not available at evaluation time.
-(This may change, as we receive feedback on use)
+.new
+The &$auth1$&, &$auth2$& and &$auth3$& variables are available for expansion.
+If unset or empty after expansion the library will provides a value for the
+protocol conversation.
+.wen


 .option server_service gsasl string &`smtp`&
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 491ff52..e1e1e3b 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -74,6 +74,9 @@ JH/17 Logging: when the deliver_time selector ise set, include the DT= field
       on delivery deferred (==) and failed (**) lines (if a delivery was
       attemtped).  Previously it was only on completion (=>) lines.


+JH/18 Authentication: the gsasl driver not provides the $authN variables in time
+      for the expansion of the server_scram_iter and server_scram_salt options.
+


Exim version 4.93
-----------------
diff --git a/src/src/auths/gsasl_exim.c b/src/src/auths/gsasl_exim.c
index d1640f1..80ae354 100644
--- a/src/src/auths/gsasl_exim.c
+++ b/src/src/auths/gsasl_exim.c
@@ -500,6 +500,36 @@ switch (exim_rc)
return GSASL_AUTHENTICATION_ERROR;
}

+
+static void
+set_exim_authvar_from_prop(Gsasl_session * sctx, Gsasl_property prop)
+{
+uschar * propval = US gsasl_property_fast(sctx, prop);
+int i = expand_nmax, j = i + 1;
+propval = propval ? string_copy(propval) : US"";
+auth_vars[i] = expand_nstring[j] = propval;
+expand_nlength[j] = Ustrlen(propval);
+expand_nmax = j;
+}
+
+static void
+set_exim_authvars_from_a_az_r_props(Gsasl_session * sctx)
+{
+if (expand_nmax > 0 ) return;
+
+/* Asking for GSASL_AUTHZID calls back into us if we use
+gsasl_property_get(), thus the use of gsasl_property_fast().
+Do we really want to hardcode limits per mechanism?  What happens when
+a new mechanism is added to the library.  It *shouldn't* result in us
+needing to add more glue, since avoiding that is a large part of the
+point of SASL. */
+
+set_exim_authvar_from_prop(sctx, GSASL_AUTHID);
+set_exim_authvar_from_prop(sctx, GSASL_AUTHZID);
+set_exim_authvar_from_prop(sctx, GSASL_REALM);
+}
+
+
 static int
 server_callback(Gsasl *ctx, Gsasl_session *sctx, Gsasl_property prop,
   auth_instance *ablock)
@@ -522,15 +552,9 @@ switch (prop)
   case GSASL_VALIDATE_SIMPLE:
     HDEBUG(D_auth) debug_printf(" VALIDATE_SIMPLE\n");
     /* GSASL_AUTHID, GSASL_AUTHZID, and GSASL_PASSWORD */
-    propval = US gsasl_property_fast(sctx, GSASL_AUTHID);
-    auth_vars[0] = expand_nstring[1] = propval ? string_copy(propval) : US"";
-    propval = US gsasl_property_fast(sctx, GSASL_AUTHZID);
-    auth_vars[1] = expand_nstring[2] = propval ? string_copy(propval) : US"";
-    propval = US gsasl_property_fast(sctx, GSASL_PASSWORD);
-    auth_vars[2] = expand_nstring[3] = propval ? string_copy(propval) : US"";
-    expand_nmax = 3;
-    for (int i = 1; i <= 3; ++i)
-      expand_nlength[i] = Ustrlen(expand_nstring[i]);
+    set_exim_authvar_from_prop(sctx, GSASL_AUTHID);
+    set_exim_authvar_from_prop(sctx, GSASL_AUTHZID);
+    set_exim_authvar_from_prop(sctx, GSASL_PASSWORD);


     cbrc = condition_check(ablock, US"server_condition", ablock->server_condition);
     checked_server_condition = TRUE;
@@ -544,12 +568,7 @@ switch (prop)
       cbrc = GSASL_AUTHENTICATION_ERROR;
       break;
       }
-    propval = US gsasl_property_fast(sctx, GSASL_AUTHZID);
-
-    /* We always set $auth1, even if only to empty string. */
-    auth_vars[0] = expand_nstring[1] = propval ? string_copy(propval) : US"";
-    expand_nlength[1] = Ustrlen(expand_nstring[1]);
-    expand_nmax = 1;
+    set_exim_authvar_from_prop(sctx, GSASL_AUTHZID);


     cbrc = condition_check(ablock,
     US"server_condition (EXTERNAL)", ablock->server_condition);
@@ -564,13 +583,7 @@ switch (prop)
       cbrc = GSASL_AUTHENTICATION_ERROR;
       break;
       }
-    propval = US gsasl_property_fast(sctx, GSASL_ANONYMOUS_TOKEN);
-
-    /* We always set $auth1, even if only to empty string. */
-
-    auth_vars[0] = expand_nstring[1] = propval ? string_copy(propval) : US"";
-    expand_nlength[1] = Ustrlen(expand_nstring[1]);
-    expand_nmax = 1;
+    set_exim_authvar_from_prop(sctx, GSASL_ANONYMOUS_TOKEN);


     cbrc = condition_check(ablock,
     US"server_condition (ANONYMOUS)", ablock->server_condition);
@@ -588,13 +601,8 @@ switch (prop)
     to the first release of Exim with this authenticator, they've been
     switched to match the ordering of GSASL_VALIDATE_SIMPLE. */


-    propval = US gsasl_property_fast(sctx, GSASL_GSSAPI_DISPLAY_NAME);
-    auth_vars[0] = expand_nstring[1] = propval ? string_copy(propval) : US"";
-    propval = US gsasl_property_fast(sctx, GSASL_AUTHZID);
-    auth_vars[1] = expand_nstring[2] = propval ? string_copy(propval) : US"";
-    expand_nmax = 2;
-    for (int i = 1; i <= 2; ++i)
-      expand_nlength[i] = Ustrlen(expand_nstring[i]);
+    set_exim_authvar_from_prop(sctx, GSASL_GSSAPI_DISPLAY_NAME);
+    set_exim_authvar_from_prop(sctx, GSASL_AUTHZID);


     /* In this one case, it perhaps makes sense to default back open?
     But for consistency, let's just mandate server_condition here too. */
@@ -608,66 +616,54 @@ switch (prop)
     HDEBUG(D_auth) debug_printf(" SCRAM_ITER\n");
     if (ob->server_scram_iter)
       {
+      set_exim_authvars_from_a_az_r_props(sctx);
       tmps = CS expand_string(ob->server_scram_iter);
+      HDEBUG(D_auth) debug_printf("  '%s'\n", tmps);
       gsasl_property_set(sctx, GSASL_SCRAM_ITER, tmps);
       cbrc = GSASL_OK;
       }
+    else
+      HDEBUG(D_auth) debug_printf("  option not set\n");
     break;


   case GSASL_SCRAM_SALT:
     HDEBUG(D_auth) debug_printf(" SCRAM_SALT\n");
-    if (ob->server_scram_iter)
+    if (ob->server_scram_salt)
       {
+      set_exim_authvars_from_a_az_r_props(sctx);
       tmps = CS expand_string(ob->server_scram_salt);
-      gsasl_property_set(sctx, GSASL_SCRAM_SALT, tmps);
+      HDEBUG(D_auth) debug_printf("  '%s'\n", tmps);
+      if (*tmps)
+    gsasl_property_set(sctx, GSASL_SCRAM_SALT, tmps);
       cbrc = GSASL_OK;
       }
+    else
+      HDEBUG(D_auth) debug_printf("  option not set\n");
     break;


   case GSASL_PASSWORD:
     HDEBUG(D_auth) debug_printf(" PASSWORD\n");
-    /* DIGEST-MD5: GSASL_AUTHID, GSASL_AUTHZID and GSASL_REALM
+    /* SCRAM-SHA-1: GSASL_AUTHID, GSASL_AUTHZID and GSASL_REALM
+       DIGEST-MD5: GSASL_AUTHID, GSASL_AUTHZID and GSASL_REALM
        CRAM-MD5: GSASL_AUTHID
        PLAIN: GSASL_AUTHID and GSASL_AUTHZID
        LOGIN: GSASL_AUTHID
      */
-    if (ob->server_scram_iter)
-      {
-      tmps = CS expand_string(ob->server_scram_iter);
-      gsasl_property_set(sctx, GSASL_SCRAM_ITER, tmps);
-      }
-    if (ob->server_scram_salt)
-      {
-      tmps = CS expand_string(ob->server_scram_salt);
-      gsasl_property_set(sctx, GSASL_SCRAM_SALT, tmps);
-      }
-
-    /* Asking for GSASL_AUTHZID calls back into us if we use
-    gsasl_property_get(), thus the use of gsasl_property_fast().
-    Do we really want to hardcode limits per mechanism?  What happens when
-    a new mechanism is added to the library.  It *shouldn't* result in us
-    needing to add more glue, since avoiding that is a large part of the
-    point of SASL. */
-
-    propval = US gsasl_property_fast(sctx, GSASL_AUTHID);
-    auth_vars[0] = expand_nstring[1] = propval ? string_copy(propval) : US"";
-    propval = US gsasl_property_fast(sctx, GSASL_AUTHZID);
-    auth_vars[1] = expand_nstring[2] = propval ? string_copy(propval) : US"";
-    propval = US gsasl_property_fast(sctx, GSASL_REALM);
-    auth_vars[2] = expand_nstring[3] = propval ? string_copy(propval) : US"";
-    expand_nmax = 3;
-    for (int i = 1; i <= 3; ++i)
-      expand_nlength[i] = Ustrlen(expand_nstring[i]);
+    set_exim_authvars_from_a_az_r_props(sctx);


     if (!ob->server_password)
+      {
+      HDEBUG(D_auth) debug_printf("option not set\n");
       break;
+      }
     if (!(tmps = CS expand_string(ob->server_password)))
       {
-      sasl_error_should_defer = f.expand_string_forcedfail ? FALSE : TRUE;
+      sasl_error_should_defer = !f.expand_string_forcedfail;
       HDEBUG(D_auth) debug_printf("server_password expansion failed, so "
       "can't tell GNU SASL library the password for %s\n", auth_vars[0]);
       return GSASL_AUTHENTICATION_ERROR;
       }
+    HDEBUG(D_auth) debug_printf("  set\n");
     gsasl_property_set(sctx, GSASL_PASSWORD, tmps);


     /* This is inadequate; don't think Exim's store stacks are geared
diff --git a/test/confs/3820 b/test/confs/3820
index b60e467..c80d4d4 100644
--- a/test/confs/3820
+++ b/test/confs/3820
@@ -77,8 +77,8 @@ sasl3:
   # a GSASL_SCRAM_SALTED_PASSWORD - but that is only documented for client mode.


   # unclear if the salt is given in binary or base64 to the library
-  server_scram_salt =    QSXCR+Q6sek8bf92
-  server_password =    pencil
+  server_scram_salt =    ${if eq {$auth1}{ph10} {QSXCR+Q6sek8bf92}}
+  server_password =    ${if eq {$auth1}{ph10} {pencil}{unset_password}}
   server_condition =    true
   server_set_id =    $auth1