pythongssapi / requests-gssapi

An authentication handler for using GSSAPI with Python Requests. Drop-in replacement for old requests-kerberos.
Other
32 stars 21 forks source link

Unclear/incorrect understanding of mutual auth and context continuation #39

Open michael-o opened 3 years ago

michael-o commented 3 years ago

I am quite confused by the handling of mutual authentication and context continuation/completion. I don't take this as a solid argument because it does not mean that this module should not implement it correctly (regardless of TLS). mod_auth_gssapi perfectly works. My SpnegoAuthenticator fully respects it. I would expect this module to solely rely on the context continuation flag instead of the mutual flag to complete the context. I don't see any checks for continuation. In fact, even if mutual is disabled the server still sends a small token: Modified curl:

$ git diff
diff --git a/lib/curl_gssapi.c b/lib/curl_gssapi.c
index 5810dad14..1b9be62f3 100644
--- a/lib/curl_gssapi.c
+++ b/lib/curl_gssapi.c
@@ -51,9 +51,6 @@ OM_uint32 Curl_gss_init_sec_context(
 {
   OM_uint32 req_flags = GSS_C_REPLAY_FLAG;

-  if(mutual_auth)
-    req_flags |= GSS_C_MUTUAL_FLAG;
-
   if(data->set.gssapi_delegation & CURLGSSAPI_DELEGATION_POLICY_FLAG) {
 #ifdef GSS_C_DELEG_POLICY_FLAG
     req_flags |= GSS_C_DELEG_POLICY_FLAG;

Against mod_auth_gssapi:

$ LD_LIBRARY_PATH=/tmp/curl/lib  /tmp/curl/bin/curl   -X HEAD --verbose  https://deblndw011x.ad001.siemens.net/repos/websvn/ -k --negotiate -u :
* Server auth using Negotiate with user ''
> HEAD /repos/websvn/ HTTP/1.1
> Host: deblndw011x.ad001.siemens.net
> Authorization: Negotiate YIIMjQYGKwYBBQUCoIIMgTCCDH2gDTALBgkqh...
> User-Agent: curl/7.79.0-DEV
> Accept: */*
>
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Thu, 12 Aug 2021 09:28:54 GMT
< Server: Apache
* Negotiate: noauthpersist -> 0, header part: true
< Persistent-Auth: true
< WWW-Authenticate: Negotiate oRQwEqADCgEAoQsGCSqGSIb3EgECAg==
< X-Frame-Options: SAMEORIGIN
< X-Powered-By: PHP/7.4.21
< Content-Language: de
< Content-Type: text/html; charset=UTF-8
* no chunk, no close, no size. Assume close to signal end
<

same with vanilla curl:

$ curl   -X HEAD --verbose  https://deblndw011x.ad001.siemens.net/repos/websvn/ -k --negotiate -u :
* Server auth using Negotiate with user ''
> HEAD /repos/websvn/ HTTP/1.1
> Host: deblndw011x.ad001.siemens.net
> Authorization: Negotiate YIIMjQYGKwYBBQUCoIIM...
> User-Agent: curl/7.78.0
> Accept: */*
>
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Date: Thu, 12 Aug 2021 09:30:19 GMT
< Server: Apache
* Negotiate: noauthpersist -> 0, header part: true
< Persistent-Auth: true
< WWW-Authenticate: Negotiate oYG3MIG0oAMKAQChCwYJKoZIhvcSAQICooGfBIGcYIGZBgkqhkiG9xIBAgICAG+BiTCBhqADAgEFoQMCAQ+iejB4oAMCARKicQRvvNAqhdgnvC0LXYN19pJxezVYDUN173D2KCPxe6mAIGDQdBFkh+4I9DNSyMlQ1UIXDHMUPu9VK931/lOSLpusjdu/mS42RWE95kp5uPxWhaQT6UmS1pTNLIB+rf78M3CE2oovKAD0TQyEb+3xrNji
< X-Frame-Options: SAMEORIGIN
< X-Powered-By: PHP/7.4.21
< Content-Language: de
< Content-Type: text/html; charset=UTF-8
* no chunk, no close, no size. Assume close to signal end
<

I am willing to provide a PR which uses this property, ironically written with your participation.

Checked also the source code of gss-client/gss-server they both use the continuation flag to complete a context based on RFC 7546.

michael-o commented 2 years ago

@jborean93 Can you please reopen this one. I'd like to discuss a possible PR for this.

jborean93 commented 2 years ago

Opened, keep in mind this is another "feature" that was migrated as is from requests-kerberos where mutual auth did have a valid use case (the context could have been used to wrap/unwrap data based on the security context). In fact it looks like frozencemetery changed the default to DISABLED https://github.com/pythongssapi/requests-gssapi/commit/498da2e55f98847c9eeb698f50ac00a2e252cacd. IIRC this is because mutual auth over HTTP is mostly useless as the messages aren't being wrapped or signed by the security context and HTTPS offers it's own server authentication + encryption.

Happy to hear your thoughts on the matter but from what I can gather there is little benefit to re-enabling it.

michael-o commented 2 years ago

Thanks, I will get back to this next week.

michael-o commented 2 years ago

So here is my understanding based von RFC 7546 which I don't consider being implemented properly:

C: Send request S: Respond with 401, Negotiate C: Create security context, set flags, generate token; consume 401 response body silently; resend request with token; evaluate whether the context needs to be continued and persist context S: Evaluate token and check whether context is complete and whether output token needs to be send C: Status is 401, authentication failed; dispose context, handle off error/status to user; status is non-401 check context continuation and get token from server response; if no token, log a warning, dispose context and handle off request to user, token validated, context complete; log debug success message (for debugging purposes); if context is not complete, generate new token; consume response, resend request with new token; continue loop until done

Remarks:

Note that context flags aren't requirements, they are indications/hints and the caller must verify whether the mech and peer have this enabled otherwise you need to fail. Cyrus SASL code is a good read on that.

Let me know what you think. This can drastically simplify the code. I'd like to take a shot on the changes if we can come to an agreement.

BTW: With the never completed context I was able to find a bug in my JGSS-based setup and only noticed it with the SSPI module, not this one or curl: https://github.com/curl/curl/issues/5235

jborean93 commented 2 years ago

Just having a read over what you've posted and I'm not against having the code process the token on return regardless of the mutual auth setting. I'm not sure how much complexity it will remove, it seems like it would mostly just be removing https://github.com/pythongssapi/requests-gssapi/blob/84e052b3fa4ffd00f81c067b27c70461e6494c76/requests_gssapi/gssapi_.py#L215-L217 with a few other tweaks in that function.

That is unless you are suggesting to change the entire workflow so that it will continually re-send the auth tokens until it is complete. This does have the added benefit of supporting auth tokens that require multiple round trips (like NTLM).

Just as an FYI that short little message you receive regardless from some of your servers is just the SPNEGO completion token

# python -m spnego --format yaml --token oRQwEqADCgEAoQsGCSqGSIb3EgECAg==
MessageType: SPNEGO NegTokenResp
Data:
  negState: accept-complete (0)
  supportedMech: Kerberos (1.2.840.113554.1.2.2)
  responseToken:
  mechListMIC:
RawData: A1143012A0030A0100A10B06092A864886F712010202

But considering the token is there and we don't need to do extra work to get it there's no reason why it shouldn't just be processed for completions sake.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 4 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.

michael-o commented 2 years ago

Don't close