| Summary: | SSLUserName is not usable by other modules | ||
|---|---|---|---|
| Product: | Apache httpd-2 | Reporter: | Kevin Bentley <kevin> |
| Component: | mod_ssl | Assignee: | Apache HTTPD Bugs Mailing List <bugs> |
| Status: | REOPENED --- | ||
| Severity: | normal | CC: | adconrad, apachebz, laforge, mdnteo, sascha-web-issues.apache.org, taffy-tyler6464 |
| Priority: | P3 | Keywords: | PatchAvailable |
| Version: | 2.0.54 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
Patch against 2.0.54 to implement a working SSLUserName feature.
Patch against 2.0.54 to let SSLUserName control the username for FakeBasicAuth |
||
I don't think anyone really wanted SSLUserName to be supposed to be a substitute for really doing authentication. If you want that, you can use FakeBasicAuth. SSLUserName was really just supposed to fix the logging problem. If you use FakeBasic auth, your username is very long and ugly. Subversion's authz_svn module uses the username for logging changes. It really sucks to have a log file that is the entire certificate subject. The reason I filed this is that I was trying to patch Subversion's authentication module to allow the use of SSLUserName. I expected that I'd only have to ensure that the module was hooked after mod_ssl, but because the SSLUserName is only filled in in the fixup hook, that isn't possible. I'm currently running a server with this patch on it, with the Subversion authz_svn module modified to support it as well. This works very well, and is much better than FakeBasicAuth. Several people on the Subversion mailing list have asked for a feature just like this. I totally agree with Kevin. FakeBasicAuth might be working for cases where you don't care about the username. But having the full certificate subject as username is definitely a problem with subversion. Not only that the log file has the dn everywhere, it also makes ViewCVS output bloated, and results in broken commit-log mails, since the username is again used as From: header in the mails. using apache+ssl+mod_dav_svn+authz+client_certificates is the only way to get a decently secure subversion repository with per-file granular permissions and strong crypto running... Yes, this could all be fixed up within svn or any other later module, rewriting the certificate subject, replacing it with the email address contained within. But I think the source of this ugliness is FakeBasicAuth in the beginning. It starts with certificate subjects in the 'passwd' files. OK sorry, I didn't understand this properly; this is done as you suggest on the trunk, I'll propose for backport. http://svn.apache.org/viewcvs.cgi/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=151493&r1=151408&r2=151493 [One of the really, really amazing things of this small world is, that you always meet the same people. :-) ] Two things: - the backport is wrong (and also the code on the trunk). The reason for this: ATM, the setting of r->user only happens if nothing in the ssl_hook_access returns DECLINED, FORBIDDEN or anything else before the subroutine actually comes to the setting of r->user, some hundreds of lines below the start of the subroutine (That is why "one method, one exit" is a good thing). - the only way to actually get this to work (at least what I found) in all corner cases is to factor out the user name setting (which should run all the times anyway, no matter what other parts of ssl_hook_access are run or not run) and add it as a separate hook to the processing chain. I built a patch against 2.0.54. I run a slightly older version of this (for Fedora Core 1 2.0.51-1.6.legacy) with Subversion 1.1.4 and client certificate authentication and mod_authz_svn) and it ran fine in all three tests that I did. :-) Side-nit: One of the good thing of the countless code and style checkers for other programming languages besides C is that they keep your line-per-method count down. ssl_hook_access is _screaming_ for a refactoring and a breaking down into smaller parts. Created attachment 14836 [details]
Patch against 2.0.54 to implement a working SSLUserName feature.
For unknown reasons, bugzilla decided to attach this file to another,
completely innocent bug report where it should not go. Sorry about this, don't
know how it happened.
Can you please describe a precise scenario where the current code is not sufficient? Most of the earlier exits from the function are for fatal errors e.g. SSL_get_peer_certificate(ssl) == NULL etc. I admit that I've skimmed over the ssl_hook_access method (it has > 600 lines) and yes, most return pathes end in HTTP_FORBIDDEN (There is a DECLINED when SSL is off but this case is not really interesting for setting the username from a SSL certificate... ;-) ) So yes, you are correct, the current (2.0.54) version of the httpd does work in my scenario (I was testing with 2.0.51 which does not have SSLUserName at all and 2.0.52 from Fedora Core 3, which has it but not in the ssl_hook_access but in ssl_hook_fixup). I still think that this method should be reworked, but this is more of a bikeshed painting issue. I close the bug. The fix for this bug appears to have somewhat broken things, as described in the Debian bug here: http://bugs.debian.org/310650 This bit from the commit seems suspect to me: * However, if FakeAuth is being used then this isn't the case so * we need to postpone setting the username until later. Then you go and test if FAKEBASICAUTH is enabled, and don't perform any magic if it is... But then "later" never comes, and the username is never set. I'm not convinced by that. If FakeBasicAuth is used then mod_ssl should let r->user be set up by the normal means, as it was is already. SSLUsername is to be used in the cases where you *don't* get r->user set at all, and yet authentication has taken place; i.e. using client certs. That's the problem it solves. I'll admit to having not read the section of code in question terribly carefully to see why it's doing what it's doing, it was merely a 20-second glance through to see why the user's installation appears to be behaving differently. If you could look at the Debian bug referenced in my last comment and either give a "oh, that new behaviour is expected and we'll update the docs" or "that's a bug, oops", that'd be great. This isn't a feature I personally use, nor one I have time to debug right now, so I was just passing the reported breakage along. Let me add my voice to this one... since I just fought with this very problem myself, and came to a similar solution before being directed to this issue. The problem is that FakeBasicAuth gives the user _no_ ability to set what r->user will become; it forces it to the DN from the certificate and that is it. I think it's reasonable to let SSLUserName have the desired effect even in FakeBasicAuth mode; I've patched my mod_ssl (from 2.0.54) to set the Authorization header based on dc->szUserName instead of clientdn if SSLUserName was specified. This appears to work fine, and allowed me to work with an unpatched mod_authz_svn (and the remote user name shows properly in access_log as well). This also means that the usernames are 'proper' in the htpasswd file and everywhere else that httpd would normally see/use them. I've attached my patch for this behavior; please let me know if there is some reason why mod_ssl should _not_ behave this way. Created attachment 17052 [details]
Patch against 2.0.54 to let SSLUserName control the username for FakeBasicAuth
(In reply to comment #13) > Created an attachment (id=17052) [details] > Patch against 2.0.54 to let SSLUserName control the username for FakeBasicAuth Is this 2.0-specific? I'm running up against the same limitation in 2.2.14. +1 to the following summary: Rather than overriding SSLUserName if set, +FakeBasicAuth should use the user name from SSLUserName if it is set, defaulting to the current one-line subject representation only if SSLUserName is not set. *** Bug 45325 has been marked as a duplicate of this bug. *** 2004, wow. I am currently trying to provide part of a client certificate DN to mod_auth_radius. I seem to be hitting this bug. (Whatever I do, only the entire DN is transmitted) Looks like I will have to fiddle inside the Radius server and parse the DN string in order to extract what I need. But is there an issue with documentation maybe? https://httpd.apache.org/docs/trunk/mod/mod_ssl.html#sslusername writes: "When the FakeBasicAuth option is enabled, this directive instead controls the value of the username embedded within the basic authentication header (see SSLOptions)." That seems to be exactly what I want to do. But it's not working. (In reply to Jim B. from comment #16) > 2004, wow. > > I am currently trying to provide part of a client certificate DN to > mod_auth_radius. I seem to be hitting this bug. (Whatever I do, only the > entire DN is transmitted) > In 2.4.x it seems like SSLUsername has been moved from fixups to access_checker AKA it is run a bit earlier. > Looks like I will have to fiddle inside the Radius server and parse the DN > string in order to extract what I need. > > But is there an issue with documentation maybe? > https://httpd.apache.org/docs/trunk/mod/mod_ssl.html#sslusername writes: > > "When the FakeBasicAuth option is enabled, this directive instead controls > the value of the username embedded within the basic authentication header > (see SSLOptions)." > > That seems to be exactly what I want to do. But it's not working. It looks like this is only true in trunk, not 2.4.x |
Because SSLUserName is implemented by a fixup hook, and fixup hooks happen after auth and access check hooks, other modules (like mod_authz_svn) cannot use this variable, since it isn't set until after they are done with authentication. A simple solution would be to move this block of code from the fixup hook to the end of the ssl_hook_Access method: /* * Set r->user if requested */ if (dc->szUserName) { val = ssl_var_lookup(r->pool, r->server, r->connection, r, (char *)dc->szUserName); if (val && val[0]) { r->user = val; } } Then, the module can register the hook after mod_ssl.c and get the user variable.