On 10/09/12 17:07, Gilles Chehade wrote:
On Tue, Oct 09, 2012 at 03:48:44PM +0200, Gilles Chehade wrote:
On Tue, Oct 09, 2012 at 03:43:03PM +0200, Alexander Hall wrote:
On 10/09/12 15:33, Gilles Chehade wrote:
Argh, you should have talked to me first ...
Both require ssl and require auth are implemented already ... I did
not commit yet because we stabilized a release and decided to not
add new features to it unless they are critical.
This feature should be committed in a few days
well well, I got the pleasure of pretending to be a real hacker
anyway... ;-)
Actually, you're diff has a nice idea regarding the enable|require part
which we'll incorporate in my diff, so you didn't hack for nothing :-)
\o/
The following diff is what I intend to commit tonight with an ok from
eric@. It applies on -current, but beware as it kills the "enable"
keyword:
listen on bnx0 [...] auth # enable auth
listen on bnx0 [...] auth-require # require auth
I thought 'enable auth' and 'require auth' were more readable, but I
could live with this too (now, should we end up having the non-enforcing
variant).
diff --git a/parse.y b/parse.y
index 8df8521..f0917c1 100644
--- a/parse.y
+++ b/parse.y
@@ -123,8 +123,9 @@ typedef struct {
%token MAP HASH LIST SINGLE SSL SMTPS CERTIFICATE ENCRYPTION
%token DB LDAP PLAIN DOMAIN SOURCE
%token RELAY BACKUP VIA DELIVER TO MAILDIR MBOX HOSTNAME
-%token ACCEPT REJECT INCLUDE ERROR MDA FROM FOR
-%token ARROW ENABLE AUTH TLS LOCAL VIRTUAL TAG ALIAS FILTER KEY DIGEST
+%token ACCEPT REJECT INCLUDE ERROR MDA FROM FOR SSLONLY AUTHONLY
+%token ARROW AUTH TLS LOCAL VIRTUAL TAG ALIAS FILTER KEY DIGEST
+%token AUTH_REQUIRE TLS_REQUIRE
%token <v.string> STRING
%token <v.number> NUMBER
%type <v.map> map
@@ -260,10 +261,12 @@ certname : CERTIFICATE STRING {
ssl : SMTPS { $$ = F_SMTPS; }
| TLS { $$ = F_STARTTLS; }
| SSL { $$ = F_SSL; }
- | /* empty */ { $$ = 0; }
+ | TLS_REQUIRE { $$ =
F_STARTTLS|F_STARTTLS_REQUIRE; }
+ | /* Empty */ { $$ = 0; }
;
-auth : ENABLE AUTH { $$ = 1; }
+auth : AUTH { $$ = F_AUTH; }
+ | AUTH_REQUIRE { $$ = F_AUTH|F_AUTH_REQUIRE; }
| /* empty */ { $$ = 0; }
;
@@ -367,7 +370,7 @@ main : QUEUE INTERVAL interval {
flags = $5;
if ($7)
- flags |= F_AUTH;
+ flags |= $7;
The if statement is pretty pointless. Together with the prior line:
flags = $5 | $7;
if ($5 && ssl_load_certfile(cert, F_SCERT) < 0) {
yyerror("cannot load certificate: %s", cert);
@@ -940,6 +943,7 @@ lookup(char *s)
{ "all", ALL },
{ "as", AS },
{ "auth", AUTH },
+ { "auth-require", AUTH_REQUIRE },
{ "backup", BACKUP },
{ "certificate", CERTIFICATE },
{ "cipher", CIPHER },
@@ -948,7 +952,6 @@ lookup(char *s)
{ "deliver", DELIVER },
{ "digest", DIGEST },
{ "domain", DOMAIN },
- { "enable", ENABLE },
{ "encryption", ENCRYPTION },
{ "expire", EXPIRE },
{ "filter", FILTER },
@@ -980,6 +983,7 @@ lookup(char *s)
{ "ssl", SSL },
{ "tag", TAG },
{ "tls", TLS },
+ { "tls-require", TLS_REQUIRE },
{ "to", TO },
{ "via", VIA },
{ "virtual", VIRTUAL },
diff --git a/smtp_session.c b/smtp_session.c
index a7b0d30..4c65159 100644
--- a/smtp_session.c
+++ b/smtp_session.c
@@ -400,6 +400,19 @@ session_rfc5321_mail_handler(struct session *s, char *args)
return 1;
}
+
+ if (s->s_l->flags & F_STARTTLS_REQUIRE)
+ if (!(s->s_flags & F_SECURE)) {
+ session_respond(s, "530 5.7.0 Must issue a STARTTLS command
first");
long line
+ return 1;
+ }
+
+ if (s->s_l->flags & F_AUTH_REQUIRE)
+ if (!(s->s_flags & F_AUTHENTICATED)) {
+ session_respond(s, "530 5.7.0 Must issue a AUTH command
first");
long line
maybe "_an_ AUTH command"
also, is the nested if(), in both cases above, for style reasons?
+ return 1;
+ }
+
if (s->s_state != S_HELO) {
session_respond(s, "503 5.5.1 Sender already specified");
return 1;
diff --git a/smtpd.conf.5 b/smtpd.conf.5
index c4ad738..a070a8d 100644
--- a/smtpd.conf.5
+++ b/smtpd.conf.5
@@ -53,7 +53,7 @@ For example:
.Bd -literal -offset indent
wan_if = "fxp0"
listen on $wan_if
-listen on $wan_if tls enable auth
+listen on $wan_if tls auth
.Ed
.Pp
Some configuration directives expect expansion of their parameters at runtime.
@@ -99,9 +99,9 @@ as returned by
.It Xo
.Ic listen on Ar interface
.Op Ic port Ar port
-.Op Ic tls | smtps
+.Op Ic tls | tls-require | smtps
.Op Ic certificate Ar name
-.Op Ic enable auth
+.Op Ic auth | auth-require
.Xc
Specify an
.Ar interface
@@ -118,6 +118,9 @@ by default on port 25,
or SMTPS
.Pq Ic smtps ,
by default on port 465.
+.Ar tls-require
+may be used to force client to establish a secure connection
+before being allowed to start a SMTP transaction.
Host certificates may be used for these connections,
and are searched for in the
.Pa /etc/mail/certs
@@ -150,11 +153,15 @@ Creation of certificates is documented in
.Xr starttls 8 .
.Pp
If the
-.Ic enable auth
+.Ic auth
parameter is used,
any remote sender that passed SMTPAUTH is treated as if
it was the server's local user that was sending the mail.
This means that filter rules using "from local" will be matched.
+If
+.Ic auth-require
+is specified, then a client may only start a SMTP transaction
+after a successful authentication.
.It Xo
.Ic map Ar map
.Ic source Ar type Ar source
@@ -468,7 +475,7 @@ a certificate valid for one year was created.
The configuration file would look like this:
.Bd -literal -offset indent
listen on lo0
-listen on bnx0 tls certificate mail.example.com enable auth
+listen on bnx0 tls certificate mail.example.com auth
map aliases source db "/etc/mail/aliases.db"
accept for local deliver to mda "/path/to/mda -f -"
accept from all for domain example.org \e
diff --git a/smtpd.h b/smtpd.h
index 37959ed..754798b 100644
--- a/smtpd.h
+++ b/smtpd.h
@@ -80,8 +80,10 @@
#define F_SMTPS 0x02
#define F_AUTH 0x04
#define F_SSL (F_SMTPS|F_STARTTLS)
+#define F_STARTTLS_REQUIRE 0x08
+#define F_AUTH_REQUIRE 0x10
-#define F_BACKUP 0x10 /* XXX */
+#define F_BACKUP 0x20 /* XXX */
#define F_SCERT 0x01
#define F_CCERT 0x02
Other than that, reads fine.
/Alexander
