Discussion:
OpenSMTPD and mask-source flag.
Peter Bisroev
2016-02-09 01:32:31 UTC
Permalink
Dear OpenSMTPD Developers!

I think there is a little "bug/feature" with respect to handling "mask-source"
parameter with "listen on" directive in smtpd.conf. The behavior makes perfect
sense from the perspective of documentation. Unfortunately it is not uniform
from the perspective of the client.

Basically, if the message is handled through smtp_accept() in smtp.c then
everything works as expected. If the message is enqueued from localhost (for
example through sendmail), the message is handled by smtp_enqueue() in smtp.c,
and at this point the behavior starts to get non-uniform. The local listener
is created upon the first invocation of smtp_enqeue(), and the listener->flags
are set to 0. Because there is no way to specify the flags for the "local"
listener, the behavior is not intuative if the client specified "mask-source"
option for the "listen on" loopback interface directive. I suspect that if
mask-source is on for lo0, the client would probably want the same behavior
for the messages submited through the local queue.

I think this situation can be handled in one of three ways. A new special
directive can be created to influence the creation of this listener, however,
this feels a little messy. Another approach, is to create support for
"listen on local" or similar, which makes everything a bit more uniform. And
the last approach could mimick what "listen on lo0" or any other loopback
device does and apply the same policy to the "local" listener. For example,
this can be implemented by searching through env->sc_listeners for loopback
listeners, and if any of them have F_MASK_SOURCE flag, apply it to the local
listener as well.

What are your thoughts? I am more than happy to provide the patch for any of
the above described scenarios assuming they would provide an adequate solution.

Thank you for your time!

Regards,
--peter
Gilles Chehade
2016-02-09 07:38:54 UTC
Permalink
Post by Peter Bisroev
Dear OpenSMTPD Developers!
Dear Peter,
Post by Peter Bisroev
I think there is a little "bug/feature" with respect to handling "mask-source"
parameter with "listen on" directive in smtpd.conf. The behavior makes perfect
sense from the perspective of documentation. Unfortunately it is not uniform
from the perspective of the client.
You are right, the feature will apply to network connections only.
Post by Peter Bisroev
Basically, if the message is handled through smtp_accept() in smtp.c then
everything works as expected. If the message is enqueued from localhost (for
example through sendmail), the message is handled by smtp_enqueue() in smtp.c,
[...]
option for the "listen on" loopback interface directive. I suspect that if
mask-source is on for lo0, the client would probably want the same behavior
for the messages submited through the local queue.
You are correct.
Post by Peter Bisroev
I think this situation can be handled in one of three ways. A new special
directive can be created to influence the creation of this listener, however,
this feels a little messy. Another approach, is to create support for
"listen on local" or similar, which makes everything a bit more uniform. And
the last approach could mimick what "listen on lo0" or any other loopback
device does and apply the same policy to the "local" listener. For example,
this can be implemented by searching through env->sc_listeners for loopback
listeners, and if any of them have F_MASK_SOURCE flag, apply it to the local
listener as well.
What are your thoughts? I am more than happy to provide the patch for any of
the above described scenarios assuming they would provide an adequate solution.
We have faced a similar issue with filters and my thoughts are that we need a
listen on socket of some kind, similar to your listen on local.

This has several benefits over "listen on local", both in ambiguity and it
new ways the ruleset can match sessions.

If you're interested to work on it, I'd be happy to discuss this with you
so you can come up with a diff :-)
--
Gilles Chehade

https://www.poolp.org @poolpOrg
Peter Bisroev
2016-02-09 14:23:17 UTC
Permalink
Hi Gilles

<snip>
Post by Gilles Chehade
We have faced a similar issue with filters and my thoughts are that we need a
listen on socket of some kind, similar to your listen on local.
This has several benefits over "listen on local", both in ambiguity and it
new ways the ruleset can match sessions.
If you're interested to work on it, I'd be happy to discuss this with you
so you can come up with a diff :-)
I would definitely be interested. Could you elaborate a bit on "listen
on socket" method?

Thank you!
--peter
Gilles Chehade
2016-02-09 14:40:16 UTC
Permalink
Post by Peter Bisroev
Hi Gilles
Hi,
Post by Peter Bisroev
<snip>
Post by Gilles Chehade
We have faced a similar issue with filters and my thoughts are that we need a
listen on socket of some kind, similar to your listen on local.
This has several benefits over "listen on local", both in ambiguity and it
new ways the ruleset can match sessions.
If you're interested to work on it, I'd be happy to discuss this with you
so you can come up with a diff :-)
I would definitely be interested. Could you elaborate a bit on "listen
on socket" method?
Well your idea for a "listen on local" is very valid and the way to go
in my opinion, however the keyword "local" is inappropriate here as we
have a very different meaning for it: a message coming from an address
that is also the address of a listener.

A network connection from 127.0.0.1 or 192.168.1.1 is local on my box,
even though it's not coming from the socket.

If we introduced a "listen on socket" of some sort, then we could have
params applied to the socket + possibly add support for multiple ones,
we could also differentiate between local sessions initiated on the
machine and local sessions initiated on the network to apply a finer
ruleset matching.

This would also solve a problem we have with filters where they are
applied to network listeners but not to the local socket which requires
a specific keyword (ok for now since they are experimental).
Post by Peter Bisroev
Thank you!
--peter
--
Gilles Chehade

https://www.poolp.org @poolpOrg
Peter Bisroev
2016-02-09 18:10:42 UTC
Permalink
Post by Gilles Chehade
Well your idea for a "listen on local" is very valid and the way to go
in my opinion, however the keyword "local" is inappropriate here as we
have a very different meaning for it: a message coming from an address
that is also the address of a listener.
A network connection from 127.0.0.1 or 192.168.1.1 is local on my box,
even though it's not coming from the socket.
If we introduced a "listen on socket" of some sort, then we could have
params applied to the socket + possibly add support for multiple ones,
we could also differentiate between local sessions initiated on the
machine and local sessions initiated on the network to apply a finer
ruleset matching.
That makes sense. So basically a new directive is added to be
supported in smtpd.conf, eg "listen on socket". Then, if this
directive is not present, then smtpd will not accept submission of
smtp messages through local socket connection that come through
smtp_enqueue(), right? Or do we make this enabled by default since
most likely it would be needed by system services that user mail/mailx
commands and this directive becomes the configuration of the local
socket listener?

As far as the name goes, maybe "listen on socket" sounds good from the
client perspective, it makes it evident to the client that they are
not configuring a network interface. Would it be a problem on the
developer side though? In general, I feel that client usability takes
priority here, and this can be nicely documented and described in the
man page.
Post by Gilles Chehade
This would also solve a problem we have with filters where they are
applied to network listeners but not to the local socket which requires
a specific keyword (ok for now since they are experimental).
That would be cool, so if this new local socket listener is created as
part of create_listener() code for example, the filter code should
just work as well without any changes (of course I could be mistaken,
have not looked in the filter support code yet).
Post by Gilles Chehade
Gilles Chehade
Thanks,
--peter
Peter Bisroev
2016-02-11 22:28:50 UTC
Permalink
Hi Gilles,

Please find my diff inline to enable "listen on socket" feature that we have
discussed. I have tested the diff with currently two supported listen options
for this listener, mask-sender and filter. Everything seems to be working OK.

These are the summary of the changes:
* Parser was adjusted to support "listen on socket", should be easier to
extend to new listener types.
* Reusing config_listener().
* struct smtpd has a new member sc_sock_listener to be used by smtp_enqueu()
* If "listen on socket" was not specified in the config file, a default
socket listener will be created in parse_config()
* I also removed "struct listener l" from parse.y as it was not being used
anywhere.
* I have tried to stick to the coding style in the source, and tried to keep
the diff as small as possible. Please let me know if something is off and
I will fix it.

If these changes look OK to you, I will create a diff for the man page and
send it through.

Thank you!
--peter



Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.181
diff -u -p -r1.181 parse.y
--- parse.y 18 Jan 2016 09:19:41 -0000 1.181
+++ parse.y 11 Feb 2016 20:59:11 -0000
@@ -97,7 +97,6 @@ static int errors = 0;
struct filter_conf *filter = NULL;
struct table *table = NULL;
struct rule *rule = NULL;
-struct listener l;
struct mta_limits *limits;
static struct pki *pki;
static struct ca *sca;
@@ -139,8 +138,9 @@ static struct listen_opts {
uint32_t options;
} listen_opts;

-static void create_listener(struct listenerlist *, struct listen_opts *);
-static void config_listener(struct listener *, struct listen_opts *);
+static struct listener *create_sock_listener(struct listen_opts *);
+static void create_if_listener(struct listenerlist *, struct listen_opts *);
+static void config_listener(struct listener *, struct listen_opts *);

struct listener *host_v4(const char *, in_port_t);
struct listener *host_v6(const char *, in_port_t);
@@ -156,6 +156,9 @@ static struct filter_conf *create_filter
static struct filter_conf *create_filter_chain(char *);
static int add_filter_arg(struct filter_conf *, char *);

+static int config_lo_filter(struct listen_opts *, char *);
+static int config_lo_mask_source(struct listen_opts *);
+
typedef struct {
union {
int64_t number;
@@ -175,7 +178,7 @@ typedef struct {
%token ACCEPT REJECT INCLUDE ERROR MDA FROM FOR SOURCE MTA PKI SCHEDULER
%token ARROW AUTH TLS LOCAL VIRTUAL TAG TAGGED ALIAS FILTER KEY CA DHPARAMS
%token AUTH_OPTIONAL TLS_REQUIRE USERBASE SENDER SENDERS MASK_SOURCE VERIFY FORWARDONLY RECIPIENT
-%token CIPHERS RECEIVEDAUTH MASQUERADE ENQUEUER
+%token CIPHERS RECEIVEDAUTH MASQUERADE SOCKET
%token <v.string> STRING
%token <v.number> NUMBER
%type <v.table> table
@@ -403,7 +406,19 @@ pki : opt_pki pki
| /* empty */
;

-opt_listen : INET4 {
+opt_sock_listen : FILTER STRING {
+ if (config_lo_filter(&listen_opts, $2)) {
+ YYERROR;
+ }
+ }
+ | MASK_SOURCE {
+ if (config_lo_mask_source(&listen_opts)) {
+ YYERROR;
+ }
+ }
+ ;
+
+opt_if_listen : INET4 {
if (listen_opts.options & LO_FAMILY) {
yyerror("address family already specified");
YYERROR;
@@ -451,13 +466,10 @@ opt_listen : INET4 {
listen_opts.port = $2;
}
| FILTER STRING {
- if (listen_opts.options & LO_FILTER) {
- yyerror("filter already specified");
+ if (config_lo_filter(&listen_opts, $2)) {
YYERROR;
}
- listen_opts.options |= LO_FILTER;
- listen_opts.filtername = $2;
- }
+}
| SMTPS {
if (listen_opts.options & LO_SSL) {
yyerror("TLS mode already specified");
@@ -596,12 +608,9 @@ opt_listen : INET4 {
listen_opts.hostnametable = t;
}
| MASK_SOURCE {
- if (listen_opts.options & LO_MASKSOURCE) {
- yyerror("mask-source already specified");
+ if (config_lo_mask_source(&listen_opts)) {
YYERROR;
}
- listen_opts.options |= LO_MASKSOURCE;
- listen_opts.flags |= F_MASK_SOURCE;
}
| RECEIVEDAUTH {
if (listen_opts.options & LO_RECEIVEDAUTH) {
@@ -653,7 +662,33 @@ opt_listen : INET4 {
}
;

-listen : opt_listen listen
+listener_type : socket_listener
+ | interface_listener
+ ;
+
+socket_listener : SOCKET sock_listen {
+ if (conf->sc_sock_listener) {
+ yyerror("socket listener already configured");
+ YYERROR;
+ }
+ conf->sc_sock_listener = create_sock_listener(&listen_opts);
+ }
+ ;
+
+interface_listener:
+ STRING if_listen {
+ listen_opts.family = AF_UNSPEC;
+ listen_opts.flags |= F_EXT_DSN;
+ listen_opts.ifx = $1;
+ create_if_listener(conf->sc_listeners, &listen_opts);
+ }
+ ;
+
+sock_listen : opt_sock_listen sock_listen
+ | /* empty */
+ ;
+
+if_listen : opt_if_listen if_listen
| /* empty */
;

@@ -846,28 +881,8 @@ main : BOUNCEWARN {
} limits_mta
| LIMIT SCHEDULER limits_scheduler
| LISTEN {
- memset(&l, 0, sizeof l);
memset(&listen_opts, 0, sizeof listen_opts);
- listen_opts.family = AF_UNSPEC;
- listen_opts.flags |= F_EXT_DSN;
- } ON STRING listen {
- listen_opts.ifx = $4;
- create_listener(conf->sc_listeners, &listen_opts);
- }
- | ENQUEUER FILTER STRING {
- if (dict_get(&conf->sc_filters, $3) == NULL) {
- yyerror("undefined filter \"%s\"", $3);
- free($3);
- YYERROR;
- }
- if (strlcpy(conf->sc_enqueue_filter, $3,
- sizeof conf->sc_enqueue_filter)
- >= sizeof conf->sc_enqueue_filter) {
- free($3);
- YYERROR;
- }
- free($3);
- }
+ } ON listener_type
| FILTER STRING STRING {
if (!strcmp($3, "chain")) {
free($3);
@@ -1449,7 +1464,6 @@ lookup(char *s)
{ "dhparams", DHPARAMS },
{ "domain", DOMAIN },
{ "encryption", ENCRYPTION },
- { "enqueuer", ENQUEUER },
{ "expire", EXPIRE },
{ "filter", FILTER },
{ "for", FOR },
@@ -1489,6 +1503,7 @@ lookup(char *s)
{ "senders", SENDERS },
{ "session", SESSION },
{ "smtps", SMTPS },
+ { "socket", SOCKET },
{ "source", SOURCE },
{ "table", TABLE },
{ "tag", TAG },
@@ -1945,6 +1960,12 @@ parse_config(struct smtpd *x_conf, const
popfile();
endservent();

+ /* If the socket listener was not configured, create a default one. */
+ if (!conf->sc_sock_listener) {
+ memset(&listen_opts, 0, sizeof listen_opts);
+ conf->sc_sock_listener = create_sock_listener(&listen_opts);
+ }
+
/* Free macros and check which have not been used. */
for (sym = TAILQ_FIRST(&symhead); sym != NULL; sym = next) {
next = TAILQ_NEXT(sym, entry);
@@ -2046,8 +2067,21 @@ symget(const char *nam)
return (NULL);
}

+static struct listener *
+create_sock_listener(struct listen_opts *lo)
+{
+ struct listener *l = xcalloc(1, sizeof(*l), "create_sock_listener");
+ lo->tag = "local";
+ lo->hostname = conf->sc_hostname;
+ l->ss.ss_family = AF_LOCAL;
+ l->ss.ss_len = sizeof(struct sockaddr *);
+ config_listener(l, lo);
+
+ return (l);
+}
+
static void
-create_listener(struct listenerlist *ll, struct listen_opts *lo)
+create_if_listener(struct listenerlist *ll, struct listen_opts *lo)
{
uint16_t flags;

@@ -2560,3 +2594,28 @@ add_filter_arg(struct filter_conf *f, ch

return (1);
}
+
+static int
+config_lo_filter(struct listen_opts *lo, char *filter_name) {
+ if (lo->options & LO_FILTER) {
+ yyerror("filter already specified");
+ return -1;
+ }
+ lo->options |= LO_FILTER;
+ lo->filtername = filter_name;
+
+ return 0;
+}
+
+static int
+config_lo_mask_source(struct listen_opts *lo) {
+ if (lo->options & LO_MASKSOURCE) {
+ yyerror("mask-source already specified");
+ return -1;
+ }
+ lo->options |= LO_MASKSOURCE;
+ lo->flags |= F_MASK_SOURCE;
+
+ return 0;
+}
+
Index: smtp.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtp.c,v
retrieving revision 1.152
diff -u -p -r1.152 smtp.c
--- smtp.c 8 Jan 2016 21:31:06 -0000 1.152
+++ smtp.c 11 Feb 2016 20:59:11 -0000
@@ -219,20 +219,9 @@ smtp_resume(void)
static int
smtp_enqueue(uid_t *euid)
{
- static struct listener local, *listener = NULL;
- char buf[HOST_NAME_MAX+1], *hostname;
- int fd[2];
-
- if (listener == NULL) {
- listener = &local;
- (void)strlcpy(listener->tag, "local", sizeof(listener->tag));
- listener->ss.ss_family = AF_LOCAL;
- listener->ss.ss_len = sizeof(struct sockaddr *);
- (void)strlcpy(listener->hostname, env->sc_hostname,
- sizeof(listener->hostname));
- (void)strlcpy(listener->filter, env->sc_enqueue_filter,
- sizeof listener->filter);
- }
+ struct listener *listener = env->sc_sock_listener;
+ char buf[HOST_NAME_MAX+1], *hostname;
+ int fd[2];

/*
* Some enqueue requests buffered in IMSG may still arrive even after
Index: smtpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.511
diff -u -p -r1.511 smtpd.h
--- smtpd.h 5 Feb 2016 19:21:04 -0000 1.511
+++ smtpd.h 11 Feb 2016 20:59:11 -0000
@@ -613,6 +613,9 @@ struct smtpd {

time_t sc_uptime;

+ /* This is a listener for a local socket used by smtp_enqueue(). */
+ struct listener *sc_sock_listener;
+
TAILQ_HEAD(listenerlist, listener) *sc_listeners;

TAILQ_HEAD(rulelist, rule) *sc_rules;
Gilles Chehade
2016-02-12 08:31:59 UTC
Permalink
Post by Peter Bisroev
Hi Gilles,
Please find my diff inline to enable "listen on socket" feature that we have
discussed. I have tested the diff with currently two supported listen options
for this listener, mask-sender and filter. Everything seems to be working OK.
* Parser was adjusted to support "listen on socket", should be easier to
extend to new listener types.
* Reusing config_listener().
* struct smtpd has a new member sc_sock_listener to be used by smtp_enqueu()
* If "listen on socket" was not specified in the config file, a default
socket listener will be created in parse_config()
* I also removed "struct listener l" from parse.y as it was not being used
anywhere.
* I have tried to stick to the coding style in the source, and tried to keep
the diff as small as possible. Please let me know if something is off and
I will fix it.
If these changes look OK to you, I will create a diff for the man page and
send it through.
I only skimmed through your diff, I need to apply it and read in
context but I like it a lot.

I'll test today and come back with comments once I've spent more
time reading it ;-)

Thanks
Post by Peter Bisroev
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.181
diff -u -p -r1.181 parse.y
--- parse.y 18 Jan 2016 09:19:41 -0000 1.181
+++ parse.y 11 Feb 2016 20:59:11 -0000
@@ -97,7 +97,6 @@ static int errors = 0;
struct filter_conf *filter = NULL;
struct table *table = NULL;
struct rule *rule = NULL;
-struct listener l;
struct mta_limits *limits;
static struct pki *pki;
static struct ca *sca;
@@ -139,8 +138,9 @@ static struct listen_opts {
uint32_t options;
} listen_opts;
-static void create_listener(struct listenerlist *, struct listen_opts *);
-static void config_listener(struct listener *, struct listen_opts *);
+static struct listener *create_sock_listener(struct listen_opts *);
+static void create_if_listener(struct listenerlist *, struct listen_opts *);
+static void config_listener(struct listener *, struct listen_opts *);
struct listener *host_v4(const char *, in_port_t);
struct listener *host_v6(const char *, in_port_t);
@@ -156,6 +156,9 @@ static struct filter_conf *create_filter
static struct filter_conf *create_filter_chain(char *);
static int add_filter_arg(struct filter_conf *, char *);
+static int config_lo_filter(struct listen_opts *, char *);
+static int config_lo_mask_source(struct listen_opts *);
+
typedef struct {
union {
int64_t number;
@@ -175,7 +178,7 @@ typedef struct {
%token ACCEPT REJECT INCLUDE ERROR MDA FROM FOR SOURCE MTA PKI SCHEDULER
%token ARROW AUTH TLS LOCAL VIRTUAL TAG TAGGED ALIAS FILTER KEY CA DHPARAMS
%token AUTH_OPTIONAL TLS_REQUIRE USERBASE SENDER SENDERS MASK_SOURCE VERIFY FORWARDONLY RECIPIENT
-%token CIPHERS RECEIVEDAUTH MASQUERADE ENQUEUER
+%token CIPHERS RECEIVEDAUTH MASQUERADE SOCKET
%token <v.string> STRING
%token <v.number> NUMBER
%type <v.table> table
@@ -403,7 +406,19 @@ pki : opt_pki pki
| /* empty */
;
-opt_listen : INET4 {
+opt_sock_listen : FILTER STRING {
+ if (config_lo_filter(&listen_opts, $2)) {
+ YYERROR;
+ }
+ }
+ | MASK_SOURCE {
+ if (config_lo_mask_source(&listen_opts)) {
+ YYERROR;
+ }
+ }
+ ;
+
+opt_if_listen : INET4 {
if (listen_opts.options & LO_FAMILY) {
yyerror("address family already specified");
YYERROR;
@@ -451,13 +466,10 @@ opt_listen : INET4 {
listen_opts.port = $2;
}
| FILTER STRING {
- if (listen_opts.options & LO_FILTER) {
- yyerror("filter already specified");
+ if (config_lo_filter(&listen_opts, $2)) {
YYERROR;
}
- listen_opts.options |= LO_FILTER;
- listen_opts.filtername = $2;
- }
+}
| SMTPS {
if (listen_opts.options & LO_SSL) {
yyerror("TLS mode already specified");
@@ -596,12 +608,9 @@ opt_listen : INET4 {
listen_opts.hostnametable = t;
}
| MASK_SOURCE {
- if (listen_opts.options & LO_MASKSOURCE) {
- yyerror("mask-source already specified");
+ if (config_lo_mask_source(&listen_opts)) {
YYERROR;
}
- listen_opts.options |= LO_MASKSOURCE;
- listen_opts.flags |= F_MASK_SOURCE;
}
| RECEIVEDAUTH {
if (listen_opts.options & LO_RECEIVEDAUTH) {
@@ -653,7 +662,33 @@ opt_listen : INET4 {
}
;
-listen : opt_listen listen
+listener_type : socket_listener
+ | interface_listener
+ ;
+
+socket_listener : SOCKET sock_listen {
+ if (conf->sc_sock_listener) {
+ yyerror("socket listener already configured");
+ YYERROR;
+ }
+ conf->sc_sock_listener = create_sock_listener(&listen_opts);
+ }
+ ;
+
+ STRING if_listen {
+ listen_opts.family = AF_UNSPEC;
+ listen_opts.flags |= F_EXT_DSN;
+ listen_opts.ifx = $1;
+ create_if_listener(conf->sc_listeners, &listen_opts);
+ }
+ ;
+
+sock_listen : opt_sock_listen sock_listen
+ | /* empty */
+ ;
+
+if_listen : opt_if_listen if_listen
| /* empty */
;
@@ -846,28 +881,8 @@ main : BOUNCEWARN {
} limits_mta
| LIMIT SCHEDULER limits_scheduler
| LISTEN {
- memset(&l, 0, sizeof l);
memset(&listen_opts, 0, sizeof listen_opts);
- listen_opts.family = AF_UNSPEC;
- listen_opts.flags |= F_EXT_DSN;
- } ON STRING listen {
- listen_opts.ifx = $4;
- create_listener(conf->sc_listeners, &listen_opts);
- }
- | ENQUEUER FILTER STRING {
- if (dict_get(&conf->sc_filters, $3) == NULL) {
- yyerror("undefined filter \"%s\"", $3);
- free($3);
- YYERROR;
- }
- if (strlcpy(conf->sc_enqueue_filter, $3,
- sizeof conf->sc_enqueue_filter)
- >= sizeof conf->sc_enqueue_filter) {
- free($3);
- YYERROR;
- }
- free($3);
- }
+ } ON listener_type
| FILTER STRING STRING {
if (!strcmp($3, "chain")) {
free($3);
@@ -1449,7 +1464,6 @@ lookup(char *s)
{ "dhparams", DHPARAMS },
{ "domain", DOMAIN },
{ "encryption", ENCRYPTION },
- { "enqueuer", ENQUEUER },
{ "expire", EXPIRE },
{ "filter", FILTER },
{ "for", FOR },
@@ -1489,6 +1503,7 @@ lookup(char *s)
{ "senders", SENDERS },
{ "session", SESSION },
{ "smtps", SMTPS },
+ { "socket", SOCKET },
{ "source", SOURCE },
{ "table", TABLE },
{ "tag", TAG },
@@ -1945,6 +1960,12 @@ parse_config(struct smtpd *x_conf, const
popfile();
endservent();
+ /* If the socket listener was not configured, create a default one. */
+ if (!conf->sc_sock_listener) {
+ memset(&listen_opts, 0, sizeof listen_opts);
+ conf->sc_sock_listener = create_sock_listener(&listen_opts);
+ }
+
/* Free macros and check which have not been used. */
for (sym = TAILQ_FIRST(&symhead); sym != NULL; sym = next) {
next = TAILQ_NEXT(sym, entry);
@@ -2046,8 +2067,21 @@ symget(const char *nam)
return (NULL);
}
+static struct listener *
+create_sock_listener(struct listen_opts *lo)
+{
+ struct listener *l = xcalloc(1, sizeof(*l), "create_sock_listener");
+ lo->tag = "local";
+ lo->hostname = conf->sc_hostname;
+ l->ss.ss_family = AF_LOCAL;
+ l->ss.ss_len = sizeof(struct sockaddr *);
+ config_listener(l, lo);
+
+ return (l);
+}
+
static void
-create_listener(struct listenerlist *ll, struct listen_opts *lo)
+create_if_listener(struct listenerlist *ll, struct listen_opts *lo)
{
uint16_t flags;
@@ -2560,3 +2594,28 @@ add_filter_arg(struct filter_conf *f, ch
return (1);
}
+
+static int
+config_lo_filter(struct listen_opts *lo, char *filter_name) {
+ if (lo->options & LO_FILTER) {
+ yyerror("filter already specified");
+ return -1;
+ }
+ lo->options |= LO_FILTER;
+ lo->filtername = filter_name;
+
+ return 0;
+}
+
+static int
+config_lo_mask_source(struct listen_opts *lo) {
+ if (lo->options & LO_MASKSOURCE) {
+ yyerror("mask-source already specified");
+ return -1;
+ }
+ lo->options |= LO_MASKSOURCE;
+ lo->flags |= F_MASK_SOURCE;
+
+ return 0;
+}
+
Index: smtp.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtp.c,v
retrieving revision 1.152
diff -u -p -r1.152 smtp.c
--- smtp.c 8 Jan 2016 21:31:06 -0000 1.152
+++ smtp.c 11 Feb 2016 20:59:11 -0000
@@ -219,20 +219,9 @@ smtp_resume(void)
static int
smtp_enqueue(uid_t *euid)
{
- static struct listener local, *listener = NULL;
- char buf[HOST_NAME_MAX+1], *hostname;
- int fd[2];
-
- if (listener == NULL) {
- listener = &local;
- (void)strlcpy(listener->tag, "local", sizeof(listener->tag));
- listener->ss.ss_family = AF_LOCAL;
- listener->ss.ss_len = sizeof(struct sockaddr *);
- (void)strlcpy(listener->hostname, env->sc_hostname,
- sizeof(listener->hostname));
- (void)strlcpy(listener->filter, env->sc_enqueue_filter,
- sizeof listener->filter);
- }
+ struct listener *listener = env->sc_sock_listener;
+ char buf[HOST_NAME_MAX+1], *hostname;
+ int fd[2];
/*
* Some enqueue requests buffered in IMSG may still arrive even after
Index: smtpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.511
diff -u -p -r1.511 smtpd.h
--- smtpd.h 5 Feb 2016 19:21:04 -0000 1.511
+++ smtpd.h 11 Feb 2016 20:59:11 -0000
@@ -613,6 +613,9 @@ struct smtpd {
time_t sc_uptime;
+ /* This is a listener for a local socket used by smtp_enqueue(). */
+ struct listener *sc_sock_listener;
+
TAILQ_HEAD(listenerlist, listener) *sc_listeners;
TAILQ_HEAD(rulelist, rule) *sc_rules;
--
Gilles Chehade

https://www.poolp.org @poolpOrg
Peter Bisroev
2016-02-12 20:53:04 UTC
Permalink
Post by Gilles Chehade
I only skimmed through your diff, I need to apply it and read in
context but I like it a lot.
I'll test today and come back with comments once I've spent more
time reading it ;-)
Thanks
Awesome, thank you Gilles!

Just in case the previous diff is OK, I am attaching the patch to the
smtpd.conf man page.

Thanks!
--peter



Index: smtpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.153
diff -u -p -r1.153 smtpd.conf.5
--- smtpd.conf.5 21 Jan 2016 23:40:27 -0000 1.153
+++ smtpd.conf.5 12 Feb 2016 20:45:00 -0000
@@ -654,6 +654,12 @@ of inflight envelopes falls below
Changing the default value might degrade performance.
.It Xo
.Bk -words
+.Ic listen on socket
+.Op Ic filter Ar name
+.Op Ic mask-source
+.Ek
+.br
+.Bk -words
.Ic listen on Ar interface
.Op Ar family
.Op Ic port Ar port
@@ -671,11 +677,25 @@ Changing the default value might degrade
.Ek
.Xc
.Pp
-Specify an
+Specify a
+.Ic socket
+or an
+.Ar interface
+to listen on for incoming connections. A
+.Ic listen on socket
+directive is used to customize listening behavior for messages submitted
+through local queues, for example, via the
+.Xr mail 1
+utility. This is an optional directive, and if not supplied,
+.Xr smtpd 8
+will listen for connections on the
+.Ic socket
+only.
+.Pp
+To listen on a specific network interface, specify an
.Ar interface
-and
-.Ar port
-to listen on.
+and an optional
+.Ar port .
An interface group, an IP address or a domain name may
be used in place of
.Ar interface .
Peter Bisroev
2016-02-12 21:29:23 UTC
Permalink
Hi Gilles,

While looking over smtp_enqueue(), I have noticed that setting of
hostname is a noop. It looks like a leftover code from a bugfix in here
(http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/smtpd/smtp.c.diff?r2=1.141&r1=1.140&f=u)

I am including a diff to smtp.c below that includes the removal of that code
(it also includes the changes to smtp.c in my first patch).

Thanks!
--peter


Index: smtp.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtp.c,v
retrieving revision 1.152
diff -u -p -r1.152 smtp.c
--- smtp.c 8 Jan 2016 21:31:06 -0000 1.152
+++ smtp.c 12 Feb 2016 20:47:54 -0000
@@ -46,7 +46,7 @@ static void smtp_setup_events(void);
static void smtp_pause(void);
static void smtp_resume(void);
static void smtp_accept(int, short, void *);
-static int smtp_enqueue(uid_t *);
+static int smtp_enqueue(void);
static int smtp_can_accept(void);
static void smtp_setup_listeners(void);
static int smtp_sni_callback(SSL *, int *, void *);
@@ -84,7 +84,7 @@ smtp_imsg(struct mproc *p, struct imsg *

case IMSG_QUEUE_SMTP_SESSION:
m_compose(p, IMSG_QUEUE_SMTP_SESSION, 0, 0,
- smtp_enqueue(NULL), imsg->data,
+ smtp_enqueue(), imsg->data,
imsg->hdr.len - sizeof imsg->hdr);
return;
}
@@ -94,7 +94,7 @@ smtp_imsg(struct mproc *p, struct imsg *
switch (imsg->hdr.type) {
case IMSG_CTL_SMTP_SESSION:
m_compose(p, IMSG_CTL_SMTP_SESSION, imsg->hdr.peerid, 0,
- smtp_enqueue(imsg->data), NULL, 0);
+ smtp_enqueue(), NULL, 0);
return;

case IMSG_CTL_PAUSE_SMTP:
@@ -217,22 +217,10 @@ smtp_resume(void)
}

static int
-smtp_enqueue(uid_t *euid)
+smtp_enqueue(void)
{
- static struct listener local, *listener = NULL;
- char buf[HOST_NAME_MAX+1], *hostname;
- int fd[2];
-
- if (listener == NULL) {
- listener = &local;
- (void)strlcpy(listener->tag, "local", sizeof(listener->tag));
- listener->ss.ss_family = AF_LOCAL;
- listener->ss.ss_len = sizeof(struct sockaddr *);
- (void)strlcpy(listener->hostname, env->sc_hostname,
- sizeof(listener->hostname));
- (void)strlcpy(listener->filter, env->sc_enqueue_filter,
- sizeof listener->filter);
- }
+ struct listener *listener = env->sc_sock_listener;
+ int fd[2];

/*
* Some enqueue requests buffered in IMSG may still arrive even after
@@ -245,13 +233,7 @@ smtp_enqueue(uid_t *euid)
if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, fd))
return (-1);

- hostname = env->sc_hostname;
- if (euid) {
- (void)snprintf(buf, sizeof(buf), "%s", hostname);
- hostname = buf;
- }
-
- if ((smtp_session(listener, fd[0], &listener->ss, hostname)) == -1) {
+ if ((smtp_session(listener, fd[0], &listener->ss, env->sc_hostname)) == -1) {
close(fd[0]);
close(fd[1]);
return (-1);
Gilles Chehade
2016-02-13 09:30:29 UTC
Permalink
Post by Peter Bisroev
Hi Gilles,
Hi,
Post by Peter Bisroev
While looking over smtp_enqueue(), I have noticed that setting of
hostname is a noop. It looks like a leftover code from a bugfix in here
(http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.sbin/smtpd/smtp.c.diff?r2=1.141&r1=1.140&f=u)
I am including a diff to smtp.c below that includes the removal of that code
(it also includes the changes to smtp.c in my first patch).
Yes, it's a leftover from a cleanup to remove the euid from headers,
it's a noop indeed.

I've adapted to -current so it can be applied without your other diff
(which is still pending review) and committed just a minute ago.

Thanks !
--
Gilles Chehade

https://www.poolp.org @poolpOrg
Peter Bisroev
2016-02-12 22:00:59 UTC
Permalink
Post by Peter Bisroev
Just in case the previous diff is OK, I am attaching the patch to the
smtpd.conf man page.
Hi Gilles,

I apologize, my previous manpage diff did not include the information regarding
the fact that connections through local socket will always be tagged 'local'.

Please find the corrected manpage diff below.

Thank you!
--peter


Index: smtpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.153
diff -u -p -r1.153 smtpd.conf.5
--- smtpd.conf.5 21 Jan 2016 23:40:27 -0000 1.153
+++ smtpd.conf.5 12 Feb 2016 21:56:51 -0000
@@ -654,6 +654,12 @@ of inflight envelopes falls below
Changing the default value might degrade performance.
.It Xo
.Bk -words
+.Ic listen on socket
+.Op Ic filter Ar name
+.Op Ic mask-source
+.Ek
+.br
+.Bk -words
.Ic listen on Ar interface
.Op Ar family
.Op Ic port Ar port
@@ -671,11 +677,28 @@ Changing the default value might degrade
.Ek
.Xc
.Pp
-Specify an
+Specify a
+.Ic socket
+or an
+.Ar interface
+to listen on for incoming connections. A
+.Ic listen on socket
+directive is used to customize listening behavior for messages submitted
+through local queues, for example, via the
+.Xr mail 1
+utility. This is an optional directive, and if not supplied,
+.Xr smtpd 8
+will listen for connections on the
+.Ic socket
+only. Clients connecting through the
+.Ic socket
+will always be tagged with the 'local'
+.Ic tag .
+.Pp
+To listen on a specific network interface, specify an
.Ar interface
-and
-.Ar port
-to listen on.
+and an optional
+.Ar port .
An interface group, an IP address or a domain name may
be used in place of
.Ar interface .
Joerg Jung
2016-02-13 19:32:23 UTC
Permalink
Post by Peter Bisroev
Post by Peter Bisroev
Just in case the previous diff is OK, I am attaching the patch to the
smtpd.conf man page.
Hi Gilles,
I apologize, my previous manpage diff did not include the information regarding
the fact that connections through local socket will always be tagged 'local'.
Please find the corrected manpage diff below.
Some minor comments inline below.
Post by Peter Bisroev
Thank you!
--peter
Index: smtpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.153
diff -u -p -r1.153 smtpd.conf.5
--- smtpd.conf.5 21 Jan 2016 23:40:27 -0000 1.153
+++ smtpd.conf.5 12 Feb 2016 21:56:51 -0000
@@ -654,6 +654,12 @@ of inflight envelopes falls below
Changing the default value might degrade performance.
.It Xo
.Bk -words
+.Ic listen on socket
+.Op Ic filter Ar name
+.Op Ic mask-source
+.Ek
+.br
+.Bk -words
.Ic listen on Ar interface
.Op Ar family
.Op Ic port Ar port
@@ -671,11 +677,28 @@ Changing the default value might degrade
.Ek
.Xc
.Pp
-Specify an
+Specify a
+.Ic socket
+or an
+.Ar interface
+to listen on for incoming connections. A
New sentence -> new line, please.
Post by Peter Bisroev
+.Ic listen on socket
+directive is used to customize listening behavior for messages submitted
+through local queues, for example, via the
+.Xr mail 1
+utility. This is an optional directive, and if not supplied,
New sentence, new line.
Post by Peter Bisroev
+.Xr smtpd 8
+will listen for connections on the
+.Ic socket
+only. Clients connecting through the
I do not like the wording "...only." here. Sounds ambiguous. However,
I'm not a native speaker. Also, new sentence, new line.
Post by Peter Bisroev
+.Ic socket
+will always be tagged with the 'local'
+.Ic tag .
+.Pp
+To listen on a specific network interface, specify an
.Ar interface
-and
-.Ar port
-to listen on.
+and an optional
+.Ar port .
An interface group, an IP address or a domain name may
be used in place of
.Ar interface .
Gilles Chehade
2016-02-13 20:57:50 UTC
Permalink
Post by Joerg Jung
Post by Peter Bisroev
Post by Peter Bisroev
Just in case the previous diff is OK, I am attaching the patch to the
smtpd.conf man page.
Hi Gilles,
I apologize, my previous manpage diff did not include the information regarding
the fact that connections through local socket will always be tagged 'local'.
Please find the corrected manpage diff below.
Some minor comments inline below.
I commited the man page after taking into consideration jung@'s comment
and rewording a paragraph.
Post by Joerg Jung
Post by Peter Bisroev
Index: smtpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.153
diff -u -p -r1.153 smtpd.conf.5
--- smtpd.conf.5 21 Jan 2016 23:40:27 -0000 1.153
+++ smtpd.conf.5 12 Feb 2016 21:56:51 -0000
@@ -654,6 +654,12 @@ of inflight envelopes falls below
Changing the default value might degrade performance.
.It Xo
.Bk -words
+.Ic listen on socket
+.Op Ic filter Ar name
+.Op Ic mask-source
+.Ek
+.br
+.Bk -words
.Ic listen on Ar interface
.Op Ar family
.Op Ic port Ar port
@@ -671,11 +677,28 @@ Changing the default value might degrade
.Ek
.Xc
.Pp
-Specify an
+Specify a
+.Ic socket
+or an
+.Ar interface
+to listen on for incoming connections. A
New sentence -> new line, please.
Post by Peter Bisroev
+.Ic listen on socket
+directive is used to customize listening behavior for messages submitted
+through local queues, for example, via the
+.Xr mail 1
+utility. This is an optional directive, and if not supplied,
New sentence, new line.
Post by Peter Bisroev
+.Xr smtpd 8
+will listen for connections on the
+.Ic socket
+only. Clients connecting through the
I do not like the wording "...only." here. Sounds ambiguous. However,
I'm not a native speaker. Also, new sentence, new line.
Post by Peter Bisroev
+.Ic socket
+will always be tagged with the 'local'
+.Ic tag .
+.Pp
+To listen on a specific network interface, specify an
.Ar interface
-and
-.Ar port
-to listen on.
+and an optional
+.Ar port .
An interface group, an IP address or a domain name may
be used in place of
.Ar interface .
--
Gilles Chehade

https://www.poolp.org @poolpOrg
Peter Bisroev
2016-02-13 21:40:15 UTC
Permalink
Thank you Joerg for your comments on the manpage diff. But it looks like
Gilles has already committed the diff according to his previous response.
Thank you Gilles!

Just in case, I am including the updated diff as I reworded it as well.
Gilles, Joerg, could you please see if rewording makes the listen on
directive less ambiguous.

Thank you!
--peter



Index: smtpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.153
diff -u -p -r1.153 smtpd.conf.5
--- smtpd.conf.5 21 Jan 2016 23:40:27 -0000 1.153
+++ smtpd.conf.5 13 Feb 2016 20:55:33 -0000
@@ -654,6 +654,12 @@ of inflight envelopes falls below
Changing the default value might degrade performance.
.It Xo
.Bk -words
+.Ic listen on socket
+.Op Ic filter Ar name
+.Op Ic mask-source
+.Ek
+.br
+.Bk -words
.Ic listen on Ar interface
.Op Ar family
.Op Ic port Ar port
@@ -671,11 +677,35 @@ Changing the default value might degrade
.Ek
.Xc
.Pp
-Specify an
+Specify a
+.Ic socket
+or an
+.Ar interface
+to listen on for incoming connections.
+A
+.Ic listen on socket
+directive is used to customize the listening behavior for messages
+submitted through local queues, for example, via the
+.Xr mail 1
+utility.
+This is an optional directive.
+By default,
+.Xr smtpd 8
+will listen for local connections on the
+.Ic socket ,
+.Ic inet4
+and/or
+.Ic inet6
+interfaces will not be enabled.
+Clients connecting through the
+.Ic socket
+will always be tagged with the 'local'
+.Ic tag .
+.Pp
+To listen on a specific network interface, specify an
.Ar interface
-and
-.Ar port
-to listen on.
+and an optional
+.Ar port .
An interface group, an IP address or a domain name may
be used in place of
.Ar interface .

Loading...