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 <simo@redhat.com>

Close #65

7 files changed, 152 insertions, 5 deletions

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
 </Location>
 
+<Location /spnego_negotiate_once>
+  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
+</Location>
+
 <Location /basic_auth_krb5>
   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')
+