New plugin and .diff attached. Comments in-line below.
John Peacock wrote:
> Elliot F wrote:
>
>> So, not that I'm replying to my own post, but (other than the input
>> validation
>> issue) what do you guys think? What changes do I need to make before
>> someone
>> commits this?
>
>
> There are a couple of things I'd like to see changed before I commit this:
>
> 1) The current code retrieves the 5 configuration files for each
> transaction, even though they are unlikely to change after loading; it
> would be better if these were obtained during register() and stashed in
> the plugin object itself (which, if you are concerned about performance
> and running the forkserver implementation, is a persistent object).
Done. Thank you for that suggestion.
> 2) In addition, each of the 5 configuration values is a single value, so
> I would prefer that there either be a single external configuration file
> containing all values or better yet, just make them config line options
> (as name/value pairs); there's a couple of ways to do the latter very
> compactly. My current favorite is:
>
> %{ $self->{"_args"} } = @args;
>
> works very nice (you get a big fat warning if you have unbalanced
> key/value pairs). Then you can check for default values one line at a
> time:
>
> $self->{"_args"}->{"ldhost"} ||= "localhost";
> $self->{"_args"}->{"ldPORT"} ||= 389;
> # etc
Done. Configuration can be given via the 'ldap' config file, or given as
arguments to the plugin. If both are set, the plugin arguments override the
configuration file settings.
One problem is that if a configuration value has a space (or spaces) in it, it
breaks. This could be a problem for many people's LDAP configs, as many DNs
have spaces, and if/when I add the configurable filter (which could very likely
have spaces,) that would be a problem as well.
> 3) Don't DENY unless you are absolutely sure this is going to be the
> only auth module loaded. In particular, return DECLINED if any of the
> mandatory configuration values are unset, so someone could potentially
> add this to a running config and then set it up without disturbing
> existing users. It is almost always best to DECLINED and let some other
> plugin take over than to DENY and block further action. It is not
> inconceivable that someone wanting to run this LDAP plugin already has
> other auth methods enabled and cannot convert all users to LDAP at the
> same time.
>
> On the theory of return values from plugins, I like to look at it this way:
>
> 1) return DENY or OK if the plugin is a _positive test_, i.e. this host
> *is* on a blacklist, this e-mail address *is* banned, or this host is in
> the relayclients list;
>
> 2) return DECLINED if the plugin is a _negative test_, i.e. this host
> address *isn't* on any blacklist, or this host *isn't* in relayclients ,
> etc.
>
> The idea is that a _postive_ test is for a specific condition which, if
> met, absolutely determines the next action. A _negative_ test simply
> states that some condition hasn't *yet* been met, but the next plugin
> might succeed.
So then if a user can't be found (either in mysql or LDAP or whatever) should
the plugin returned DENY (under the assumption it's the only auth plugin) or
DECLINED (if there is more than one way for the user to AUTH)?
For that matter, should the auth plugin ever return DENY? If there is more than
one auth plugin (for whatever reason), then if the first fails, none of the
others will run. A corner case, yes, but interesting anyway.
If a DECLINED is returned, it simply means the relayclient variable won't be set
for the connection, and the user won't be able to relay. I wonder how that
would affect the auth portion, as the client should get a success/fail after
trying to auth against the SMTP server.
> HTH
>
> John
#!/usr/bin/perl -Tw
sub register {
my ( $self, $qp, @args ) = @_;
$self->register_hook( "auth-plain", "authldap" );
$self->register_hook( "auth-login", "authldap" );
# pull config defaults in from file
%{ $self->{"ldconf"} } = map { (split /\s+/, $_, 2)[0,1] }
$self->qp->config('ldap');
# override ldap config defaults with plugin args
for my $ldap_arg (@args) {
%{ $self->{"ldconf"} } = map { (split /\s+/, $_, 2)[0,1] } $ldap_arg;
}
# do light validation of ldap_host and ldap_port to satisfy -T
my $ldhost = $self->{"ldconf"}->{'ldap_host'};
my $ldport = $self->{"ldconf"}->{'ldap_port'};
if (($ldhost) && ($ldhost =~ m/^(([a-z0-9]+\.?)+)$/)) {
$self->{"ldconf"}->{'ldap_host'} = $1
} else {
undef $self->{"ldconf"}->{'ldap_host'};
}
if (($ldport) && ($ldport =~ m/^(\d+)$/)) {
$self->{"ldconf"}->{'ldap_port'} = $1
} else {
undef $self->{"ldconf"}->{'ldap_port'};
}
# set any values that are not already
$self->{"ldconf"}->{"ldap_host"} ||= "127.0.0.1";
$self->{"ldconf"}->{"ldap_port"} ||= 389;
$self->{"ldconf"}->{"ldap_timeout"} ||= 5;
$self->{"ldconf"}->{"ldap_auth_filter_attr"} ||= "uid";
}
sub authldap {
use Net::LDAP qw(:all);
use Qpsmtpd::Constants;
my ( $self, $transaction, $method, $user, $passClear, $passHash, $ticket ) =
@_;
my ($ldhost, $ldport, $ldwait, $ldbase, $ldmattr, $lduserdn, $ldh, $mesg);
# pull values in from config
$ldhost = $self->{"ldconf"}->{"ldap_host"};
$ldport = $self->{"ldconf"}->{"ldap_port"};
$ldbase = $self->{"ldconf"}->{"ldap_base"};
# log error here and DECLINE if no baseDN, because a custom baseDN is
required:
unless ($ldbase) {
$self->log(LOGERROR, "authldap/$method - please configure ldap_base" ) &&
return ( DECLINED, "authldap/$method - temporary auth error" );
}
$ldwait = $self->{"ldconf"}->{'ldap_timeout'};
$ldmattr = $self->{"ldconf"}->{'ldap_auth_filter_attr'};
my ( $pw_name, $pw_domain ) = split "@", lc($user);
# find dn of user matching supplied username
$ldh = Net::LDAP->new($ldhost, port=>$ldport, timeout=>$ldwait ) or
$self->log(LOGALERT, "authldap/$method - error in initial conn" ) &&
return ( DENY, "authldap/$method - temporary auth error" );
# find the user's DN
$mesg = $ldh->search(
base=>$ldbase,
scope=>'sub',
filter=>"$ldmattr=$pw_name",
attrs=>['uid'],
timeout=>$ldwait,
sizelimit=>'1') or
$self->log(LOGALERT, "authldap/$method - err in search for user" ) &&
return ( DENY, "authldap/$method - temporary auth error" );
# deal with errors if they exist
if ( $mesg->code ) {
$self->log(LOGALERT, "authldap/$method - err " . $mesg->code . " in search
for user" );
return ( DENY, "authldap/$method - temporary auth error" );
}
# unbind, so as to allow a rebind below
$ldh->unbind if ($ldh);
# bind against directory as user with password supplied
if (($mesg->count) && ($lduserdn = $mesg->entry->dn)) {
$ldh = Net::LDAP->new($ldhost, port=>$ldport, timeout=>$ldwait ) or
$self->log(LOGALERT, "authldap/$method - err in user conn" ) &&
return ( DENY, "authldap/$method - temporary auth error" );
# here's the whole reason for the script
$mesg = $ldh->bind($lduserdn, password=>$passClear, timeout=>$ldwait);
$ldh->unbind if ($ldh);
# deal with errors if they exist, or allow success
if ( $mesg->code ) {
$self->log(LOGALERT, "authldap/$method - error in user bind" );
return ( DENY, "authldap/$method - wrong username or password" );
} else {
$self->log( LOGINFO, "authldap/$method - $user auth success" );
$self->log( LOGDEBUG, "authldap/$method - user: $user, pass: $passClear"
);
return ( OK, "authldap/$method" );
}
# if the plugin couldn't find user's entry
} else {
$self->log(LOGALERT, "authldap/$method - user not found" ) &&
return ( DENY, "authldap/$method - wrong username or password" );
}
$ldh->disconnect;
}
=head1 NAME
auth_ldap_bind - Authenticate user via an LDAP bind
=head1 DESCRIPTION
This plugin authenticates users against an LDAP Directory. The plugin
first performs a lookup for an entry matching the connecting user. This
lookup uses the 'ldap_auth_filter_attr' attribute to match the connecting
user to their LDAP DN. Once the plugin has found the user's DN, the plugin
will attempt to bind to the Directory as that DN with the password that has
been supplied.
=head1 CONFIGURATION
Configuration items can be held in either the 'ldap' configuration file, or as
arguments to the plugin.
Configuration items in the 'ldap' configuration file
are set one per line, starting the line with the configuration item key,
followed by a space, then the values associated with the configuration item.
Configuration items given as arguments to the plugin are keys and values
separated by spaces. Note that this means any DNs or values with spaces will
be effectively broken.
The only configuration item which is required is 'ldap_base'. This tells the
plugin what your base DN is. The plugin will not work until it has been
configured.
The configuration items 'ldap_host' and 'ldap_port' specify the host and port
at which your Directory server may be contacted. If these are not specified,
the plugin will use port '389' on 'localhost'.
The configuration item 'ldap_timeout' specifies how long the plugin should
wait for a response from your Directory server. By default, the value is 5
seconds.
The configuration item 'ldap_auth_filter_attr' specifies how the plugin should
find the user in your Directory. By default, the plugin will look up the user
based on the 'uid' attribute.
=head1 NOTES
Each auth requires an initial lookup to find the user's DN. Ideally, the
plugin would simply bind as the user without the need for this lookup(see
FUTURE DIRECTION below).
This plugin requires that the Directory allow anonymous bind (see FUTURE
DIRECTION below).
=head1 FUTURE DIRECTION
A configurable LDAP filter should be made available, to account for users
who are over quota, have had their accounts disabled, or whatever other
arbitrary requirements.
A configurable DN template (uid=$USER,ou=$DOMAIN,$BASE). This would prevent
the need of the initial user lookup, as the DN is created from the template.
A configurable bind DN, for Directories that do not allow anonymous bind.
Another plugin ('ldap_auth_cleartext'?), to allow retrieval of plain-text
passwords from the Directory, permitting CRAM-MD5 or other hash algorithm
authentication.
=head1 AUTHOR
Elliot Foster <[EMAIL PROTECTED]>
=head1 COPYRIGHT AND LICENSE
Copyright (c) 2005 Elliot Foster
This plugin is licensed under the same terms as the qpsmtpd package itself.
Please see the LICENSE file included with qpsmtpd for details.
=cut
--- auth_ldap_bind.orig 2005-04-12 08:50:52.000000000 -0700
+++ auth_ldap_bind.new 2005-04-12 10:07:06.171752632 -0700
@@ -1,48 +1,59 @@
-# bind to directory as user/pass given
-#
-# $user (form of '[EMAIL PROTECTED]' or just 'user'), $passclear, $passHash
(md5?)
-#
+#!/usr/bin/perl -Tw
sub register {
- my ( $self, $qp ) = @_;
+ my ( $self, $qp, @args ) = @_;
$self->register_hook( "auth-plain", "authldap" );
$self->register_hook( "auth-login", "authldap" );
-}
-
-sub authldap {
- ## TODO: add unencrypted password retrieval in the future for CRAM
- ## TODO: add ability to specify custom filter in config
- ## Example: (&(uid=$USER)(customattr=$DOMAIN)(!(accountstatus=disabled)))
- use Net::LDAP qw(:all);
- use Qpsmtpd::Constants;
- #use Digest::HMAC_MD5 qw(hmac_md5_hex);
+ # pull config defaults in from file
+ %{ $self->{"ldconf"} } = map { (split /\s+/, $_, 2)[0,1] }
$self->qp->config('ldap');
- my ( $self, $transaction, $method, $user, $passClear, $passHash, $ticket ) =
- @_;
- my ($ldhost, $ldport, $ldwait, $ldbase, $ldmattr, $lduserdn, $ldh, $mesg);
+ # override ldap config defaults with plugin args
+ for my $ldap_arg (@args) {
+ %{ $self->{"ldconf"} } = map { (split /\s+/, $_, 2)[0,1] } $ldap_arg;
+ }
# do light validation of ldap_host and ldap_port to satisfy -T
- $ldhost = $self->qp->config('ldap_host');
- $ldport = $self->qp->config('ldap_port');
+ my $ldhost = $self->{"ldconf"}->{'ldap_host'};
+ my $ldport = $self->{"ldconf"}->{'ldap_port'};
if (($ldhost) && ($ldhost =~ m/^(([a-z0-9]+\.?)+)$/)) {
- $ldhost = $1;
+ $self->{"ldconf"}->{'ldap_host'} = $1
} else {
- $ldhost = "localhost";
+ undef $self->{"ldconf"}->{'ldap_host'};
}
if (($ldport) && ($ldport =~ m/^(\d+)$/)) {
- $ldport = $1;
+ $self->{"ldconf"}->{'ldap_port'} = $1
} else {
- $ldport = "389";
+ undef $self->{"ldconf"}->{'ldap_port'};
}
- # log error here and DENY because a custom baseDN is required:
- $ldbase = $self->qp->config('ldap_base');
+
+ # set any values that are not already
+ $self->{"ldconf"}->{"ldap_host"} ||= "127.0.0.1";
+ $self->{"ldconf"}->{"ldap_port"} ||= 389;
+ $self->{"ldconf"}->{"ldap_timeout"} ||= 5;
+ $self->{"ldconf"}->{"ldap_auth_filter_attr"} ||= "uid";
+}
+
+sub authldap {
+ use Net::LDAP qw(:all);
+ use Qpsmtpd::Constants;
+
+ my ( $self, $transaction, $method, $user, $passClear, $passHash, $ticket ) =
+ @_;
+ my ($ldhost, $ldport, $ldwait, $ldbase, $ldmattr, $lduserdn, $ldh, $mesg);
+
+ # pull values in from config
+ $ldhost = $self->{"ldconf"}->{"ldap_host"};
+ $ldport = $self->{"ldconf"}->{"ldap_port"};
+ $ldbase = $self->{"ldconf"}->{"ldap_base"};
+
+ # log error here and DECLINE if no baseDN, because a custom baseDN is
required:
unless ($ldbase) {
$self->log(LOGERROR, "authldap/$method - please configure ldap_base" ) &&
- return ( DENY, "authldap/$method - temporary auth error" );
+ return ( DECLINED, "authldap/$method - temporary auth error" );
}
- $ldwait = $self->qp->config('ldap_timeout') || "5";
- $ldmattr = $self->qp->config('ldap_auth_filter_attr') || "uid";
+ $ldwait = $self->{"ldconf"}->{'ldap_timeout'};
+ $ldmattr = $self->{"ldconf"}->{'ldap_auth_filter_attr'};
my ( $pw_name, $pw_domain ) = split "@", lc($user);
@@ -72,7 +83,7 @@
$ldh->unbind if ($ldh);
# bind against directory as user with password supplied
- if ( $lduserdn = $mesg->entry->dn ) {
+ if (($mesg->count) && ($lduserdn = $mesg->entry->dn)) {
$ldh = Net::LDAP->new($ldhost, port=>$ldport, timeout=>$ldwait ) or
$self->log(LOGALERT, "authldap/$method - err in user conn" ) &&
return ( DENY, "authldap/$method - temporary auth error" );
@@ -91,9 +102,9 @@
return ( OK, "authldap/$method" );
}
- # this is if the plugin couldn't find user's entry
+ # if the plugin couldn't find user's entry
} else {
- $self->log(LOGALERT, "authldap/$method - err in search results" ) &&
+ $self->log(LOGALERT, "authldap/$method - user not found" ) &&
return ( DENY, "authldap/$method - wrong username or password" );
}
@@ -115,19 +126,30 @@
=head1 CONFIGURATION
-The only configuration file which is required is the 'ldap_base' file. This
-file tells the plugin what your base DN is. The plugin will deny access (the
-sane default) until it has been configured.
+Configuration items can be held in either the 'ldap' configuration file, or as
+arguments to the plugin.
+
+Configuration items in the 'ldap' configuration file
+are set one per line, starting the line with the configuration item key,
+followed by a space, then the values associated with the configuration item.
+
+Configuration items given as arguments to the plugin are keys and values
+separated by spaces. Note that this means any DNs or values with spaces will
+be effectively broken.
+
+The only configuration item which is required is 'ldap_base'. This tells the
+plugin what your base DN is. The plugin will not work until it has been
+configured.
-The configuration files 'ldap_host' and 'ldap_port' specify the host and port
+The configuration items 'ldap_host' and 'ldap_port' specify the host and port
at which your Directory server may be contacted. If these are not specified,
the plugin will use port '389' on 'localhost'.
-The configuration file 'ldap_timeout' specifies how long the plugin should
+The configuration item 'ldap_timeout' specifies how long the plugin should
wait for a response from your Directory server. By default, the value is 5
seconds.
-The configuration file 'ldap_auth_filter_attr' specifies how the plugin should
+The configuration item 'ldap_auth_filter_attr' specifies how the plugin should
find the user in your Directory. By default, the plugin will look up the user
based on the 'uid' attribute.