Bug 31418 - SSLUserName is not usable by other modules
Summary: SSLUserName is not usable by other modules
Status: REOPENED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_ssl (show other bugs)
Version: 2.0.54
Hardware: All All
: P3 normal with 9 votes (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: PatchAvailable
: 45325 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-09-25 03:58 UTC by Kevin Bentley
Modified: 2020-09-26 13:52 UTC (History)
6 users (show)



Attachments
Patch against 2.0.54 to implement a working SSLUserName feature. (3.54 KB, patch)
2005-04-26 01:26 UTC, Henning Schmiedehausen
Details | Diff
Patch against 2.0.54 to let SSLUserName control the username for FakeBasicAuth (1.16 KB, patch)
2005-11-27 01:03 UTC, Kevin P. Fleming
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Bentley 2004-09-25 03:58:07 UTC
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.
Comment 1 Joe Orton 2004-10-08 07:23:05 UTC
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.
Comment 2 Kevin Bentley 2004-10-14 20:35:42 UTC
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.
Comment 3 Harald Welte 2004-10-19 22:16:24 UTC
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.
Comment 4 Joe Orton 2005-02-10 19:32:56 UTC
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
Comment 5 Henning Schmiedehausen 2005-04-26 01:23:47 UTC
[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. 
Comment 6 Henning Schmiedehausen 2005-04-26 01:26:45 UTC
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.
Comment 7 Joe Orton 2005-04-26 13:30:33 UTC
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.
Comment 8 Henning Schmiedehausen 2005-04-27 09:59:06 UTC
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.
Comment 9 Adam Conrad 2005-05-25 04:14:15 UTC
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.
Comment 10 Joe Orton 2005-06-01 11:24:54 UTC
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.
Comment 11 Adam Conrad 2005-06-01 12:24:39 UTC
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.
Comment 12 Kevin P. Fleming 2005-11-27 01:02:33 UTC
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.
Comment 13 Kevin P. Fleming 2005-11-27 01:03:13 UTC
Created attachment 17052 [details]
Patch against 2.0.54 to let SSLUserName control the username for FakeBasicAuth
Comment 14 Peter Thomas 2010-02-25 01:03:10 UTC
(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.
Comment 15 Stefan Fritsch 2011-06-29 18:38:05 UTC
*** Bug 45325 has been marked as a duplicate of this bug. ***
Comment 16 Jim B. 2020-09-26 13:43:11 UTC
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.
Comment 17 Eric Covener 2020-09-26 13:52:23 UTC
(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