Re: smtpd: malloc+strlcpy -> strndup
On Sat, Sep 01, 2018 at 11:31:49PM +0200, Gilles Chehade wrote:
> On Sat, Sep 01, 2018 at 09:20:59PM +0800, Michael Mikonos wrote:
> > Hello,
> >
> > Replace a malloc+strlcpy with strndup in cmdline_symset().
> > Parameter s is a "keyname=value" string and sym is the
> > "keyname" part.
> >
> > If s is "=value", sym will be an empty string.
> > The patch doesn't change this behaviour although
> > it might be undesirable to call symset() with
> > an empty string. Possibly it could also return -1
> > if len is zero. Thoughts?
> >
>
> Not opposed to the diff but at this late hour I find it easier to read
> the malloc+strlcpy and be sure there's not an off-by-one than with the
> strndup version, I'll read again tomorrow.
In my understanding the length argument of strndup(3) doesn't include
the terminating NUL character. I think the linux manual for strndup(3)
is slightly clearer on this because it has the text:
... only n bytes are copied, and a terminating null byte ('\0') is
added.
> Just wanted to remind you that this function is shared between daemons
> so this can't be an smtpd-only change :-)
>
>
> > Index: parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> > retrieving revision 1.218
> > diff -u -p -u -r1.218 parse.y
> > --- parse.y 25 Aug 2018 19:05:23 - 1.218
> > +++ parse.y 1 Sep 2018 12:42:45 -
> > @@ -2129,11 +2129,10 @@ cmdline_symset(char *s)
> > if ((val = strrchr(s, '=')) == NULL)
> > return (-1);
> >
> > - len = strlen(s) - strlen(val) + 1;
> > - if ((sym = malloc(len)) == NULL)
> > - errx(1, "cmdline_symset: malloc");
> > -
> > - (void)strlcpy(sym, s, len);
> > + len = strlen(s) - strlen(val);
> > + sym = strndup(s, len);
> > + if (sym == NULL)
> > + errx(1, "%s: strndup", __func__);
> >
> > ret = symset(sym, val + 1, 1);
> > free(sym);
> >
>
> --
> Gilles Chehade
>
> https://www.poolp.org @poolpOrg
Re: smtpd: improve syntax for relay host
Hi.
Same diff with associated manpage update.
If there is no objection, I'd like to commit this quickly.
Eric.
Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.199
diff -u -p -r1.199 smtpd.conf.5
--- smtpd.conf.51 Sep 2018 19:56:28 - 1.199
+++ smtpd.conf.52 Sep 2018 18:56:44 -
@@ -228,7 +228,38 @@ to advertise during the HELO phase.
.It Cm host Ar relay-url
Do not perform MX lookups but relay messages to the relay host described by
.Ar relay-url .
-If the URL uses TLS, the certificate will be verified by default.
+The format for
+.Ar relay-url
+is
+.Sm off
+.Op Ar proto No :// Op Ar label No @
+.Ar host Op : Ar port .
+.Sm on
+The following protocols are available:
+.Pp
+.Bl -tag -width "smtp+notls" -compact
+.It smtp
+Normal SMTP session with opportunistic STARTTLS.
+.It smtp+tls
+Normal SMTP session with mandatory STARTTLS.
+.It smtp+notls
+Plain text SMTP session without TLS.
+.It lmtp
+LMTP session.
+.It smtps
+SMTP session with forced TLS on connection.
+.El
+.Pp
+If not specified, the
+.Dq smtp
+protocol is used.
+.Pp
+Specifying an auth label toggles authentication.
+An auth table must also be defined for this action.
+The protocol must explicitely require TLS.
+.Pp
+If TLS is explicitely required, the server certificate
+will be verified by default.
.It Cm tls no-verify
Do not require a valid certificate for the specified host.
.It Cm auth Pf < Ar table Ns >
Index: to.c
===
RCS file: /cvs/src/usr.sbin/smtpd/to.c,v
retrieving revision 1.31
diff -u -p -r1.31 to.c
--- to.c7 Jun 2018 11:31:51 - 1.31
+++ to.c2 Sep 2018 18:56:44 -
@@ -310,15 +310,11 @@ text_to_relayhost(struct relayhost *rela
* new schemas should be *appended* otherwise the default
* schema index needs to be updated later in this function.
*/
- { "smtp://",0 },
+ { "smtp://",RELAY_TLS_OPTIONAL },
+ { "smtp+tls://",RELAY_STARTTLS },
+ { "smtp+notls://", 0 },
{ "lmtp://",RELAY_LMTP },
- { "smtp+tls://",RELAY_TLS_OPTIONAL },
- { "smtps://", RELAY_SMTPS },
- { "tls://", RELAY_STARTTLS },
- { "smtps+auth://", RELAY_SMTPS|RELAY_AUTH },
- { "tls+auth://",RELAY_STARTTLS|RELAY_AUTH },
- { "secure://", RELAY_SMTPS|RELAY_STARTTLS },
- { "secure+auth://", RELAY_SMTPS|RELAY_STARTTLS|RELAY_AUTH }
+ { "smtps://", RELAY_SMTPS }
};
const char *errstr = NULL;
char *p, *q;
@@ -341,8 +337,8 @@ text_to_relayhost(struct relayhost *rela
if (strstr(buffer, "://"))
return 0;
- /* no schema, default to smtp+tls:// */
- i = 2;
+ /* no schema, default to smtp:// */
+ i = 0;
p = buffer;
}
else
@@ -397,10 +393,13 @@ text_to_relayhost(struct relayhost *rela
return 0;
if ((relay->flags & RELAY_LMTP) && (relay->port == 0))
return 0;
- if (relay->authlabel[0] == '\0' && relay->flags & RELAY_AUTH)
- return 0;
- if (relay->authlabel[0] != '\0' && !(relay->flags & RELAY_AUTH))
- return 0;
+ if (relay->authlabel[0]) {
+ /* disallow auth on non-tls scheme. */
+ if (!(relay->flags & (RELAY_STARTTLS | RELAY_SMTPS)))
+ return 0;
+ relay->flags |= RELAY_AUTH;
+ }
+
return 1;
}
Corrected patch for smtpd.conf(5) man page
The earlier patch I created was obviously no good. Sorry for the noise. Included is the fixed patch that just adds some text for properly using an mda wrapper in the actions: Index: smtpd.conf.5 === RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v retrieving revision 1.199 diff -u -p -u -r1.199 smtpd.conf.5 --- smtpd.conf.51 Sep 2018 19:56:28 - 1.199 +++ smtpd.conf.52 Sep 2018 15:53:57 - @@ -205,6 +205,9 @@ Use the mapping for virtual expansion. The aliasing table format is described in .Xr table 5 . +.It Cm wrapper Ar name +Use the wrapper specified in +.Cm mda wrapper. .El .Pp The relay delivery methods also support additional options:
usbd_close_pipe.9: usbd_abort_pipe is void
Hi, spotted this while removing return value from usbd_close_pipe, following cleanup by pirofti@[0], which made usbd_abort_pipe void. -Artturi [0]: https://github.com/openbsd/src/commit/1e087f7cf25ce711b6cef28d054ab4a68be213d2 diff --git share/man/man9/usbd_close_pipe.9 share/man/man9/usbd_close_pipe.9 index 650182d5284..3c2027c337e 100644 --- share/man/man9/usbd_close_pipe.9 +++ share/man/man9/usbd_close_pipe.9 @@ -25,7 +25,7 @@ .In dev/usb/usbdi.h .Ft usbd_status .Fn usbd_close_pipe "struct usbd_pipe *pipe" -.Ft usbd_status +.Ft void .Fn usbd_abort_pipe "struct usbd_pipe *pipe" .Sh DESCRIPTION The
Update to smtpd.conf(5) man page
Hello tech@, Below is a patch to clarify how/where to use the mda wrapper feature of OpenSMTPD: --- smtpd.conf.5Sun Sep 2 10:16:26 2018 +++ smtpd.conf.5.newSun Sep 2 10:16:53 2018 @@ -159,7 +159,7 @@ Relay the message to another SMTP server. .It Cm wrapper Ar name Use the wrapper specified in -.Cm mda wrapper +.Cm mda wrapper. .El .Pp The local delivery methods support additional options: Thanks, Matt
Re: ospfd: pledge parent process
On Sat, Sep 01, 2018 at 10:38:09PM +0200, Sebastian Benoit wrote:
> Remi Locherer(remi.loche...@relo.ch) on 2018.09.01 21:53:21 +0200:
> > Hi,
> >
> > Since slaacd is able to use pledge in the parent process I thought it may
> > be possible for ospfd too.
> >
> > It works fine until ospfd gets reloaded. At this point it uses setsockopt
> > to set the priority filter on the routing socket.
> >
> > Since I could not find a promise for this I extended wroute. Does this make
> > sense? Would another promise or something completely different be better?
>
> just route would be good enough, because route is for receiving routes,
> and the route filter just changes which routes you get.
>
> does the
>
> area ... {
> demote carp
>
> }
>
> feature and the
>
>
>interface if { demote carp ... }
>
> feature still work with this pledge?
No, it does not:
79534 ospfdCALL recvmsg(3,0x7f7f8a40,0)
79534 ospfdGIO fd 3 read 36 bytes
"8\0\0\0$\0\0\0\0\0\0\0Wx\^A\0carp\0\0\0\0\0\0\0\0\0\0\0\0\^A\0\0\0"
79534 ospfdSTRU struct msghdr { name=0x0, namelen=0, iov=0x7f7f8a30,
iovlen=1, control=0x7f7f8a70, controllen=0, flags=0x80 }
79534 ospfdSTRU struct iovec { base=0xcc3203c5034, len=65499 }
79534 ospfdRET recvmsg 36/0x24
79534 ospfdCALL socket(AF_INET,0x2,0)
79534 ospfdPLDG socket, "dns", errno 1 Operation not permitted
79534 ospfdPSIG SIGABRT SIG_DFL
79534 ospfdNAMI "ospfd.core"
This is from socket(AF_INET, SOCK_DGRAM, 0) in carp_demote_get. The same
function needs ioctl(s, SIOCGIFGATTR, (caddr_t)&ifgr) afterwards.
Re: ospfd: pledge parent process
On Sun, Sep 02, 2018 at 08:05:55AM +0200, Remi Locherer wrote:
> On Sat, Sep 01, 2018 at 10:38:09PM +0200, Sebastian Benoit wrote:
> > Remi Locherer(remi.loche...@relo.ch) on 2018.09.01 21:53:21 +0200:
> > > Hi,
> > >
> > > Since slaacd is able to use pledge in the parent process I thought it may
> > > be possible for ospfd too.
> > >
> > > It works fine until ospfd gets reloaded. At this point it uses setsockopt
> > > to set the priority filter on the routing socket.
> > >
> > > Since I could not find a promise for this I extended wroute. Does this
> > > make
> > > sense? Would another promise or something completely different be better?
> >
> > just route would be good enough, because route is for receiving routes,
> > and the route filter just changes which routes you get.
>
> ospfd is not happy with "pledge("stdio rpath sendfd route", NULL)"
>
> During reload:
>
> kr_reload: priority filter disabled
> orig_rtr_lsa: area 0.0.0.0
> Abort trap (core dumped)
> orig_rtr_lsa: stub net, interface pair0
> orig_rtr_lsa: stub net, interface vether0
> [...]
>
> ospfd[2432]: pledge "inet", syscall 105
Theo pointed out a ktrace is needed here to understand what is going on:
18069 ospfdGIO fd 2 wrote 36 bytes
"kr_reload: priority filter disabled
"
18069 ospfdRET write 36/0x24
18069 ospfdCALL setsockopt(6,17,3,0x7f7efb70,4)
18069 ospfdPLDG setsockopt, "inet", errno 1 Operation not permitted
18069 ospfdPSIG SIGABRT SIG_DFL
18069 ospfdNAMI "ospfd.core"
This is from the following line in osfpd/kroute.c
if (setsockopt(kr_state.fd, AF_ROUTE, ROUTE_PRIOFILTER, &filter_prio,
sizeof(filter_prio)) == -1) {
