Project

General

Profile

Actions
Copy link

Feature #2967

closed

mod_authn_gssapi requires delegation?

Added by lameventanas almost 5 years ago. Updated over 4 years ago.

Status:
Fixed
Priority:
Normal
Category:
mod_auth
Target version:
1.4.55
ASK QUESTIONS IN Forums:

Description

Currently mod_authn_gssapi requires delegation in order to authenticate the client.

It should be optional, this patch makes it so.

Files

Download all files
0001-mod_authn_gssapi-allow-authentication-without-delega.patch (1.63 KB) 0001-mod_authn_gssapi-allow-authentication-without-delega.patch lameventanas, 2019-07-17 03:07
0001-mod_authn_gssapi-add-store-credentials-config-option.patch (4.61 KB) 0001-mod_authn_gssapi-add-store-credentials-config-option.patch lameventanas, 2019-07-19 09:34
Actions
Copy link
 #1

Updated by gstrauss almost 5 years ago

  • Category set to mod_auth

Thank you for your patch. I'll look at it more closely soon.

To maintain compatibility with current behavior, should there be a lighttpd configuration option added to disabling the requirement of cred delegation?

When delegating creds, why did you change failure to store the creds to simply a warning rather than an error?

Please wrap (at least) the outer if () with if () { ... } rather than multi-line if statements without curly-braces, and do not remove add spurious whitespace changes (removing line)

Please also validate that your patch is produced from the latest lighttpd development (HEAD of lighttpd git master) or that you specify the version of lighttpd source code from which you produced the patch.

Actions
Copy link
 #2

Updated by lameventanas almost 5 years ago

Thanks for looking into this.

Yes, this patch is from the latest lighttpd development, but I guess I will have to make a new one once we agree on how to handle this.

I think whether credentials are delegated or not shouldn't influence the outcome of the ongoing authentication, and the current behavior is a bug.

Delegated credentials aren't used by lighttpd, but can be used by whatever CGI follows. Then, how about letting the CGI choose the course of action? (ie: continue with less functionality, deny access, show instructions on how to configure the browser).

We could add a config option to control if credentials are saved at all, and how to handle them, something like:

auth.backend.gssapi.store-credentials = yes|no[|forced]

Some possible ways of implementing this:

store-credentials Behavior Delegated Stored Outcome
A yes Patch v1 false Allow (CGI can follow up if needed)
true false Allow (CGI can follow up if needed)
B yes Apache's way false Allow (CGI can follow up if needed)
true false Server error 5xx
C yes Backward compatible-ish false Deny
true false Server error 5xx
D yes Patch v1 false Allow (CGI can follow up if needed)
true false Allow (CGI can follow up if needed)
forced Backward compatible-ish false Deny
true false Server error 5xx
E yes Apache's way false Allow (CGI can follow up if needed)
true false Server error 5xx
forced Backward compatible-ish false Deny
true false Server error 5xx

If we go this route, my preference in order is: A, B, D, E, C

Actions
Copy link
 #3

Updated by gstrauss almost 5 years ago

  • Tracker changed from Bug to Feature

I think whether credentials are delegated or not shouldn't influence the outcome of the ongoing authentication, and the current behavior is a bug.

The current module behavior is the same as when it was released. The behavior is functional.

Your opinion ("I think") does not qualify what is and is not a bug. You are going to have to get over yourself if you would like this conversation to continue.

Let me be clear: you are making a feature request to loosen a requirement in the code. The way it was coded was not an accident and is not a bug. You are making a feature request that the code be modified to support an alternate use case. It is a reasonable feature request, which is why I am entertaining it, but it is not a bug.

Actions
Copy link
 #4

Updated by gstrauss almost 5 years ago

Untested patch, but I think this what you are requesting. Is it?

--- a/src/mod_authn_gssapi.c
+++ b/src/mod_authn_gssapi.c
@@ -421,17 +421,16 @@ static handler_t mod_authn_gssapi_check_spnego(server *srv, connection *con, plu
         goto end;
     }

-    if (!(acc_flags & GSS_C_DELEG_FLAG)) {
-        log_error_write(srv, __FILE__, __LINE__, "ss", "Unable to delegate credentials for user:", token_out.value);
-        goto end;
-    }
-
     /* check the allow-rules */
     if (!http_auth_match_rules(require, token_out.value, NULL, NULL)) {
         goto end;
     }

-    ret = mod_authn_gssapi_store_gss_creds(srv, con, p, token_out.value, client_cred);
+    ret = 1; /* success */
+
+    if (acc_flags & GSS_C_DELEG_FLAG)
+        ret = mod_authn_gssapi_store_gss_creds(srv, con, p, token_out.value, client_cred);
+
     if (ret)
         http_auth_setenv(con, token_out.value, token_out.length, CONST_STR_LEN("GSSAPI"));

Actions
Copy link
 #5

Updated by lameventanas almost 5 years ago

It looks good, the only difference is that it sends a 401 status when credentials can't be stored.

Here is another one, it adds a "auth.backend.gssapi.store-credentials" option (disabled by default).
It returns a 500 status if enabled and there is an error storing the credentials.
This is how the Apache module works.

Actions
Copy link
 #6

Updated by gstrauss over 4 years ago

  • Status changed from New to Patch Pending
  • Target version changed from 1.4.x to 1.4.55

Here is another one, it adds a "auth.backend.gssapi.store-credentials" option (disabled by default).

Backwards compatibility with existing behavior must be maintained. Please re-read my first post more carefully.

Also, if storing of delegated credentials is enabled, then it is an error if credentials have not been delegated.

Actions
Copy link
 #7

Updated by gstrauss over 4 years ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100
Actions
Copy link

Also available in: Atom