Export error environment variables.

These should help developers to show nicer error messages to users (e.g. invalid credentials, not configured libraries, etc.).

diff --git a/README_name_attr b/README_name_attr
index 1dcdec0..0955214 100644 (file)
--- a/README_name_attr
+++ b/README_name_attr
@@ -1,3 +1,5 @@
+The module provides the following option to deal with GSS API name attributes:
+
 #### GssapiNameAttributes
 
 Enables the module to source Name Attributes from the client name
@@ -18,3 +20,18 @@ and with the string "0 attributes found", if no attributes are set.
     GssapiNameAttributes RADIUS_USER_NAME urn:ietf:params:gss:radius-attribute 1
     GssapiNameAttributes EPPN urn:ietf:params:gss:federated-saml-attribute urn:oasis:names:tc:SAML:2.0:attrname-format:uri urn:oid:1.3.6.1.4.1.5923.1.1.1.6
 
+#### Feedback on authentication failure reason
+
+In addition to this, in the event of an authentication failure,
+ the module exports an environment variable called MAG_ERROR wich contains one 
+of the following values:
+* "NO_AUTH_DATA" when the client did not send any authentication data (usually because the
+ appropriate libraries are not installed on the browser).
+* "UNSUP_AUTH_TYPE" when the client sent authentication data of an invalid type.
+* "GSS_MECH_ERROR" when the GSS mechanism failed for some reason (e.g. invalid credentials). 
+
+In addition to this, whenever MAG_ERROR takes a value of "GSS_MECH_ERROR", an additional environment
+variable named GSS_ERROR_STR is sourced. This variable contains the result of
+the gss_display_status() call and may help web developers to show a more appropriate error page/string
+to the user.
+

diff --git a/gss.c b/gss.c
index 75a40bd..f29c2ad 100644 (file)
--- a/gss.c
+++ b/gss.c
@@ -356,7 +356,7 @@ static char mag_get_name_attr(request_rec *req,
 
 static apr_status_t mag_gss_name_attrs_cleanup(void *data)
 {
-    gss_conn_ctx_t *gss_ctx = (struct gss_conn_ctx_t *)data;
+    gss_conn_ctx_t *gss_ctx = (gss_conn_ctx_t *) data;
     free(gss_ctx->name_attributes);
     gss_ctx->name_attributes = NULL;
     return 0;
@@ -416,34 +416,63 @@ static void mag_set_env_name_attr(request_rec *req, gss_conn_ctx_t *gss_ctx,
     }
 }
 
-static char* mag_escape_display_value(request_rec *req, gss_buffer_desc disp_value)
+static char *mag_escape_display_value(request_rec *req,
+                                      gss_buffer_desc disp_value)
 {
-    /* This function returns a copy (in the pool) of the given gss_buffer_t where every
-     * occurrence of " has been replaced by \". This string is NULL terminated */
-    int i = 0, j = 0, n_quotes = 0;
+    /* This function returns a copy (in the pool) of the given gss_buffer_t
+     * where some characters are escaped as required by RFC4627. The string is
+     * NULL terminated */
+    char *value = disp_value.value;
     char *escaped_value = NULL;
-    char *value = (char*) disp_value.value;
-
-    // count number of quotes in the input string
-    for (i = 0, j = 0; i < disp_value.length; i++)
-        if (value[i] == '"')
-            n_quotes++;
-
-    // if there are no quotes, just return a copy of the string
-    if (n_quotes == 0)
-        return apr_pstrndup(req->pool, value, disp_value.length);
-
-    // gss_buffer_t are not \0 terminated, but our result will be
-    escaped_value = apr_palloc(req->pool, disp_value.length + n_quotes + 1);
-    for (i = 0,j = 0; i < disp_value.length; i++, j++) {
-        if (value[i] == '"') {
-            escaped_value[j] = '\\';
-            j++;
+    char *p = NULL;
+    size_t i = 0;
+
+    /* gss_buffer_t are not \0 terminated, but our result will be. Hence,
+     * escaped length will be original length * 6 + 1 in the worst case */
+    p = escaped_value = apr_palloc(req->pool, disp_value.length * 6 + 1);
+    for (i = 0; i < disp_value.length; i++) {
+        switch (value[i]) {
+        case '"':
+            memcpy(p, "\\\"", 2);
+            p += 2;
+            break;
+        case '\\':
+            memcpy(p, "\\\\", 2);
+            p += 2;
+            break;
+        case '\b':
+            memcpy(p, "\\b", 2);
+            p += 2;
+            break;
+        case '\t':
+            memcpy(p, "\\t", 2);
+            p += 2;
+            break;
+        case '\r':
+            memcpy(p, "\\r", 2);
+            p += 2;
+            break;
+        case '\f':
+            memcpy(p, "\\f", 2);
+            p += 2;
+            break;
+        case '\n':
+            memcpy(p, "\\n", 2);
+            p += 2;
+            break;
+        default:
+            if (value[i] <= 0x1F) {
+                apr_snprintf(p, 7, "\\u%04d", (int)value[i]);
+                p += 6;
+            } else {
+                *p = value[i];
+                p += 1;
+            }
+            break;
         }
-        escaped_value[j] = value[i];
     }
-    // make the string NULL terminated
-    escaped_value[j] = '\0';
+    /* make the string NULL terminated */
+    *p = '\0';
     return escaped_value;
 }
 

diff --git a/mod_auth_gssapi.c b/mod_auth_gssapi.c
index d533594..dbd0da2 100644 (file)
--- a/mod_auth_gssapi.c
+++ b/mod_auth_gssapi.c
@@ -181,9 +181,11 @@ gss_authenticate(request_rec *r, gss_auth_config *conf, gss_conn_ctx ctx,
   }
 
   if (GSS_ERROR(major_status)) {
-     gss_log(APLOG_MARK, APLOG_ERR, 0, r,
-            "%s", get_gss_error(r, major_status, minor_status,
-                                "Failed to establish authentication"));
+     // at least this error should be populated, to provider further information
+     // to the user (maybe)
+     char *error = get_gss_error(r, major_status, minor_status, "Failed to establish authentication");
+     apr_table_set(r->subprocess_env, "GSS_ERROR_STR", error);
+     gss_log(APLOG_MARK, APLOG_ERR, 0, r, error);
 #if 0
      /* Don't offer the Negotiate method again if call to GSS layer failed */
      /* XXX ... which means we don't return the "error" output */
@@ -257,6 +259,10 @@ static int mag_post_config(apr_pool_t *cfgpool, apr_pool_t *log,
     return OK;
 }
 
+#define MAG_ERROR_NO_AUTH_DATA    "NO_AUTH_DATA"
+#define MAG_ERROR_UNSUP_AUTH_TYPE "UNSUP_AUTH_TYPE"
+#define MAG_ERROR_GSS_MECH        "GSS_MECH_ERROR"
+
 static int
 gss_authenticate_user(request_rec *r)
 {
@@ -290,6 +296,7 @@ gss_authenticate_user(request_rec *r)
         gss_log(APLOG_MARK, APLOG_DEBUG, 0, r,
                "Client hasn't sent any authentication data, giving up");
         set_http_headers(r, conf, "\0");
+        apr_table_set(r->subprocess_env, "MAG_ERROR", MAG_ERROR_NO_AUTH_DATA);
         return HTTP_UNAUTHORIZED;
     }
 
@@ -299,6 +306,7 @@ gss_authenticate_user(request_rec *r)
                "Unsupported authentication type (%s) requested by client",
                (auth_type) ? auth_type : "(NULL)");
         set_http_headers(r, conf, "\0");
+        apr_table_set(r->subprocess_env, "MAG_ERROR", MAG_ERROR_UNSUP_AUTH_TYPE);
         return HTTP_UNAUTHORIZED;
     }
 
@@ -333,8 +341,9 @@ gss_authenticate_user(request_rec *r)
     if (ret == OK) {
        r->user = apr_pstrdup(r->pool, conn_ctx->user);
        r->ap_auth_type = "Negotiate";
+    } else {
+      apr_table_set(r->subprocess_env, "MAG_ERROR", MAG_ERROR_GSS_MECH);
     }
-
     /* debug LOG ??? */
 
     return ret;

diff --git a/mod_auth_gssweb.c b/mod_auth_gssweb.c
index 491e755..5b1a0d5 100644 (file)
--- a/mod_auth_gssweb.c
+++ b/mod_auth_gssweb.c
@@ -520,8 +520,10 @@ gssweb_authenticate_user(request_rec *r)
   r->ap_auth_type = "GSSWeb";
 
   /* TODO: Make it a single call! */
-  mag_get_name_attributes(r, conf, client_name, conn_ctx);
-  mag_set_req_data(r, conf, conn_ctx);
+  if (conf->name_attributes) {
+    mag_get_name_attributes(r, conf, client_name, conn_ctx);
+    mag_set_req_data(r, conf, conn_ctx);
+  }
 
   ret = OK;