From 76faf24834c2b060c1c813eec489c1b2d978a4ba Mon Sep 17 00:00:00 2001 From: Alejandro Perez Date: Fri, 24 Mar 2017 07:37:22 +0000 Subject: [PATCH] Export error environment variables. These should help developers to show nicer error messages to users (e.g. invalid credentials, not configured libraries, etc.). --- README_name_attr | 17 ++++++++++++ gss.c | 79 +++++++++++++++++++++++++++++++++++++------------------ mod_auth_gssapi.c | 17 +++++++++--- mod_auth_gssweb.c | 6 +++-- 4 files changed, 88 insertions(+), 31 deletions(-) diff --git a/README_name_attr b/README_name_attr index 1dcdec0..0955214 100644 --- 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 --- 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 --- 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 --- 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; -- 2.1.4