From f9cc36700c95a88ff7d7489167094556ac0e75cc Mon Sep 17 00:00:00 2001 From: James Groffen Date: Fri, 8 Jan 2016 17:01:50 +1030 Subject: [PATCH] Add option to not send a Negotiate headers If negotiation was attempted but failed do not send a new Negotiate header. Useful when only one single sign on mechanism is allowed and to avoid misleading login prompts in some browsers. Added a test of the GssapiDontReauth option to the test suite. Also added SPNEGO no auth test. [SS: reworded and fixed commit subject/comment] [SS: fixed whitespace errors and 80 column wrappings] Reviewed-by: Simo Sorce Close #65 --- README | 26 ++++++++++++++++++++++++++ src/mod_auth_gssapi.c | 27 ++++++++++++++++++++++----- src/mod_auth_gssapi.h | 1 + tests/httpd.conf | 16 ++++++++++++++++ tests/magtests.py | 29 +++++++++++++++++++++++++++++ tests/t_spnego_negotiate_once.py | 37 +++++++++++++++++++++++++++++++++++++ tests/t_spnego_no_auth.py | 21 +++++++++++++++++++++ 7 files changed, 152 insertions(+), 5 deletions(-) create mode 100755 tests/t_spnego_negotiate_once.py create mode 100755 tests/t_spnego_no_auth.py diff --git a/README b/README index 72135cb..65ce17b 100644 --- a/README +++ b/README @@ -264,3 +264,29 @@ underscores for environment variable names. #### Example GssapiNameAttributes json GssapiNameAttributes RADIUS_NAME urn:ietf:params:gss:radius-attribute_1 + + +### GssapiNegotiateOnce + +When this option is enabled the Negotiate header will not be resent if +Negotiation has already been attempted but failed. + +Normally when a client fails to use Negotiate authentication, a HTTP 401 +response is returned with a WWW-Authenticate: Negotiate header, implying that +the client can retry to use Negotiate with different credentials or a +different mechanism. + +Consider enabling GssapiNegotiateOnce when only one single sign on mechanism +is allowed, or when GssapiBasicAuth is enabled. + +**NOTE:** if the initial Negotiate attempt fails, some browsers will fallback +to other Negotiate mechanisms, prompting the user for login credentials and +reattempting negotiation. This situation can mislead users - for example if +krb5 authentication failed and no other mechanisms are allowed, a user could +be prompted for login information even though any login information provided +cannot succeed. When this occurs, some browsers will not fall back to a Basic +Auth mechanism. Enable GssapiNegotiateOnce to avoid this situation. + +- **Enable with:** GssapiNegotiateOnce On +- **Default:** GssapiNegotiateOnce Off + diff --git a/src/mod_auth_gssapi.c b/src/mod_auth_gssapi.c index 088fb88..dd4e6bc 100644 --- a/src/mod_auth_gssapi.c +++ b/src/mod_auth_gssapi.c @@ -674,6 +674,7 @@ static int mag_auth(request_rec *req) gss_buffer_desc lname = GSS_C_EMPTY_BUFFER; struct mag_conn *mc = NULL; int i; + bool send_auth_header = true; type = ap_auth_type(req); if ((type == NULL) || (strcasecmp(type, "GSSAPI") != 0)) { @@ -765,6 +766,9 @@ static int mag_auth(request_rec *req) auth_header_type = ap_getword_white(req->pool, &auth_header); if (!auth_header_type) goto done; + /* We got auth header, sending auth header would mean re-auth */ + send_auth_header = !cfg->negotiate_once; + for (i = 0; auth_types[i] != NULL; i++) { if (strcasecmp(auth_header_type, auth_types[i]) == 0) { auth_type = i; @@ -957,11 +961,14 @@ done: apr_table_add(req->err_headers_out, req_cfg->rep_proto, reply); } } else if (ret == HTTP_UNAUTHORIZED) { - apr_table_add(req->err_headers_out, req_cfg->rep_proto, "Negotiate"); - - if (is_mech_allowed(desired_mechs, gss_mech_ntlmssp, - cfg->gss_conn_ctx)) { - apr_table_add(req->err_headers_out, req_cfg->rep_proto, "NTLM"); + if (send_auth_header) { + apr_table_add(req->err_headers_out, + req_cfg->rep_proto, "Negotiate"); + if (is_mech_allowed(desired_mechs, gss_mech_ntlmssp, + cfg->gss_conn_ctx)) { + apr_table_add(req->err_headers_out, req_cfg->rep_proto, + "NTLM"); + } } if (cfg->use_basic_auth) { apr_table_add(req->err_headers_out, req_cfg->rep_proto, @@ -1229,6 +1236,14 @@ static const char *mag_allow_mech(cmd_parms *parms, void *mconfig, return NULL; } +static const char *mag_negotiate_once(cmd_parms *parms, void *mconfig, int on) +{ + struct mag_config *cfg = (struct mag_config *)mconfig; + + cfg->negotiate_once = on ? true : false; + return NULL; +} + #define GSS_NAME_ATTR_USERDATA "GSS Name Attributes Userdata" static apr_status_t mag_name_attrs_cleanup(void *data) @@ -1360,6 +1375,8 @@ static const command_rec mag_commands[] = { #endif AP_INIT_ITERATE("GssapiAllowedMech", mag_allow_mech, NULL, OR_AUTHCFG, "Allowed Mechanisms"), + AP_INIT_FLAG("GssapiNegotiateOnce", mag_negotiate_once, NULL, OR_AUTHCFG, + "Don't resend negotiate header on negotiate failure"), AP_INIT_RAW_ARGS("GssapiNameAttributes", mag_name_attrs, NULL, OR_AUTHCFG, "Name Attributes to be exported as environ variables"), { NULL } diff --git a/src/mod_auth_gssapi.h b/src/mod_auth_gssapi.h index 4e9fdf3..ea563ec 100644 --- a/src/mod_auth_gssapi.h +++ b/src/mod_auth_gssapi.h @@ -74,6 +74,7 @@ struct mag_config { bool use_basic_auth; gss_OID_set_desc *allowed_mechs; gss_OID_set_desc *basic_mechs; + bool negotiate_once; struct mag_name_attributes *name_attributes; }; diff --git a/tests/httpd.conf b/tests/httpd.conf index 1e249ec..f10a7ca 100644 --- a/tests/httpd.conf +++ b/tests/httpd.conf @@ -137,6 +137,22 @@ CoreDumpDirectory /tmp Require valid-user + + AuthType GSSAPI + AuthName "Login Negotiate Once" + GssapiSSLonly Off + GssapiUseSessions On + Session On + SessionCookieName gssapi_session path=/spnego_negotiate_once;httponly + GssapiCredStore ccache:${HTTPROOT}/tmp/httpd_krb5_ccache + GssapiCredStore client_keytab:${HTTPROOT}/http.keytab + GssapiCredStore keytab:${HTTPROOT}/http.keytab + GssapiBasicAuth Off + GssapiAllowedMech krb5 + GssapiNegotiateOnce On + Require valid-user + + Options +Includes AddOutputFilter INCLUDES .html diff --git a/tests/magtests.py b/tests/magtests.py index 8075197..5abede0 100755 --- a/tests/magtests.py +++ b/tests/magtests.py @@ -283,6 +283,34 @@ def test_spnego_auth(testdir, testenv, testlog): else: sys.stderr.write('SPNEGO Proxy Auth: SUCCESS\n') + with (open(testlog, 'a')) as logfile: + spnego = subprocess.Popen(["tests/t_spnego_no_auth.py"], + stdout=logfile, stderr=logfile, + env=testenv, preexec_fn=os.setsid) + spnego.wait() + if spnego.returncode != 0: + sys.stderr.write('SPNEGO No Auth: FAILED\n') + else: + sys.stderr.write('SPNEGO No Auth: SUCCESS\n') + + +def test_spnego_negotiate_once(testdir, testenv, testlog): + + spnego_negotiate_once_dir = os.path.join(testdir, 'httpd', 'html', + 'spnego_negotiate_once') + os.mkdir(spnego_negotiate_once_dir) + shutil.copy('tests/index.html', spnego_negotiate_once_dir) + + with (open(testlog, 'a')) as logfile: + spnego = subprocess.Popen(["tests/t_spnego_negotiate_once.py"], + stdout=logfile, stderr=logfile, + env=testenv, preexec_fn=os.setsid) + spnego.wait() + if spnego.returncode != 0: + sys.stderr.write('SPNEGO Negotiate Once: FAILED\n') + else: + sys.stderr.write('SPNEGO Negotiate Once: SUCCESS\n') + def test_basic_auth_krb5(testdir, testenv, testlog): @@ -358,6 +386,7 @@ if __name__ == '__main__': test_spnego_auth(testdir, testenv, testlog) + test_spnego_negotiate_once(testdir, testenv, testlog) testenv = {'MAG_USER_NAME': USR_NAME, 'MAG_USER_PASSWORD': USR_PWD, diff --git a/tests/t_spnego_negotiate_once.py b/tests/t_spnego_negotiate_once.py new file mode 100755 index 0000000..7c7179a --- /dev/null +++ b/tests/t_spnego_negotiate_once.py @@ -0,0 +1,37 @@ +#!/usr/bin/python +# Copyright (C) 2015 - mod_auth_gssapi contributors, see COPYING for license. + +import os +import requests +from requests_kerberos import HTTPKerberosAuth, OPTIONAL + + +if __name__ == '__main__': + sess = requests.Session() + url = 'http://%s/spnego_negotiate_once/' % ( + os.environ['NSS_WRAPPER_HOSTNAME']) + + # ensure a 401 with the appropriate WWW-Authenticate header is returned + # when no auth is provided + r = sess.get(url) + if r.status_code != 401: + raise ValueError('Spnego Negotiate Once failed - 401 expected') + if not (r.headers.get("WWW-Authenticate") and + r.headers.get("WWW-Authenticate").startswith("Negotiate")): + raise ValueError('Spnego Negotiate Once failed - WWW-Authenticate ' + 'Negotiate header missing') + + # test sending a bad Authorization header with GssapiNegotiateOnce enabled + r = sess.get(url, headers={"Authorization": "Negotiate badvalue"}) + if r.status_code != 401: + raise ValueError('Spnego Negotiate Once failed - 401 expected') + if r.headers.get("WWW-Authenticate"): + raise ValueError('Spnego Negotiate Once failed - WWW-Authenticate ' + 'Negotiate present but GssapiNegotiateOnce is ' + 'enabled') + + # ensure a 200 is returned when valid auth is provided + r = sess.get(url, auth=HTTPKerberosAuth()) + if r.status_code != 200: + raise ValueError('Spnego Negotiate Once failed') + diff --git a/tests/t_spnego_no_auth.py b/tests/t_spnego_no_auth.py new file mode 100755 index 0000000..34a6481 --- /dev/null +++ b/tests/t_spnego_no_auth.py @@ -0,0 +1,21 @@ +#!/usr/bin/python +# Copyright (C) 2015 - mod_auth_gssapi contributors, see COPYING for license. + +import os +import requests +from requests_kerberos import HTTPKerberosAuth, OPTIONAL + + +if __name__ == '__main__': + sess = requests.Session() + url = 'http://%s/spnego/' % os.environ['NSS_WRAPPER_HOSTNAME'] + + r = sess.get(url) + if r.status_code != 401: + raise ValueError('Spnego failed - 401 expected') + + if not (r.headers.get("WWW-Authenticate") and + r.headers.get("WWW-Authenticate").startswith("Negotiate")): + raise ValueError('Spnego failed - WWW-Authenticate Negotiate header ' + 'missing') + -- 2.1.4