From e2612c7873a9bbc79e9c250e700d634792570e59 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 29 Jul 2008 16:19:53 -0400 Subject: on the security of this plugin.. --- doc/plugins/contrib/unixauth/discussion.mdwn | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 doc/plugins/contrib/unixauth/discussion.mdwn (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth/discussion.mdwn b/doc/plugins/contrib/unixauth/discussion.mdwn new file mode 100644 index 000000000..162e5d323 --- /dev/null +++ b/doc/plugins/contrib/unixauth/discussion.mdwn @@ -0,0 +1,17 @@ +The security of this plugin scares me. As noted in the plugin +documentation, you basically have to use it with SSL, since snooping on the +login password doesn't give you an essentially useless account -- it gives +you an actual account on the machine! + +Also, apparently pwauth defers *all* auth attempts if one fails, and it +does this by using a lock file, and sleeping after a failed auth attempt. +Which is needed to avoid brute-forcing, since this is a significant +password.. but how will that interact with ikiwiki? Well, ikiwiki _also_ +uses a lock file. So, at a minimum, someone can not only try to brute-force +the pwauth password, but the ikiwiki processes that stack up due to that +will also keep ikiwiki's lock held. Which basically DOSes the wiki for +everyone else; noone else can try to log in, or log out, or edit a page, +all of which require taking the lock. + +So I don't think I'll be accepting this plugin into ikiwiki itself.. +--[[Joey]] -- cgit v1.2.3 From 6838f9b6e5fb39a307eef3f7eea20497d4398f8b Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Tue, 29 Jul 2008 21:45:17 -0400 Subject: --- doc/plugins/contrib/unixauth/discussion.mdwn | 2 ++ 1 file changed, 2 insertions(+) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth/discussion.mdwn b/doc/plugins/contrib/unixauth/discussion.mdwn index 162e5d323..5f542745d 100644 --- a/doc/plugins/contrib/unixauth/discussion.mdwn +++ b/doc/plugins/contrib/unixauth/discussion.mdwn @@ -15,3 +15,5 @@ all of which require taking the lock. So I don't think I'll be accepting this plugin into ikiwiki itself.. --[[Joey]] + +Thanks for the comments. That's definitely an undesirable interaction between pwauth and ikiwiki; in my current application it wouldn't be a serious problem, but I'd like this plugin to be general-purpose and safe enough for inclusion in ikiwiki. It's the system-users-are-wiki-users idea I'm married to here, not pwauth itself; can you suggest another approach I might take? -- cgit v1.2.3 From e55c0798441111170d731eb3ec7a4874dadda79b Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Tue, 29 Jul 2008 21:45:50 -0400 Subject: --- doc/plugins/contrib/unixauth/discussion.mdwn | 1 + 1 file changed, 1 insertion(+) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth/discussion.mdwn b/doc/plugins/contrib/unixauth/discussion.mdwn index 5f542745d..7bfdc9665 100644 --- a/doc/plugins/contrib/unixauth/discussion.mdwn +++ b/doc/plugins/contrib/unixauth/discussion.mdwn @@ -17,3 +17,4 @@ So I don't think I'll be accepting this plugin into ikiwiki itself.. --[[Joey]] Thanks for the comments. That's definitely an undesirable interaction between pwauth and ikiwiki; in my current application it wouldn't be a serious problem, but I'd like this plugin to be general-purpose and safe enough for inclusion in ikiwiki. It's the system-users-are-wiki-users idea I'm married to here, not pwauth itself; can you suggest another approach I might take? +-- [[schmonz]] -- cgit v1.2.3 From 2c1e02aa4574f6c264aee6e498da4d0ed6b2ed4b Mon Sep 17 00:00:00 2001 From: "http://www.cse.unsw.edu.au/~willu/" Date: Tue, 29 Jul 2008 23:39:15 -0400 Subject: alternate suggestion --- doc/plugins/contrib/unixauth/discussion.mdwn | 2 ++ 1 file changed, 2 insertions(+) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth/discussion.mdwn b/doc/plugins/contrib/unixauth/discussion.mdwn index 7bfdc9665..91c59ff1d 100644 --- a/doc/plugins/contrib/unixauth/discussion.mdwn +++ b/doc/plugins/contrib/unixauth/discussion.mdwn @@ -18,3 +18,5 @@ So I don't think I'll be accepting this plugin into ikiwiki itself.. Thanks for the comments. That's definitely an undesirable interaction between pwauth and ikiwiki; in my current application it wouldn't be a serious problem, but I'd like this plugin to be general-purpose and safe enough for inclusion in ikiwiki. It's the system-users-are-wiki-users idea I'm married to here, not pwauth itself; can you suggest another approach I might take? -- [[schmonz]] + +> Have you considered using [[plugins/httpauth]] and then the appropriate apache module? There are apache modules like [mod_authnz_external](http://unixpapa.com/mod_auth_external.html) that might help. The advantage of these solutions is that they usually make the security implications explicit. -- Will -- cgit v1.2.3 From dd25c7c4afa8f57e909fed63fb6bcf1648de531b Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 01:25:05 -0400 Subject: --- doc/plugins/contrib/unixauth/discussion.mdwn | 3 +++ 1 file changed, 3 insertions(+) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth/discussion.mdwn b/doc/plugins/contrib/unixauth/discussion.mdwn index 91c59ff1d..863e3c91a 100644 --- a/doc/plugins/contrib/unixauth/discussion.mdwn +++ b/doc/plugins/contrib/unixauth/discussion.mdwn @@ -20,3 +20,6 @@ Thanks for the comments. That's definitely an undesirable interaction between pw -- [[schmonz]] > Have you considered using [[plugins/httpauth]] and then the appropriate apache module? There are apache modules like [mod_authnz_external](http://unixpapa.com/mod_auth_external.html) that might help. The advantage of these solutions is that they usually make the security implications explicit. -- Will + +Actually, yes. That's how I made sure I had pwauth working to begin with. I'm partial to the form-based approach because I'm not aware of any way to reliably "log out" browsers from HTTP authentication. If that *is* reliably possible, then I worked way too hard for no reason. ;-) +-- [[schmonz]] -- cgit v1.2.3 From 85aff81cfe50f677ca0e1c307bd55c336ea73288 Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 12:20:58 -0400 Subject: revamp, so it's vampier --- doc/plugins/contrib/unixauth.mdwn | 47 ++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 8 deletions(-) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth.mdwn b/doc/plugins/contrib/unixauth.mdwn index 12f885c33..6cdf87f6a 100644 --- a/doc/plugins/contrib/unixauth.mdwn +++ b/doc/plugins/contrib/unixauth.mdwn @@ -3,9 +3,16 @@ This plugin authenticates users against the Unix user database. It presents a similar UI to [[plugins/passwordauth]], but simpler, as there's no need to be able to register or change one's password. -[pwauth](http://www.unixpapa.com/pwauth/) must be installed and working. In particular, it must be configured to recognize the UID of the calling web server, or authentication will always fail. Set `pwauth_path` to the full path of your pwauth binary. +To authenticate, either [checkpassword](http://cr.yp.to/checkpwd.html) or [pwauth](http://www.unixpapa.com/pwauth/) must be installed and configured. `checkpassword` is strongly preferred. If your web server runs as an unprivileged user -- as it darn well should! -- then `checkpassword` needs to be setuid root. Other checkpassword implementations are available, notably [checkpassword-pam](http://checkpasswd-pam.sourceforge.net/). -As [with passwordauth](/security/#index14h2), be wary of sending usernames and passwords in cleartext. Unlike with passwordauth, sniffing these credentials can get an attacker much further than mere wiki access. SSL with this plugin is a __must__. +Config variables that affect the behavior of `unixauth`: + +* `unixauth_type`: defaults to unset, can be "checkpassword" or "pwauth" +* `unixauth_command`: defaults to unset, should contain the full path and any arguments +* `unixauth_sslrequire`: defaults to 1, can be 0 +* `sslcookie`: needs to be 1 if `unixauth_sslrequire` is 1 (perhaps this should be done automatically?) + +__Security__: [As with passwordauth](/security/#index14h2), be wary of sending usernames and passwords in cleartext. Unlike passwordauth, sniffing `unixauth` credentials can get an attacker much further than mere wiki access. Therefore, this plugin defaults to not even _displaying_ the login form fields unless we're running under SSL. Nobody should be able to do anything remotely dumb until the admin has done at least a little thinking. After that, dumb things are always possible. ;-) [[!toggle id="code" text="unixauth.pm"]] @@ -40,13 +47,26 @@ As [with passwordauth](/security/#index14h2), be wary of sending usernames and p } my $ret=0; - if (! exists $config{pwauth_path}) { - $config{pwauth_path}="/usr/libexec/pwauth"; + if (! exists $config{unixauth_type}) { + # admin needs to carefully think over his configuration + return 0; + } + elsif ($config{unixauth_type} eq "checkpassword") { + open UNIXAUTH, "|$config{unixauth_command} true 3<&0" or die("Could not run $config{unixauth_type}"); + print UNIXAUTH "$user\0$password\0Y123456\0"; + close UNIXAUTH; + $ret=!($?>>8); + } + elsif ($config{unixauth_type} eq "pwauth") { + open UNIXAUTH, "|$config{unixauth_command}" or die("Could not run $config{unixauth_type}"); + print UNIXAUTH "$user\n$password\n"; + close UNIXAUTH; + $ret=!($?>>8); + } + else { + # no such authentication type + return 0; } - open PWAUTH, "|$config{pwauth_path}" or die("Could not run pwauth"); - print PWAUTH "$user\n$password\n"; - close PWAUTH; - $ret=!($?>>8); if ($ret) { my $userinfo=IkiWiki::userinfo_retrieve(); @@ -69,6 +89,16 @@ As [with passwordauth](/security/#index14h2), be wary of sending usernames and p my $session=$params{session}; my $cgi=$params{cgi}; + # if not under SSL, die before even showing a login form, + # unless the admin explicitly says it's fine + if (! exists $config{unixauth_requiressl}) { + $config{unixauth_requiressl} = 1; + } + if ($config{unixauth_requiressl} && \ + (! $config{sslcookie} || ! exists $ENV{'HTTPS'})) { + die("SSL required to login. Contact your administrator."); + } + if ($form->title eq "signin") { $form->field(name => "name", required => 0); $form->field(name => "password", type => "password", required => 0); @@ -93,6 +123,7 @@ As [with passwordauth](/security/#index14h2), be wary of sending usernames and p ); } + # XXX is this reachable? looks like no elsif ($submittype eq "Login") { $form->field( name => "name", -- cgit v1.2.3 From 3b9fe3a1b64b011a72b6c54cb172a27922250d8b Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 12:21:55 -0400 Subject: update --- doc/plugins/contrib/unixauth/discussion.mdwn | 3 +++ 1 file changed, 3 insertions(+) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth/discussion.mdwn b/doc/plugins/contrib/unixauth/discussion.mdwn index 863e3c91a..a209b9030 100644 --- a/doc/plugins/contrib/unixauth/discussion.mdwn +++ b/doc/plugins/contrib/unixauth/discussion.mdwn @@ -23,3 +23,6 @@ Thanks for the comments. That's definitely an undesirable interaction between pw Actually, yes. That's how I made sure I had pwauth working to begin with. I'm partial to the form-based approach because I'm not aware of any way to reliably "log out" browsers from HTTP authentication. If that *is* reliably possible, then I worked way too hard for no reason. ;-) -- [[schmonz]] + +I've added support for [checkpassword](http://cr.yp.to/checkpwd/interface.html), since those generally don't have any rate-limiting cleverness to interfere with ikiwiki's, and made a few other changes. Please check out the plugin docs again and let me know if this is closer to being acceptable. +-- [[schmonz]] -- cgit v1.2.3 From e4b096ac411494416d73e344e0aaeaacabf2f266 Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 14:27:35 -0400 Subject: http(oop)s --- doc/plugins/contrib/unixauth.mdwn | 2 ++ 1 file changed, 2 insertions(+) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth.mdwn b/doc/plugins/contrib/unixauth.mdwn index 6cdf87f6a..f369cd6ad 100644 --- a/doc/plugins/contrib/unixauth.mdwn +++ b/doc/plugins/contrib/unixauth.mdwn @@ -14,6 +14,8 @@ Config variables that affect the behavior of `unixauth`: __Security__: [As with passwordauth](/security/#index14h2), be wary of sending usernames and passwords in cleartext. Unlike passwordauth, sniffing `unixauth` credentials can get an attacker much further than mere wiki access. Therefore, this plugin defaults to not even _displaying_ the login form fields unless we're running under SSL. Nobody should be able to do anything remotely dumb until the admin has done at least a little thinking. After that, dumb things are always possible. ;-) +_XXX hang on, looks like we don't have the huge CGI environment so testing for ${HTTPS} always fails; need another way to be sure_ + [[!toggle id="code" text="unixauth.pm"]] [[!toggleable id="code" text=""" -- cgit v1.2.3 From bf0483ed96755bb0aee22cf5e10a6f764cd15327 Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 14:53:45 -0400 Subject: okay, tested to really work as advertised --- doc/plugins/contrib/unixauth.mdwn | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth.mdwn b/doc/plugins/contrib/unixauth.mdwn index f369cd6ad..7442b6291 100644 --- a/doc/plugins/contrib/unixauth.mdwn +++ b/doc/plugins/contrib/unixauth.mdwn @@ -14,13 +14,31 @@ Config variables that affect the behavior of `unixauth`: __Security__: [As with passwordauth](/security/#index14h2), be wary of sending usernames and passwords in cleartext. Unlike passwordauth, sniffing `unixauth` credentials can get an attacker much further than mere wiki access. Therefore, this plugin defaults to not even _displaying_ the login form fields unless we're running under SSL. Nobody should be able to do anything remotely dumb until the admin has done at least a little thinking. After that, dumb things are always possible. ;-) -_XXX hang on, looks like we don't have the huge CGI environment so testing for ${HTTPS} always fails; need another way to be sure_ +`unixauth` tests for the presence of the `HTTPS` environment variable. `Wrapper.pm` needs to be tweaked to pass it through; without that, the plugin fails closed. + +[[!toggle id="diff" text="Wrapper.pm.diff"]] + +[[!toggleable id="diff" text=""" + + --- Wrapper.pm.orig 2008-07-29 00:09:10.000000000 -0400 + +++ Wrapper.pm + @@ -28,7 +28,7 @@ sub gen_wrapper () { #{{{ + my @envsave; + push @envsave, qw{REMOTE_ADDR QUERY_STRING REQUEST_METHOD REQUEST_URI + CONTENT_TYPE CONTENT_LENGTH GATEWAY_INTERFACE + - HTTP_COOKIE REMOTE_USER} if $config{cgi}; + + HTTP_COOKIE REMOTE_USER HTTPS} if $config{cgi}; + my $envsave=""; + foreach my $var (@envsave) { + $envsave.=<<"EOF" + +"""]] [[!toggle id="code" text="unixauth.pm"]] [[!toggleable id="code" text=""" - #!/usr/bin/perl + #!/usr//bin/perl # Ikiwiki unixauth authentication. package IkiWiki::Plugin::unixauth; @@ -96,9 +114,10 @@ _XXX hang on, looks like we don't have the huge CGI environment so testing for $ if (! exists $config{unixauth_requiressl}) { $config{unixauth_requiressl} = 1; } - if ($config{unixauth_requiressl} && \ - (! $config{sslcookie} || ! exists $ENV{'HTTPS'})) { - die("SSL required to login. Contact your administrator."); + if ($config{unixauth_requiressl}) { + if ((! $config{sslcookie}) || (! exists $ENV{'HTTPS'})) { + die("SSL required to login. Contact your administrator.
"); + } } if ($form->title eq "signin") { -- cgit v1.2.3 From 7b37c78ad895509a2517e019b954c9ff2bf71549 Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 14:55:58 -0400 Subject: fix cutto --- doc/plugins/contrib/unixauth.mdwn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth.mdwn b/doc/plugins/contrib/unixauth.mdwn index 7442b6291..f7019b4b8 100644 --- a/doc/plugins/contrib/unixauth.mdwn +++ b/doc/plugins/contrib/unixauth.mdwn @@ -38,7 +38,7 @@ __Security__: [As with passwordauth](/security/#index14h2), be wary of sending u [[!toggleable id="code" text=""" - #!/usr//bin/perl + #!/usr/bin/perl # Ikiwiki unixauth authentication. package IkiWiki::Plugin::unixauth; -- cgit v1.2.3 From bd5f94d2ac85d6fb5d60400345fe41b91f2d303c Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 15:09:44 -0400 Subject: more suid --- doc/plugins/contrib/unixauth.mdwn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth.mdwn b/doc/plugins/contrib/unixauth.mdwn index f7019b4b8..1917a242d 100644 --- a/doc/plugins/contrib/unixauth.mdwn +++ b/doc/plugins/contrib/unixauth.mdwn @@ -3,7 +3,7 @@ This plugin authenticates users against the Unix user database. It presents a similar UI to [[plugins/passwordauth]], but simpler, as there's no need to be able to register or change one's password. -To authenticate, either [checkpassword](http://cr.yp.to/checkpwd.html) or [pwauth](http://www.unixpapa.com/pwauth/) must be installed and configured. `checkpassword` is strongly preferred. If your web server runs as an unprivileged user -- as it darn well should! -- then `checkpassword` needs to be setuid root. Other checkpassword implementations are available, notably [checkpassword-pam](http://checkpasswd-pam.sourceforge.net/). +To authenticate, either [checkpassword](http://cr.yp.to/checkpwd.html) or [pwauth](http://www.unixpapa.com/pwauth/) must be installed and configured. `checkpassword` is strongly preferred. If your web server runs as an unprivileged user -- as it darn well should! -- then `checkpassword` needs to be setuid root. (Or your ikiwiki CGI wrapper, I guess, but don't do that.) Other checkpassword implementations are available, notably [checkpassword-pam](http://checkpasswd-pam.sourceforge.net/). Config variables that affect the behavior of `unixauth`: -- cgit v1.2.3 From fd160168332e90569f0f87b573a39687797c3fcd Mon Sep 17 00:00:00 2001 From: "http://schmonz.livejournal.com/" Date: Wed, 30 Jul 2008 15:11:47 -0400 Subject: s/sslrequire/requiressl/g --- doc/plugins/contrib/unixauth.mdwn | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth.mdwn b/doc/plugins/contrib/unixauth.mdwn index 1917a242d..2de6fc51f 100644 --- a/doc/plugins/contrib/unixauth.mdwn +++ b/doc/plugins/contrib/unixauth.mdwn @@ -9,8 +9,8 @@ Config variables that affect the behavior of `unixauth`: * `unixauth_type`: defaults to unset, can be "checkpassword" or "pwauth" * `unixauth_command`: defaults to unset, should contain the full path and any arguments -* `unixauth_sslrequire`: defaults to 1, can be 0 -* `sslcookie`: needs to be 1 if `unixauth_sslrequire` is 1 (perhaps this should be done automatically?) +* `unixauth_requiressl`: defaults to 1, can be 0 +* `sslcookie`: needs to be 1 if `unixauth_requiressl` is 1 (perhaps this should be done automatically?) __Security__: [As with passwordauth](/security/#index14h2), be wary of sending usernames and passwords in cleartext. Unlike passwordauth, sniffing `unixauth` credentials can get an attacker much further than mere wiki access. Therefore, this plugin defaults to not even _displaying_ the login form fields unless we're running under SSL. Nobody should be able to do anything remotely dumb until the admin has done at least a little thinking. After that, dumb things are always possible. ;-) -- cgit v1.2.3 From 5e85039dc3a329a064a0d3053bbca2ed066f5292 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 30 Jul 2008 15:47:28 -0400 Subject: response --- doc/plugins/contrib/unixauth/discussion.mdwn | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'doc/plugins/contrib') diff --git a/doc/plugins/contrib/unixauth/discussion.mdwn b/doc/plugins/contrib/unixauth/discussion.mdwn index a209b9030..c4f5ff269 100644 --- a/doc/plugins/contrib/unixauth/discussion.mdwn +++ b/doc/plugins/contrib/unixauth/discussion.mdwn @@ -26,3 +26,7 @@ Actually, yes. That's how I made sure I had pwauth working to begin with. I'm pa I've added support for [checkpassword](http://cr.yp.to/checkpwd/interface.html), since those generally don't have any rate-limiting cleverness to interfere with ikiwiki's, and made a few other changes. Please check out the plugin docs again and let me know if this is closer to being acceptable. -- [[schmonz]] + +> I actually think that the rate limiting is a good thing. After all, +> ikiwiki doesn't do its own login rate limiting. Just need to find a way +> to disentangle the two locks. --[[Joey]] -- cgit v1.2.3