Option to disable WWW-Authenticate: Negotiate header on failed attempt

[jgroffen]

Add option to not send a WWW-Authenticate: Negotiate header if negotiation was attempted but failed. Useful when only one single sign on mechanism is allowed to avoid misleading login prompts in some browsers.

I'm working with FreeIPA which recently (4.2) switched to mod_auth_gssapi. I found that I was getting a browser login popup on IE and Chrome browsers when attempting to log in to the FreeIPA admin console. This normally doesn't happen.

I tracked down the reason with Wireshark: my browser sends a kerberos ticket that I happen to have - but this is for a different realm. When this fails mod_auth_gssapi returns a 401 response with a WWW-Authenticate: Negotiate response header. The older mod_auth_kerb would not do this - if authentication was attempted but failed then no negotiate response header was sent.

While the current mod_auth_gssapi behavior is correct according to RFC 7235, some browsers react to getting another WWW-Authenticate: Negotiate response header by prompting the user for credentials and trying other negotiate mechanisms. Given that the only mechanism allowed in my case is krb5, this won't work.

I have added a GssapiNegotiateOnce option. This option can be enabled so that the negotiate response header is not sent when negotiate is attempted but fails. This option also affects the NTLM header if NTLM mechanisms are enabled.

The basic auth fallback benefits from this patch also. Browsers rightfully prefer Negotiate over Basic Auth, but IE and Chrome (and possibly other browsers) will not fallback to the Basic Auth mechanism while Negotiate is offered, even after a failed Negotiate attempt. See https://github.com/modauthgssapi/mod_auth_gssapi/pull/65#issuecomment-181856134

I have tested my scenario and this patch fixes my issues. I also updated the readme with a description of the new option and a bit of text as to why you might want to enable it.

Tests have been added to ensure GssapiNegotiateOnce behaves as expected. Also a test was added to ensure the WWW-Authenticate: Negotiate header is sent on the initial request.

[jgroffen]

I have incorporated feedback from simo5 and frenche, and rebased.

The original commit got clobbered when I rebased after incorporating feedback. That commit has some useful comments on it and can still be viewed at https://github.com/jgroffen/mod_auth_gssapi/commit/3570aa86f250d3c8ee6f9602bdfd7c93437688ef.

[iboukris]

I think this one isn't necessary since now "output.length != 0" so we will enter the the first "if" down there

[iboukris]

Hmm, did you test the new patch? I don't see how the initial www-auth header will get sent with the updated patch.

So maybe it could be simplified to something like:

diff --git a/src/mod_auth_gssapi.c b/src/mod_auth_gssapi.c
index 6d12036..a4bb579 100644
--- a/src/mod_auth_gssapi.c
+++ b/src/mod_auth_gssapi.c
@@ -672,6 +672,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)) {
@@ -763,6 +764,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->dont_reauth;
+
     for (i = 0; auth_types[i] != NULL; i++) {
         if (strcasecmp(auth_header_type, auth_types[i]) == 0) {
             auth_type = i;
@@ -955,10 +959,13 @@ 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 (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)) {
+        if (send_auth_header && 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) {

[jgroffen]

Hello frenche, you are right - if GssapiDontReauth is on then the initial negotiate header is not sent. When I tested with FreeIPA I didn't notice this - I didn't realise the freeipa command line client sends the negotiate header without being prompted. I'm incorporating your feedback now.

[jgroffen]

Your change is much better - and works great too. I've tested with FreeIPA and a more vanilla Apache configuration. I'm now adding a test for the GssapiDontReauth option to the test suite.

[jgroffen]

I added a test for the GssapiDontReauth option, and tested the tests.

I also added a SPNEGO test to ensure if no auth is sent with the request, a WWW-Authenticate: Negotiate response header is returned.

The new tests are in a separate commit, but I can squash them together if you prefer.

[simo5]

Btw, can you move all the above formatting changes in a separate PR ?

[jgroffen]

Hello Simo, sure thing. Should be done by this time tomorrow.

[jgroffen]

The separate pull request has been created. I've rebased this pull request, removing the formatting changes unrelated to this PR.

[jgroffen]

I believe the browser is trying NTLM even though we do not advertise it. Here is some testing I've done:

Configuration:
GssapiAllowedMech krb5
Basic Auth and dont reauth are not enabled.

Browser: Chrome v47

1. Request a resource
2. Response: 401 with a single WWW-Authenticate: Negotiate header
3. Request Resource with Authorization: Negotiate header - Wireshark reports this as a GSS-API / SPNEGO Negotiation
4. Response: 401 with a single WWW-Authenticate: Negotiate header
5. Browser then pops up a login prompt. I enter some details and:
6. Request Resource with Authorization: Negotiate header - Wireshark reports this as an NTLMSSP_NEGOTIATE header
7. Response: 401 with a single WWW-Authenticate: Negotiate header

I never see a WWW-Authenticate: NTLM header.

[iboukris]

As per tests I've run a while ago, when Kerberos is not available both IE and Chrome will prompt for password even when getting only Negotiate header.

Then, if you enter credentials the browser won't even bother to generate an SPNEGO token with one mechanism, but rather sends a raw NTLM token (with a Negotiate header).

As a matter of fact, even if we advertise both NTLM / Negotiate headers, some browser will still send a raw NTLM token with Negotiate header.

[jgroffen]

Thanks frenche.

In that case should we change the name of the option to something like GssapiNegotiateOnce, and change the behavior to only affect the negotiate header?

If we did this and enabled GssapiNegotiateOnce as well as both NTLM and Negotiate mechanisms then both headers would initially be sent. Then If the browser tries a Negotiate response that fails, only the WWW-Authenticate: NTLM header would be returned to the browser.

This may help browsers that send a raw NTLM token with a Negotiate header as well - it would push the browser into sending an NTLM header instead. I'm not sure about this though.

[jgroffen]

Rebased to move unrelated formatting changes to a separate PR, and refresh this PR with recent changes to mod_auth_gssapi/master.

Simo had a useful comment on the following commit that got clobbered from the rebase: https://github.com/jgroffen/mod_auth_gssapi/commit/8c346c3e758b3cc1df2783dfa73048d69086f804#commitcomment-15425412

[iboukris]

The newly suggested name sound fine to me (I was only uncomfortable with the initial name because it sounded related to SPNEGO negotiation which isn't the case).

About NTLM, I think we want to disable it along with Negotiate as otherwise browsers will prompt for credentials (when the server has ntlmssp installed).

[jgroffen]

I changed the option name to GssapiNegotiateOnce and re-ran the tests, as well as my own testing. As suggested by frenche NTLM is still disabled along with Negotiate.

[simo5]

@jgroffen sorry for taking longer than usual to deal with this PR, I've been busy and travel looms ahead. Rest assured I want to get this feature in, I just need some time to do a thorough review, sorry.

Simo.

[jgroffen]

Thanks @simo5, let me know if you need anything.

[amessina]

@jgroffen Thank you for this PR. I believe I was encountering a similar issue when logging in from a remote Windows AD domain that wasn't connected in any way to my server's Kerberos domain. When my server was using the following, I found that the remote Windows users never fell back to Basic auth, which is what I needed to happen.

GssapiBasicAuth On
GssapiAllowedMech krb5

[jgroffen]

Hello @amessina - you are welcome!

I've seen this behavior as well, if the Negotiate and Basic Auth headers are sent then browsers will attempt to Negotiate first. If the Negotiate fails then some browsers (IE and Chrome) will prompt the user for credentials and make a second Negotiate attempt with a raw NTLM header.

This second attempt cannot succeed because NTLM is not configured. This cycle repeats and stops the browser from falling all the way back to a Basic Auth attempt.

This patch adds an option to stop the server sending a Negotiate header after a failed Negotiate attempt. If Basic Auth fallback is configured, only the Basic Auth header is returned after a failed negotiate, and the browser will then fall back to Basic Auth correctly.

[amessina]

Then @jgroffen, your patch is exactly the fix I'm looking for. @simo5, if you implement this, would you be willing to test that the following works (or I can if you spin me up a F23 x86_64 RPM in Koji):

GssapiSSLonly On
GssapiBasicAuth On
GssapiAllowedMech krb5

GssapiUseS4U2Proxy On
GssapiCredStore keytab:/etc/httpd/conf/HTTP.keytab
GssapiCredStore client_keytab:/etc/httpd/conf/HTTP.keytab
# The following must be writable by the apache user
GssapiCredStore ccache:FILE:/run/httpd/clientcaches/krb5ccache_HTTP
GssapiDelegCcacheDir /run/httpd/clientcaches

GssapiUseSessions On
Session On
SessionEnv On
SessionCookieName gssapi_session path=/example;domain=example.com;httponly;secure;

My goal is to use delegated credentials to query a PostgreSQL server from a PHP application. It currently works perfectly with clients/users who are able to authenticate using Negotiate/KRB5 and I'd love to have it continue working even when the Windows (unrelated domain) users fallback to Basic auth.

[amessina]

Ok, I can confirm that @jgroffen's patches applied atop 3361a79 do allow true fallback to Basic auth even when connecting from a Windows domain which is wonderful. However, the use of delegated credentials still fails when falling back to Basic auth--it seems like the client creds aren't stored "fast enough" for the environment to use them, but that's another ticket: #69. I am attempting the following config:

GssapiSSLonly On
GssapiBasicAuth On
GssapiAllowedMech krb5

GssapiUseS4U2Proxy On
GssapiCredStore keytab:/etc/httpd/conf/HTTP.keytab
GssapiCredStore client_keytab:/etc/httpd/conf/HTTP.keytab
# The following must be writable by the apache user
GssapiCredStore ccache:FILE:/run/httpd/clientcaches/krb5ccache_HTTP
GssapiDelegCcacheDir /run/httpd/clientcaches

GssapiUseSessions On
Session On
SessionEnv On
SessionCookieName gssapi_session path=/example;domain=example.com;httponly;secure;

[jgroffen]

I have updated feedback from @simo5 and rebased. Re-ran the tests and checked behavior in my FreeIPA deploy. All looks good.

[jgroffen]

Hello @simo5, I updated the README to mention that you may want to enable GssapiNegotiateOnce if you are using basic auth fallback. I imagine this could be a gotcha that users may not expect GssapiNegotiateOnce could help with. I also noticed a tiny typo in the GssapiBasicAuth description that I fixed here instead of a separate patch, I hope that's OK.

[jgroffen]

Hello again @simo5 - after thinking about the typo overnight I thought better of including it in this patch to keep the history clean - this PR is one commit related to a single issue. I'll raise a separate PR for the typo, even if it's a single character.

[simo5]

Thank you @jgroffen
I pushed the code to master now, sorry for the long delays and your patience on getting this code in, truly appreciated.

FYI:
I fixed up the commit message to adhere to conventions (first line must not be longer than 52 chars, then empty line, then text no wider than 80 chars (preferably no more than 72).
I also fixed a bunch of stray whites paces sprinkled through the README and new code, as well as some 80 columns rule violations.

[jgroffen]

You are most welcome @simo5 - Thanks for the tidy-up and for letting me know so I can improve.