OpenSMTPD and mask-source flag.

[Peter Bisroev]

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]

On Mon, Feb 08, 2016 at 08:32:31PM -0500, Peter Bisroev wrote:
> Dear OpenSMTPD Developers!
>

Dear Peter,

> 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.

> 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.

> 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]

Hi Gilles

<snip>

> 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]

On Tue, Feb 09, 2016 at 09:23:17AM -0500, Peter Bisroev wrote:
> Hi Gilles
>

Hi,

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).



> Thank you!
> --peter

[Peter Bisroev]

> 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.

> 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).

> Gilles Chehade
>
> https://www.poolp.org @poolpOrg

Thanks,
--peter

[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.

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]

On Thu, Feb 11, 2016 at 05:28:50PM -0500, Peter Bisroev wrote:
> 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.
>

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

--

[Peter Bisroev]

> 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]

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);

[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 .

[Gilles Chehade]

On Fri, Feb 12, 2016 at 04:29:23PM -0500, Peter Bisroev wrote:
> Hi Gilles,
>

Hi,

> 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 !

[Joerg Jung]

On Fri, Feb 12, 2016 at 05:00:59PM -0500, Peter Bisroev wrote:
> > 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.

New sentence -> new line, please.

> +.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.

> +.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.

[Gilles Chehade]

On Sat, Feb 13, 2016 at 08:32:23PM +0100, Joerg Jung wrote:
> On Fri, Feb 12, 2016 at 05:00:59PM -0500, Peter Bisroev wrote:
> > > 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.

I think jmc@ will find better wording than a german and a french ;-)

[Peter Bisroev]

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