Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix negotiate authentication between CGIs and scheduler #19

Merged

Conversation

scabrero
Copy link
Contributor

Fixes the infinite loop when "Negotiate" authentication is used and allows "Local" certificate based authentication when connecting to localhost:

    Client                       CGI
    Browser <- Remote conn -> admin.cgi <--- Localhost conn --->  Scheduler
      |                           |                                    |
      + --- HTTP/POST /admin/ --> |                                    |
      |                           + --- CUPS-Get-Devices ------------> |
      |                           |                                    |
      |                           | <-- 401 Unauthorized --------------+
      |                           |     WWW-Authenticate:              |
      |                           |       Negotiate, (PeerCred,) Local |
      |                           |                                    |
      | <-- 401 Unauthorized -----+                                    |
      |     WWW-Authenticate:     |                                    |
      |       Negotiate           |                                    |
      |                           |                                    |
      | --- HTTP/POST /admin/ --> |                                    |
      |     Authorization:        + --- IPP CUPS-GetDevices ---------> |
      |       Negotiate           |     Authorization: Local <cert>    |
      |                           |                                    |

Fixes apple/cups issue #5596

SetAuthorizationString with NULL argument sets an empty string.

Related: apple#5596

Signed-off-by: Samuel Cabrero <scabrero@suse.de>
Related: apple#5596

Signed-off-by: Samuel Cabrero <scabrero@suse.de>
If connecting to localhost then proceed to ask the client for the
authorization using cupsGetPassword2. The get password callback will
return 401 to the client with WWW-Authenticate: Negotiate.

Fixes: apple#5596

Signed-off-by: Samuel Cabrero <scabrero@suse.de>
PeerCred is also possible if address family is AF_LOCAL. This will allow
the CGI programs to generate the authorization from the local
certificates based on PID also when Negotiate is used for local
connections:

Client                       CGI
Browser <- Remote conn -> admin.cgi <--- Localhost conn --->  Scheduler
  |                           |                                    |
  + --- HTTP/POST /admin/ --> |                                    |
  |                           + --- CUPS-Get-Devices ------------> |
  |                           |                                    |
  |                           | <-- 401 Unauthorized --------------+
  |                           |     WWW-Authenticate:              |
  |                           |       Negotiate, (PeerCred,) Local |
  |                           |                                    |
  | <-- 401 Unauthorized -----+                                    |
  |     WWW-Authenticate:     |                                    |
  |       Negotiate           |                                    |
  |                           |                                    |
  | --- HTTP/POST /admin/ --> |                                    |
  |     Authorization:        + --- IPP CUPS-GetDevices ---------> |
  |       Negotiate           |     Authorization: Local <cert>    |
  |                           |                                    |

Fixes: apple#5596

Signed-off-by: Samuel Cabrero <scabrero@suse.de>
Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to do more in-depth testing for this because the interactions between Kerberos and local/peer credentials/authref are a constant source of pain.

@@ -2109,18 +2109,13 @@ cupsdSendHeader(
}
else if (auth_type == CUPSD_AUTH_NEGOTIATE)
{
#if defined(SO_PEERCRED) && defined(AF_LOCAL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break macOS clients. We never send Kerberos credentials over a domain socket for a native (IPP) client since we use the login UID to do authorization. Need to think about how this can be managed... :/

Copy link
Contributor Author

@scabrero scabrero Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware how these changes can affect macOS.

The 'PeerCred' mech will still be in WWW-Authenticate string together with 'Negotiate' and 'Local', added just below:

https://github.com/scabrero/cups/blob/web-interface-negotiate-fix/scheduler/client.c#L2135

Then the cups_local_auth will work also for macOS because of this check:

 if (http->hostaddr->addr.sa_family == AF_LOCAL &&
      !getenv("GATEWAY_INTERFACE") &&	/* Not via CGI programs... */
      cups_auth_find(www_auth, "PeerCred"))
  {

/* 1 if local connection */
cups_is_local_connection(http_t *http) /* I - HTTP connection to server */
{
if (!httpAddrLocalhost(http->hostaddr) && _cups_strcasecmp(http->hostname, "localhost") != 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably just be:

return (httpAddLocalhost(http->hostaddr) || !_cups_strcasecmp(http->hostname, "localhost"));

@michaelrsweet michaelrsweet added bug Something isn't working investigating Investigating the issue priority-low labels Nov 4, 2020
@michaelrsweet michaelrsweet self-assigned this Nov 4, 2020
@michaelrsweet michaelrsweet added this to the v2.3.3op1 milestone Nov 11, 2020
@michaelrsweet michaelrsweet modified the milestones: v2.3.3op1, v2.3.3op2 Nov 26, 2020
@scabrero
Copy link
Contributor Author

scabrero commented Dec 1, 2020

Hi Michael,

could I ask the status of this? Is it still under evaluation? Do you require anything from my side to move forward?

@michaelrsweet
Copy link
Member

@scabrero Yes, otherwise I would have closed it with a reason.

Like I said in my original review, I need time to ensure this doesn't cause regressions. This includes setting up a test environment with a Kerberos/AD server, which I no longer have easily available in a test lab, and setting up a macOS VM where I can muck around with the root partition. This isn't a full-time, paid gig for me anymore, I'm volunteering my time and computer resources to help keep CUPS moving forward. So please be patient - some PRs and issues take longer to review and verify.

@michaelrsweet michaelrsweet modified the milestones: v2.3.3op2, v2.3.3op3 Jan 15, 2021
@michaelrsweet
Copy link
Member

OK, I'm going to merge this change. If it causes problems on macOS and Apple actually uses our fork, they can fix it.

Also, I should note that we are deprecating Kerberos support in CUPS 2.4.0, with OAuth coming to pick up the SSO slack.

@michaelrsweet michaelrsweet merged commit 3ff789e into OpenPrinting:master Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working investigating Investigating the issue priority-low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants