Re: smtpd: make relay to smarthost to verify TLS by default
On Thu, May 31, 2018 at 10:25:54PM +0200, Eric Faurot wrote:
>
> Hello.
>
> This makes sense, indeed.
>
> Here is a slightly updated diff for your proposal. It makes the
> documentatino more accurate: the server certificate is always
> verified, the flag is only meant to accept invalid certificates. It
> also fixes build (apparently the mta.c chunk was incorrect).
the initial diff was to set F_TLS_VERIFY by default, and only remove it
on request. with the new diff, the logic is inversed. it makes no
functional changes, but it means the code logic could be more fragile as
if code path changes (in future developpement or refactoring) invalidate
certificate could be accepted by default.
but it is fine with me (and invalid certificates are logged).
another comment inline.
Thanks
--
Sebastien Marie
> Index: mta.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 mta.c
> --- mta.c 31 May 2018 11:56:10 - 1.212
> +++ mta.c 31 May 2018 19:56:03 -
> @@ -677,6 +677,9 @@ mta_handle_envelope(struct envelope *evp
> return;
> }
>
> + if (dispatcher->u.remote.tls_noverify == 0)
> + evp->agent.mta.relay.flags |= F_TLS_VERIFY;
> +
I am unsure if the condition should be:
if (smarthost && dispatcher->u.remote.tls_noverify == 0)
it ensures the flag will be set only if smarthost is used.
here, the flag will be added everytime (and removed only with "tls
no-verify" which is usable only with "host example.com").
> relay = mta_relay(evp);
> /* ignore if we don't know the limits yet */
> if (relay->limits &&
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.206
> diff -u -p -r1.206 parse.y
> --- parse.y 30 May 2018 19:01:58 - 1.206
> +++ parse.y 31 May 2018 19:56:04 -
> @@ -182,7 +182,7 @@ typedef struct {
> %token KEY
> %token LIMIT LISTEN LMTP LOCAL
> %token MAIL_FROM MAILDIR MASK_SRC MASQUERADE MATCH MAX_MESSAGE_SIZE
> MAX_DEFERRED MBOX MDA MTA MX
> -%token NODSN
> +%token NODSN NOVERIFY
> %token ON
> %token PKI PORT
> %token QUEUE
> @@ -541,6 +541,19 @@ HELO STRING {
>
> dispatcher->u.remote.smarthost = strdup(t->t_name);
> }
> +| TLS NOVERIFY {
> + if (dispatcher->u.remote.smarthost == NULL) {
> + yyerror("tls no-verify may not be specified without host on a
> dispatcher");
> + YYERROR;
> + }
> +
> + if (dispatcher->u.remote.tls_noverify == 1) {
> + yyerror("tls no-verify already specified for this dispatcher");
> + YYERROR;
> + }
> +
> + dispatcher->u.remote.tls_noverify = 1;
> +}
> | AUTH tables {
> struct table *t = $2;
>
> @@ -1571,6 +1584,7 @@ lookup(char *s)
> { "mta",MTA },
> { "mx", MX },
> { "no-dsn", NODSN },
> + { "no-verify", NOVERIFY },
> { "on", ON },
> { "pki",PKI },
> { "port", PORT },
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.183
> diff -u -p -r1.183 smtpd.conf.5
> --- smtpd.conf.5 31 May 2018 13:36:35 - 1.183
> +++ smtpd.conf.5 31 May 2018 19:56:04 -
> @@ -205,6 +205,9 @@ 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.
> +.It Cm tls Ar no-verify
> +Do not require valid certificate for the specified host.
> .It Cm auth Pf < Ar table Ns >
> Use the mapping
> .Ar table
> Index: smtpd.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.545
> diff -u -p -r1.545 smtpd.h
> --- smtpd.h 29 May 2018 21:05:52 - 1.545
> +++ smtpd.h 31 May 2018 19:56:05 -
> @@ -1068,6 +1068,7 @@ struct dispatcher_remote {
>
> char*smarthost;
> char*auth;
> + int tls_noverify;
>
> int backup;
> char*backupmx;
Re: [Patch] mg(1): Experimental UTF-8 support
To make testing easier, I temporarily committed a port to WIP repo: https://github.com/jasperla/openbsd-wip/tree/master/editors/umg I won't submit it to ports@, this is experimental and only for testing.
Re: de-hole some structs on amd64
Hi,
Is there any feedback on this?
Thanks
> > > I tested removing some slop (i.e. structure packing/de-holing) on amd64,
> > > this went through a full kernel + userland build.
> > >
> >
> > Parts of this are probably okay, but there's some stuff which needs better
> > placement vs comments and at least one move which needs a justification for
> > it being safe (or not).
>
> Thanks for your feedback!
>
> > > --- a/sys/sys/proc.h
> > > +++ b/sys/sys/proc.h
> > > @@ -170,8 +170,8 @@ struct process {
> > >
> > > /* The following fields are all zeroed upon creation in process_new. */
> > > #defineps_startzerops_klist
> > > - struct klist ps_klist; /* knotes attached to this process
> > > */
> > > int ps_flags; /* PS_* flags. */
> > > + struct klist ps_klist; /* knotes attached to this process
> > > */
> > >
> >
> > Nope: you've moved ps_flags from inside the "zeroed out on fork" region to
> > outside of it
> > a) without justifying why that's safe, and
> > b) while leaving it below the comment saying that it's zeroed, when it no
> > longer is.
>
> My fault, I didn't read the defines properly before sending. Fixed by
> defining ps_startzero to point to ps_flags, so it is zero'd out as before.
>
> >
> > Do any of the other moves here cross a start/end zero/copy marker?
>
> Thanks for the hint. I re-checked now from the process_new() and thread_new()
> functions in kern_fork.c. All the moves have been made within the
> startcopy/startzero and endcopy/endzero defines in both struct proc and
> struct process. So the memset to 0, and memcpy from parents will work as
> before. I updated a comment to point to thread_new() function, so it is clear
> where struct proc is inited. Please let me know if I have overlooked anything.
>
> >
> > > @@ -285,6 +284,7 @@ struct proc {
> > > struct futex *p_futex; /* Current sleeping futex. */
> > >
> > > /* substructures: */
> > > + LIST_ENTRY(proc) p_hash;/* Hash chain. */
> > > struct filedesc *p_fd; /* copy of p_p->ps_fd */
> > > struct vmspace *p_vmspace; /* copy of p_p->ps_vmspace */
> > >
> >
> > p_hash isn't a substructure, so putting it below the /* substructures: */
> > comment is wrong. Please pay attention to the comments and consider how
> > the apply (or don't) to the members you're moving.
>
> Fixed.
>
> >
> > > @@ -305,6 +304,11 @@ struct proc {
> > > longp_thrslpid; /* for thrsleep syscall */
> > >
> > > /* scheduling */
> > > + struct cpu_info * volatile p_cpu; /* CPU we're running on. */
> > > +
> > > + struct rusage p_ru;/* Statistics */
> > > + struct tusage p_tu;/* accumulated times. */
> > > + struct timespec p_rtime; /* Real time. */
> > > u_int p_estcpu;/* Time averaged value of p_cpticks. */
> > > int p_cpticks; /* Ticks of cpu time. */
> > >
> >
> > Again, you've separated the scheduling parameter from the /* scheduling */
> > comment, putting member that aren't about scheduling between them.
>
> Fixed. The structs rusage/tusage/timespec are not part of scheduling, so I
> moved them before the scheduling comment.
>
> Updated diff follows. This survived a kernel compile, reboot, and use for
> quite some time.
>
>
> diff --git a/sys/sys/proc.h b/sys/sys/proc.h
> index 1c7ea4697e2..d6082cb0551 100644
> --- a/sys/sys/proc.h
> +++ b/sys/sys/proc.h
> @@ -169,9 +169,9 @@ struct process {
> pid_t ps_pid; /* Process identifier. */
>
> /* The following fields are all zeroed upon creation in process_new. */
> -#define ps_startzerops_klist
> - struct klist ps_klist; /* knotes attached to this process */
> +#define ps_startzerops_flags
> int ps_flags; /* PS_* flags. */
> + struct klist ps_klist; /* knotes attached to this process */
>
> struct proc *ps_single;/* Single threading to this thread. */
> int ps_singlecount; /* Not yet suspended threads. */
> @@ -200,15 +200,6 @@ struct process {
> struct pgrp *ps_pgrp; /* Pointer to process group. */
> struct emul *ps_emul; /* Emulation information */
>
> - charps_comm[MAXCOMLEN+1];
> -
> - vaddr_t ps_strings; /* User pointers to argv/env */
> - vaddr_t ps_sigcode; /* User pointer to the signal code */
> - vaddr_t ps_sigcoderet; /* User pointer to sigreturn retPC */
> - u_long ps_sigcookie;
> - u_int ps_rtableid;/* Process routing table/domain. */
> - charps_nice;/* Process "nice" value. */
> -
> struct uprof { /* profile arguments */
> caddr_t pr_base;/* buffer base */
> size_t pr_size;/* buffer size */
> @@ -216,
Re: smtpd: make relay to smarthost to verify TLS by default
On Thu, May 31, 2018 at 10:25:54PM +0200, Eric Faurot wrote:
> On Thu, May 31, 2018 at 04:06:31PM +0200, Sebastien Marie wrote:
> > Hi,
> >
> > When using smarthost ("host" option of "relay") for outgoing mails, TLS
> > connection aren't verified. If it could make sens for standard MX, I
> > think it would be better to verify the connection by default if the user
> > specifies a TLS-aware url for the relay.
> >
> > The diff below changes the behaviour of:
> > action "foo" relay host "smtps://example.com"
> >
> > Currently, smtpd will connect to example.com without verifying TLS at
> > all. There is no option to force such verification (it was present in
> > with previous grammar).
> >
> > With the following diff, the TLS connection is verified by default (and
> > the connection aborted on error). Opportunistic TLS will be still possible
> > with a new option: tls no-verify.
> >
> > So, for having the old behaviour the syntax will be:
> > action "foo" relay host "smtps://example.com" tls no-verify
> >
> > It affects only smarthost connection. Outgoing using MX still use
> > opportunistic TLS.
> >
> > Regarding the diff, it adds F_TLS_VERIFY option by default for each call
> > of text_to_relayhost(), so also for "smtp://" or "lmtp://" urls. I checked
> > that "smtp://" isn't affected by such flag (there is no TLS connection
> > to verify), and I hope it will be the same for "lmtp://" (confirmation
> > will be welcome). Next, the grammar is extended to permit to clear the
> > flag if requested. smtpd(1) already have all the magic to check the
> > connection.
> >
> > Thanks.
> > --
> > Sebastien Marie
>
> Hello.
>
> This makes sense, indeed.
>
> Here is a slightly updated diff for your proposal. It makes the
> documentatino more accurate: the server certificate is always
> verified, the flag is only meant to accept invalid certificates. It
> also fixes build (apparently the mta.c chunk was incorrect).
>
ok by me
> Index: mta.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 mta.c
> --- mta.c 31 May 2018 11:56:10 - 1.212
> +++ mta.c 31 May 2018 19:56:03 -
> @@ -677,6 +677,9 @@ mta_handle_envelope(struct envelope *evp
> return;
> }
>
> + if (dispatcher->u.remote.tls_noverify == 0)
> + evp->agent.mta.relay.flags |= F_TLS_VERIFY;
> +
> relay = mta_relay(evp);
> /* ignore if we don't know the limits yet */
> if (relay->limits &&
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.206
> diff -u -p -r1.206 parse.y
> --- parse.y 30 May 2018 19:01:58 - 1.206
> +++ parse.y 31 May 2018 19:56:04 -
> @@ -182,7 +182,7 @@ typedef struct {
> %token KEY
> %token LIMIT LISTEN LMTP LOCAL
> %token MAIL_FROM MAILDIR MASK_SRC MASQUERADE MATCH MAX_MESSAGE_SIZE
> MAX_DEFERRED MBOX MDA MTA MX
> -%token NODSN
> +%token NODSN NOVERIFY
> %token ON
> %token PKI PORT
> %token QUEUE
> @@ -541,6 +541,19 @@ HELO STRING {
>
> dispatcher->u.remote.smarthost = strdup(t->t_name);
> }
> +| TLS NOVERIFY {
> + if (dispatcher->u.remote.smarthost == NULL) {
> + yyerror("tls no-verify may not be specified without host on a
> dispatcher");
> + YYERROR;
> + }
> +
> + if (dispatcher->u.remote.tls_noverify == 1) {
> + yyerror("tls no-verify already specified for this dispatcher");
> + YYERROR;
> + }
> +
> + dispatcher->u.remote.tls_noverify = 1;
> +}
> | AUTH tables {
> struct table *t = $2;
>
> @@ -1571,6 +1584,7 @@ lookup(char *s)
> { "mta",MTA },
> { "mx", MX },
> { "no-dsn", NODSN },
> + { "no-verify", NOVERIFY },
> { "on", ON },
> { "pki",PKI },
> { "port", PORT },
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.183
> diff -u -p -r1.183 smtpd.conf.5
> --- smtpd.conf.5 31 May 2018 13:36:35 - 1.183
> +++ smtpd.conf.5 31 May 2018 19:56:04 -
> @@ -205,6 +205,9 @@ 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.
> +.It Cm tls Ar no-verify
> +Do not require valid certificate for the specified host.
> .It Cm auth Pf < Ar table Ns >
> Use the mapping
> .Ar table
> Index: smtpd.h
> ===
> RCS file: /cvs/src/usr.
Re: smtpd: make relay to smarthost to verify TLS by default
On Thu, May 31, 2018 at 04:06:31PM +0200, Sebastien Marie wrote:
> Hi,
>
> When using smarthost ("host" option of "relay") for outgoing mails, TLS
> connection aren't verified. If it could make sens for standard MX, I
> think it would be better to verify the connection by default if the user
> specifies a TLS-aware url for the relay.
>
> The diff below changes the behaviour of:
> action "foo" relay host "smtps://example.com"
>
> Currently, smtpd will connect to example.com without verifying TLS at
> all. There is no option to force such verification (it was present in
> with previous grammar).
>
> With the following diff, the TLS connection is verified by default (and
> the connection aborted on error). Opportunistic TLS will be still possible
> with a new option: tls no-verify.
>
> So, for having the old behaviour the syntax will be:
> action "foo" relay host "smtps://example.com" tls no-verify
>
> It affects only smarthost connection. Outgoing using MX still use
> opportunistic TLS.
>
> Regarding the diff, it adds F_TLS_VERIFY option by default for each call
> of text_to_relayhost(), so also for "smtp://" or "lmtp://" urls. I checked
> that "smtp://" isn't affected by such flag (there is no TLS connection
> to verify), and I hope it will be the same for "lmtp://" (confirmation
> will be welcome). Next, the grammar is extended to permit to clear the
> flag if requested. smtpd(1) already have all the magic to check the
> connection.
>
> Thanks.
> --
> Sebastien Marie
Hello.
This makes sense, indeed.
Here is a slightly updated diff for your proposal. It makes the
documentatino more accurate: the server certificate is always
verified, the flag is only meant to accept invalid certificates. It
also fixes build (apparently the mta.c chunk was incorrect).
Eric.
Index: mta.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
retrieving revision 1.212
diff -u -p -r1.212 mta.c
--- mta.c 31 May 2018 11:56:10 - 1.212
+++ mta.c 31 May 2018 19:56:03 -
@@ -677,6 +677,9 @@ mta_handle_envelope(struct envelope *evp
return;
}
+ if (dispatcher->u.remote.tls_noverify == 0)
+ evp->agent.mta.relay.flags |= F_TLS_VERIFY;
+
relay = mta_relay(evp);
/* ignore if we don't know the limits yet */
if (relay->limits &&
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.206
diff -u -p -r1.206 parse.y
--- parse.y 30 May 2018 19:01:58 - 1.206
+++ parse.y 31 May 2018 19:56:04 -
@@ -182,7 +182,7 @@ typedef struct {
%token KEY
%token LIMIT LISTEN LMTP LOCAL
%token MAIL_FROM MAILDIR MASK_SRC MASQUERADE MATCH MAX_MESSAGE_SIZE
MAX_DEFERRED MBOX MDA MTA MX
-%token NODSN
+%token NODSN NOVERIFY
%token ON
%token PKI PORT
%token QUEUE
@@ -541,6 +541,19 @@ HELO STRING {
dispatcher->u.remote.smarthost = strdup(t->t_name);
}
+| TLS NOVERIFY {
+ if (dispatcher->u.remote.smarthost == NULL) {
+ yyerror("tls no-verify may not be specified without host on a
dispatcher");
+ YYERROR;
+ }
+
+ if (dispatcher->u.remote.tls_noverify == 1) {
+ yyerror("tls no-verify already specified for this dispatcher");
+ YYERROR;
+ }
+
+ dispatcher->u.remote.tls_noverify = 1;
+}
| AUTH tables {
struct table *t = $2;
@@ -1571,6 +1584,7 @@ lookup(char *s)
{ "mta",MTA },
{ "mx", MX },
{ "no-dsn", NODSN },
+ { "no-verify", NOVERIFY },
{ "on", ON },
{ "pki",PKI },
{ "port", PORT },
Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.183
diff -u -p -r1.183 smtpd.conf.5
--- smtpd.conf.531 May 2018 13:36:35 - 1.183
+++ smtpd.conf.531 May 2018 19:56:04 -
@@ -205,6 +205,9 @@ 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.
+.It Cm tls Ar no-verify
+Do not require valid certificate for the specified host.
.It Cm auth Pf < Ar table Ns >
Use the mapping
.Ar table
Index: smtpd.h
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.545
diff -u -p -r1.545 smtpd.h
--- smtpd.h 29 May 2018 21:05:52 - 1.545
+++ smtpd.h 31 May 2018 19:56:05 -
@@ -1068,6 +1068,7 @@ struct dispatcher_remote {
char*smarthost;
char*auth;
+ int tls_noverify;
one case per ioctl in in_ioctl_change_ifaddr(), take 2
Here's a fixed version of my previous diff that does not accidentally
add a walk over the ifa_list to SIOCSIFADDR.
Instead, start with a merged version of SIOCSIFADDR, add a copy of the
walk to both SIOCAIFADDR and SIOCDIFADDR and add the allocation of the
ia to SIACAIFADDR. As before, start each case with a privilege check.
Index: sys/netinet/in.c
===
RCS file: /var/cvs/src/sys/netinet/in.c,v
retrieving revision 1.157
diff -u -p -r1.157 in.c
--- sys/netinet/in.c31 May 2018 17:17:01 - 1.157
+++ sys/netinet/in.c31 May 2018 17:20:17 -
@@ -334,26 +334,10 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
}
switch (cmd) {
- case SIOCAIFADDR:
- case SIOCDIFADDR:
- if (ifra->ifra_addr.sin_family == AF_INET) {
- for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
- if ((ifa->ifa_addr->sa_family == AF_INET) &&
- ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
- ifra->ifra_addr.sin_addr.s_addr)
- break;
- }
- ia = ifatoia(ifa);
- }
- if (cmd == SIOCDIFADDR && ia == NULL) {
- error = EADDRNOTAVAIL;
- goto err;
- }
- /* FALLTHROUGH */
case SIOCSIFADDR:
if (!privileged) {
error = EPERM;
- goto err;
+ break;
}
if (ia == NULL) {
@@ -373,10 +357,7 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
newifaddr = 1;
} else
newifaddr = 0;
- break;
- }
- switch(cmd) {
- case SIOCSIFADDR:
+
in_ifscrub(ifp, ia);
error = in_ifinit(ifp, ia, satosin(&ifr->ifr_addr), newifaddr);
if (error)
@@ -387,6 +368,38 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
case SIOCAIFADDR: {
int needinit = 0;
+ if (!privileged) {
+ error = EPERM;
+ break;
+ }
+
+ if (ifra->ifra_addr.sin_family == AF_INET) {
+ for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
+ if ((ifa->ifa_addr->sa_family == AF_INET) &&
+ ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
+ ifra->ifra_addr.sin_addr.s_addr)
+ break;
+ }
+ ia = ifatoia(ifa);
+ }
+ if (ia == NULL) {
+ ia = malloc(sizeof *ia, M_IFADDR, M_WAITOK | M_ZERO);
+ ia->ia_addr.sin_family = AF_INET;
+ ia->ia_addr.sin_len = sizeof(ia->ia_addr);
+ ia->ia_ifa.ifa_addr = sintosa(&ia->ia_addr);
+ ia->ia_ifa.ifa_dstaddr = sintosa(&ia->ia_dstaddr);
+ ia->ia_ifa.ifa_netmask = sintosa(&ia->ia_sockmask);
+ ia->ia_sockmask.sin_len = 8;
+ if (ifp->if_flags & IFF_BROADCAST) {
+ ia->ia_broadaddr.sin_len = sizeof(ia->ia_addr);
+ ia->ia_broadaddr.sin_family = AF_INET;
+ }
+ ia->ia_ifp = ifp;
+
+ newifaddr = 1;
+ } else
+ newifaddr = 0;
+
if (ia->ia_addr.sin_family == AF_INET) {
if (ifra->ifra_addr.sin_len == 0)
ifra->ifra_addr = ia->ia_addr;
@@ -423,6 +436,25 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
break;
}
case SIOCDIFADDR:
+ if (!privileged) {
+ error = EPERM;
+ break;
+ }
+
+ if (ifra->ifra_addr.sin_family == AF_INET) {
+ for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
+ if ((ifa->ifa_addr->sa_family == AF_INET) &&
+ ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
+ ifra->ifra_addr.sin_addr.s_addr)
+ break;
+ }
+ ia = ifatoia(ifa);
+ }
+ if (ia == NULL) {
+ error = EADDRNOTAVAIL;
+ break;
+ }
+
/*
* Even if the individual steps were safe, shouldn't
* these kinds of changes happen atomically? What
@@ -437,7 +469,6 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
Re: smtpd: make relay to smarthost to verify TLS by default
On Thu, May 31, 2018 at 04:06:31PM +0200, Sebastien Marie wrote:
> Hi,
>
> When using smarthost ("host" option of "relay") for outgoing mails, TLS
> connection aren't verified. If it could make sens for standard MX, I
> think it would be better to verify the connection by default if the user
> specifies a TLS-aware url for the relay.
>
> The diff below changes the behaviour of:
> action "foo" relay host "smtps://example.com"
>
> Currently, smtpd will connect to example.com without verifying TLS at
> all. There is no option to force such verification (it was present in
> with previous grammar).
>
> With the following diff, the TLS connection is verified by default (and
> the connection aborted on error). Opportunistic TLS will be still possible
> with a new option: tls no-verify.
>
> So, for having the old behaviour the syntax will be:
> action "foo" relay host "smtps://example.com" tls no-verify
>
> It affects only smarthost connection. Outgoing using MX still use
> opportunistic TLS.
>
> Regarding the diff, it adds F_TLS_VERIFY option by default for each call
> of text_to_relayhost(), so also for "smtp://" or "lmtp://" urls. I checked
> that "smtp://" isn't affected by such flag (there is no TLS connection
> to verify), and I hope it will be the same for "lmtp://" (confirmation
> will be welcome). Next, the grammar is extended to permit to clear the
> flag if requested. smtpd(1) already have all the magic to check the
> connection.
>
this is ok to me, i'll give eric@ some time to react and will commit
when i get back home tonight if he didn't
> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.183
> diff -u -p -r1.183 smtpd.conf.5
> --- smtpd.conf.5 31 May 2018 13:36:35 - 1.183
> +++ smtpd.conf.5 31 May 2018 13:50:55 -
> @@ -205,6 +205,9 @@ 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 connection will be verified by default.
> +.It Cm tls Ar no-verify
> +Do not perform tls verification for the specified host.
> .It Cm auth Pf < Ar table Ns >
> Use the mapping
> .Ar table
> Index: smtpd.h
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
> retrieving revision 1.545
> diff -u -p -r1.545 smtpd.h
> --- smtpd.h 29 May 2018 21:05:52 - 1.545
> +++ smtpd.h 31 May 2018 13:50:56 -
> @@ -1068,6 +1068,7 @@ struct dispatcher_remote {
>
> char*smarthost;
> char*auth;
> + int tls_noverify;
>
> int backup;
> char*backupmx;
> Index: to.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/to.c,v
> retrieving revision 1.30
> diff -u -p -r1.30 to.c
> --- to.c 29 May 2018 21:05:52 - 1.30
> +++ to.c 31 May 2018 13:50:57 -
> @@ -348,7 +348,8 @@ text_to_relayhost(struct relayhost *rela
> else
> p = buffer + strlen(schemas[i].name);
>
> - relay->flags = schemas[i].flags;
> + /* require valid tls by default (if tls is used) */
> + relay->flags = schemas[i].flags | F_TLS_VERIFY;
>
> /* need to specify an explicit port for LMTP */
> if (relay->flags & F_LMTP)
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.206
> diff -u -p -r1.206 parse.y
> --- parse.y 30 May 2018 19:01:58 - 1.206
> +++ parse.y 31 May 2018 13:50:58 -
> @@ -182,7 +182,7 @@ typedef struct {
> %token KEY
> %token LIMIT LISTEN LMTP LOCAL
> %token MAIL_FROM MAILDIR MASK_SRC MASQUERADE MATCH MAX_MESSAGE_SIZE
> MAX_DEFERRED MBOX MDA MTA MX
> -%token NODSN
> +%token NODSN NOVERIFY
> %token ON
> %token PKI PORT
> %token QUEUE
> @@ -541,6 +541,19 @@ HELO STRING {
>
> dispatcher->u.remote.smarthost = strdup(t->t_name);
> }
> +| TLS NOVERIFY {
> + if (dispatcher->u.remote.smarthost == NULL) {
> + yyerror("tls no-verify may not be specified without host on a
> dispatcher");
> + YYERROR;
> + }
> +
> + if (dispatcher->u.remote.tls_noverify == 1) {
> + yyerror("tls no-verify already specified for this dispatcher");
> + YYERROR;
> + }
> +
> + dispatcher->u.remote.tls_noverify = 1;
> +}
> | AUTH tables {
> struct table *t = $2;
>
> @@ -1571,6 +1584,7 @@ lookup(char *s)
> { "mta",MTA },
> { "mx", MX },
> { "no-dsn", NODSN },
> + { "no-verify", NOVERIFY },
> { "on", ON },
> { "pki",PKI },
>
Re: bgpd ignore aspath with to large attributes
On Thu, May 31, 2018 at 11:12:38AM +, Job Snijders wrote: > On Wed, May 30, 2018 at 03:18:45PM +0200, Claudio Jeker wrote: > > This adds a protection to handle aspaths overly large attributes in > > bgpd. The main idea is to protect other bgp routes downstream for > > hitting the limit with is often not well catched. > > I am not sure this is sound logic. The BGP UPDATE limit has been 4096 > for decades, and protocol developers are even looking into extending it > beyond 4096. If a downstream router cannot handle the legal limit that > is on them, they need to fix their non-compliance. Adding support for beyond 4096 size should not be hard and I assume the RFC would cover many of the cases where sizes mix and stuff stops to fit. I don't fully remember why I added this but it had to do with a propagation issue because of hitting the limit. I'm not even 100% sure if bgpd is handling this in a reasonable way right now. > > The limit is currently a bit arbitarily set to 4096 - 1024 which is > > afaik the same limit that bird uses. > > Arbitrary limits are arbitrary, not sure I agree with how BIRD handles > this either. I agree with this. In general I would prefer if we just handle the case as clean as possible. One issue I see is that we can grow the attributes internally over 4096 and then the prefix fails to be sent out (to some neighbor but maybe not all). The cheap way of solving this is to allow enough scratch room as done in the diff. Another option is probably to handle the error case correctly and withdraw the update in Adj-RIB-Out. What I'm not sure is if in that case also the Loc-RIB entry should be flagged invalid. I'll check how my Adj-RIB-Out patch is handling this. Will work on a better solution for this corner case. -- :wq Claudio
smtpd: make relay to smarthost to verify TLS by default
Hi,
When using smarthost ("host" option of "relay") for outgoing mails, TLS
connection aren't verified. If it could make sens for standard MX, I
think it would be better to verify the connection by default if the user
specifies a TLS-aware url for the relay.
The diff below changes the behaviour of:
action "foo" relay host "smtps://example.com"
Currently, smtpd will connect to example.com without verifying TLS at
all. There is no option to force such verification (it was present in
with previous grammar).
With the following diff, the TLS connection is verified by default (and
the connection aborted on error). Opportunistic TLS will be still possible
with a new option: tls no-verify.
So, for having the old behaviour the syntax will be:
action "foo" relay host "smtps://example.com" tls no-verify
It affects only smarthost connection. Outgoing using MX still use
opportunistic TLS.
Regarding the diff, it adds F_TLS_VERIFY option by default for each call
of text_to_relayhost(), so also for "smtp://" or "lmtp://" urls. I checked
that "smtp://" isn't affected by such flag (there is no TLS connection
to verify), and I hope it will be the same for "lmtp://" (confirmation
will be welcome). Next, the grammar is extended to permit to clear the
flag if requested. smtpd(1) already have all the magic to check the
connection.
Thanks.
--
Sebastien Marie
Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.183
diff -u -p -r1.183 smtpd.conf.5
--- smtpd.conf.531 May 2018 13:36:35 - 1.183
+++ smtpd.conf.531 May 2018 13:50:55 -
@@ -205,6 +205,9 @@ 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 connection will be verified by default.
+.It Cm tls Ar no-verify
+Do not perform tls verification for the specified host.
.It Cm auth Pf < Ar table Ns >
Use the mapping
.Ar table
Index: smtpd.h
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.545
diff -u -p -r1.545 smtpd.h
--- smtpd.h 29 May 2018 21:05:52 - 1.545
+++ smtpd.h 31 May 2018 13:50:56 -
@@ -1068,6 +1068,7 @@ struct dispatcher_remote {
char*smarthost;
char*auth;
+ int tls_noverify;
int backup;
char*backupmx;
Index: to.c
===
RCS file: /cvs/src/usr.sbin/smtpd/to.c,v
retrieving revision 1.30
diff -u -p -r1.30 to.c
--- to.c29 May 2018 21:05:52 - 1.30
+++ to.c31 May 2018 13:50:57 -
@@ -348,7 +348,8 @@ text_to_relayhost(struct relayhost *rela
else
p = buffer + strlen(schemas[i].name);
- relay->flags = schemas[i].flags;
+ /* require valid tls by default (if tls is used) */
+ relay->flags = schemas[i].flags | F_TLS_VERIFY;
/* need to specify an explicit port for LMTP */
if (relay->flags & F_LMTP)
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.206
diff -u -p -r1.206 parse.y
--- parse.y 30 May 2018 19:01:58 - 1.206
+++ parse.y 31 May 2018 13:50:58 -
@@ -182,7 +182,7 @@ typedef struct {
%token KEY
%token LIMIT LISTEN LMTP LOCAL
%token MAIL_FROM MAILDIR MASK_SRC MASQUERADE MATCH MAX_MESSAGE_SIZE
MAX_DEFERRED MBOX MDA MTA MX
-%token NODSN
+%token NODSN NOVERIFY
%token ON
%token PKI PORT
%token QUEUE
@@ -541,6 +541,19 @@ HELO STRING {
dispatcher->u.remote.smarthost = strdup(t->t_name);
}
+| TLS NOVERIFY {
+ if (dispatcher->u.remote.smarthost == NULL) {
+ yyerror("tls no-verify may not be specified without host on a
dispatcher");
+ YYERROR;
+ }
+
+ if (dispatcher->u.remote.tls_noverify == 1) {
+ yyerror("tls no-verify already specified for this dispatcher");
+ YYERROR;
+ }
+
+ dispatcher->u.remote.tls_noverify = 1;
+}
| AUTH tables {
struct table *t = $2;
@@ -1571,6 +1584,7 @@ lookup(char *s)
{ "mta",MTA },
{ "mx", MX },
{ "no-dsn", NODSN },
+ { "no-verify", NOVERIFY },
{ "on", ON },
{ "pki",PKI },
{ "port", PORT },
Index: mta.c
===
RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v
retrieving revision 1.212
diff -u -p -r1.212 mta.c
--- mta.c 31 May 2018 11:56:10 - 1.212
+++ mta.c 31 May 2018 13:51:00 -
@@ -616,6 +616,9 @@ mta_route_next_task(struct mta_relay *re
Re: corrections in smtpd.conf(5)
On Thu, May 31, 2018 at 03:22:02PM +0200, Sebastien Marie wrote: > Hi, > > The following diff corrects smtpd.conf man page in two ways: > > - Replace virtual(5) reference by table(5) as virtual table format is > documentation in table(5) man page under "Aliasing tables" section. > > - Add "auth " documentation. Example at end of the man page uses > it, so it should be documented. > This reads ok, thanks ! > Index: smtpd.conf.5 > === > RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v > retrieving revision 1.182 > diff -u -p -r1.182 smtpd.conf.5 > --- smtpd.conf.5 30 May 2018 13:44:03 - 1.182 > +++ smtpd.conf.5 31 May 2018 13:14:30 - > @@ -179,9 +179,9 @@ option. > .It Cm virtual Pf < Ar table Ns > > Use the mapping > .Ar table > -for > -.Xr virtual 5 > -expansion. > +for virtual expansion. > +The aliasing table format is described in > +.Xr table 5 . > .El > .Pp > The relay delivery methods also support additional options: > @@ -205,6 +205,17 @@ 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 . > +.It Cm auth Pf < Ar table Ns > > +Use the mapping > +.Ar table > +for connecting to > +.Ar relay-url > +using credentials. > +This option is usable only with > +.Cm host > +option. > +The credential table format is described in > +.Xr table 5 . > .It Cm mail\-from Ar mailaddr > Use > .Ar mailaddr > -- Gilles Chehade https://www.poolp.org @poolpOrg
corrections in smtpd.conf(5)
Hi, The following diff corrects smtpd.conf man page in two ways: - Replace virtual(5) reference by table(5) as virtual table format is documentation in table(5) man page under "Aliasing tables" section. - Add "auth " documentation. Example at end of the man page uses it, so it should be documented. Thanks. -- Sebastien Marie Index: smtpd.conf.5 === RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v retrieving revision 1.182 diff -u -p -r1.182 smtpd.conf.5 --- smtpd.conf.530 May 2018 13:44:03 - 1.182 +++ smtpd.conf.531 May 2018 13:14:30 - @@ -179,9 +179,9 @@ option. .It Cm virtual Pf < Ar table Ns > Use the mapping .Ar table -for -.Xr virtual 5 -expansion. +for virtual expansion. +The aliasing table format is described in +.Xr table 5 . .El .Pp The relay delivery methods also support additional options: @@ -205,6 +205,17 @@ 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 . +.It Cm auth Pf < Ar table Ns > +Use the mapping +.Ar table +for connecting to +.Ar relay-url +using credentials. +This option is usable only with +.Cm host +option. +The credential table format is described in +.Xr table 5 . .It Cm mail\-from Ar mailaddr Use .Ar mailaddr
Re: in_ioctl: one case per ioctl
On Wed, May 30, 2018 at 04:49:15AM +0200, Theo Buehler wrote:
> We can finally get rid of one switch in both, in_ioctl() and
> in_ioctl_change_ifaddr(). With this diff we have one case per ioctl,
> each case dealing with an ioctl starts with a privilege check before any
> global data is modified and the code paths are now straightforward.
Unfortunately, I saw too late that the first version of this diff
contained a mistake in the SIOCSIFADDR case. Let's go more slowly and
let me do this in two steps instead.
Here's the unchanged first part of the diff dealing with in_ioctl()
again. I'll send a fixed version of the second part once this is in.
Index: sys/netinet/in.c
===
RCS file: /cvs/src/sys/netinet/in.c,v
retrieving revision 1.155
diff -u -p -r1.155 in.c
--- sys/netinet/in.c31 May 2018 11:49:26 - 1.155
+++ sys/netinet/in.c31 May 2018 12:00:57 -
@@ -248,33 +248,28 @@ in_ioctl(u_long cmd, caddr_t data, struc
}
}
+ if (ia && satosin(&ifr->ifr_addr)->sin_addr.s_addr) {
+ for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
+ if ((ifa->ifa_addr->sa_family == AF_INET) &&
+ ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
+ satosin(&ifr->ifr_addr)->sin_addr.s_addr) {
+ ia = ifatoia(ifa);
+ break;
+ }
+ }
+ }
+ if (ia == NULL) {
+ NET_UNLOCK();
+ return (EADDRNOTAVAIL);
+ }
+
switch (cmd) {
- case SIOCSIFNETMASK:
case SIOCSIFDSTADDR:
- case SIOCSIFBRDADDR:
if (!privileged) {
error = EPERM;
- goto err;
+ break;
}
- if (ia && satosin(&ifr->ifr_addr)->sin_addr.s_addr) {
- for (; ifa != NULL; ifa = TAILQ_NEXT(ifa, ifa_list)) {
- if ((ifa->ifa_addr->sa_family == AF_INET) &&
- ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
- satosin(&ifr->ifr_addr)->sin_addr.s_addr) {
- ia = ifatoia(ifa);
- break;
- }
- }
- }
- if (ia == NULL) {
- error = EADDRNOTAVAIL;
- goto err;
- }
- break;
- }
- switch (cmd) {
- case SIOCSIFDSTADDR:
if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
error = EINVAL;
break;
@@ -291,6 +286,11 @@ in_ioctl(u_long cmd, caddr_t data, struc
break;
case SIOCSIFBRDADDR:
+ if (!privileged) {
+ error = EPERM;
+ break;
+ }
+
if ((ifp->if_flags & IFF_BROADCAST) == 0) {
error = EINVAL;
break;
@@ -299,12 +299,16 @@ in_ioctl(u_long cmd, caddr_t data, struc
break;
case SIOCSIFNETMASK:
+ if (!privileged) {
+ error = EPERM;
+ break;
+ }
+
ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr =
ifra->ifra_addr.sin_addr.s_addr;
break;
}
-err:
NET_UNLOCK();
return (error);
}
Re: new semapahore implementation using atomics and futexes
On 09/05/18(Wed) 14:19, Paul Irofti wrote:
> > [...]
> > I'd prefer if we could teach each other how stuff really work :o)
>
> Frankly someone else will have to enlighten me (or us) if we really
> need to do this.
That's what guenther@ and visa@ did. So I believe you should move
forward and commit this. Make sure the archs list contain all the
new shiny FUTEX archs and don't forget the *_compat.c version :)
With all of that ok mpi@.
> diff --git lib/librthread/Makefile lib/librthread/Makefile
> index 4c3e127491d..5dfb140290e 100644
> --- lib/librthread/Makefile
> +++ lib/librthread/Makefile
> @@ -30,12 +30,19 @@ SRCS= rthread.c \
> rthread_rwlock.c \
> rthread_rwlockattr.c \
> rthread_sched.c \
> - rthread_sem.c \
> rthread_sig.c \
> rthread_stack.c \
> rthread_spin_lock.c \
> sched_prio.c
>
> +# Architectures that implement atomics
> +.if ${MACHINE_ARCH} == "amd64" || ${MACHINE_ARCH} == "i386" || \
> +${MACHINE_ARCH} == "mips64" || ${MACHINE_ARCH} == "mips64el"
> +SRCS+= rthread_sem_atomic.c
> +.else
> +SRCS+= rthread_sem.c
> +.endif
> +
> SRCDIR= ${.CURDIR}/../libpthread
> .include "${SRCDIR}/man/Makefile.inc"
> .include
> diff --git lib/librthread/rthread_sem_atomic.c
> lib/librthread/rthread_sem_atomic.c
> new file mode 100644
> index 000..c2de5d25de2
> --- /dev/null
> +++ lib/librthread/rthread_sem_atomic.c
> @@ -0,0 +1,429 @@
> +/* $OpenBSD$ */
> +/*
> + * Copyright (c) 2004,2005,2013 Ted Unangst
> + * Copyright (c) 2018 Paul Irofti
> + * All Rights Reserved.
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include "rthread.h"
> +#include "cancel.h" /* in libc/include */
> +#include "synch.h"
> +
> +#define SHARED_IDENT ((void *)-1)
> +
> +/* SHA256_DIGEST_STRING_LENGTH includes nul */
> +/* "/tmp/" + sha256 + ".sem" */
> +#define SEM_PATH_SIZE (5 + SHA256_DIGEST_STRING_LENGTH + 4)
> +
> +/* long enough to be hard to guess */
> +#define SEM_RANDOM_NAME_LEN 10
> +
> +/*
> + * Size of memory to be mmap()'ed by named semaphores.
> + * Should be >= SEM_PATH_SIZE and page-aligned.
> + */
> +#define SEM_MMAP_SIZE_thread_pagesize
> +
> +/*
> + * Internal implementation of semaphores
> + */
> +int
> +_sem_wait(sem_t sem, int can_eintr, const struct timespec *abstime,
> +int *delayed_cancel)
> +{
> + int r = 0;
> + int v, ov;
> +
> + atomic_inc_int(&sem->waitcount);
> + for (;;) {
> + while ((v = sem->value) > 0) {
> + ov = atomic_cas_uint(&sem->value, v, v - 1);
> + if (ov == v) {
> + membar_enter_after_atomic();
> + atomic_dec_int(&sem->waitcount);
> + return 0;
> + }
> + }
> + if (r)
> + break;
> +
> + r = _twait(&sem->value, 0, CLOCK_REALTIME, abstime);
> + /* ignore interruptions other than cancelation */
> + if ((r == ECANCELED && *delayed_cancel == 0) ||
> + (r == EINTR && !can_eintr) || r == EAGAIN)
> + r = 0;
> + }
> + atomic_dec_int(&sem->waitcount);
> +
> + return r;
> +}
> +
> +/* always increment count */
> +int
> +_sem_post(sem_t sem)
> +{
> + membar_exit_before_atomic();
> + atomic_inc_int(&sem->value);
> + _wake(&sem->value, 1);
> + return 0;
> +}
> +
> +/*
> + * exported semaphores
> + */
> +int
> +sem_init(sem_t *semp, int pshared, unsigned int value)
> +{
> + sem_t sem;
> +
> + if (value > SEM_VALUE_MAX) {
> + errno = EINVAL;
> + return (-1);
> + }
> +
> + if (pshared) {
> + errno = EPERM;
> + return (-1);
> +#ifdef notyet
> + char name[SEM_RANDOM_NAME_LEN];
> + sem_t *sempshared;
> + int i;
> +
> + for (;;) {
> + for (i = 0; i < SEM_RANDOM_NAME_LEN - 1; i++)
> + name[i] = arc4random_uniform(2
Re: fdinsert(), take 2
Martin Pieuchot wrote:
> Here's a new version of my diff to remove the FIF_LARVAL flag.
>
> Larval files still exist, but at this stage they aren't present in
> `fd_ofiles[]'. That means we don't need specific tricks in fd_getfile()
> and fd_iterfile(). The idea is to put files in shared data structures
> once they are properly setup. For that we use a new function: fdinsert().
>
> This diff is rebased on top of the recent changes and it includes the
> following:
> - simplification of fd_inuse() pointed by visa@
> - Add a KASSERT() in fdinsert(), also suggested by visa@
> - remove references to FIF_LARVAL
> - use fd_getfile() & fdremove() to simplify fdrelease()
>
>
> This diff has been tested as part of the big "unlocking" diff I sent
> last week, so I'm confident, but reviews & tests are more than welcome!
>
> Ok?
Been running it for a while without any problems.
Mathieu.
>
> Index: kern/kern_exec.c
> ===
> RCS file: /cvs/src/sys/kern/kern_exec.c,v
> retrieving revision 1.195
> diff -u -p -r1.195 kern_exec.c
> --- kern/kern_exec.c 28 Apr 2018 03:13:04 - 1.195
> +++ kern/kern_exec.c 28 May 2018 09:41:15 -
> @@ -584,7 +584,7 @@ sys_execve(struct proc *p, void *v, regi
> struct vnode *vp;
> int indx;
>
> - if ((error = falloc(p, 0, &fp, &indx)) != 0)
> + if ((error = falloc(p, &fp, &indx)) != 0)
> break;
> #ifdef DIAGNOSTIC
> if (indx != i)
> @@ -607,10 +607,9 @@ sys_execve(struct proc *p, void *v, regi
> fp->f_type = DTYPE_VNODE;
> fp->f_ops = &vnops;
> fp->f_data = (caddr_t)vp;
> - FILE_SET_MATURE(fp, p);
> - } else {
> - FRELE(fp, p);
> + fdinsert(p->p_fd, indx, 0, fp);
> }
> + FRELE(fp, p);
> }
> fdpunlock(p->p_fd);
> if (error)
> Index: kern/exec_script.c
> ===
> RCS file: /cvs/src/sys/kern/exec_script.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 exec_script.c
> --- kern/exec_script.c2 May 2018 02:24:56 - 1.44
> +++ kern/exec_script.c28 May 2018 09:41:15 -
> @@ -170,17 +170,20 @@ check_shell:
> #endif
>
> fdplock(p->p_fd);
> - error = falloc(p, 0, &fp, &epp->ep_fd);
> - fdpunlock(p->p_fd);
> - if (error)
> + error = falloc(p, &fp, &epp->ep_fd);
> + if (error) {
> + fdpunlock(p->p_fd);
> goto fail;
> + }
>
> epp->ep_flags |= EXEC_HASFD;
> fp->f_type = DTYPE_VNODE;
> fp->f_ops = &vnops;
> fp->f_data = (caddr_t) scriptvp;
> fp->f_flag = FREAD;
> - FILE_SET_MATURE(fp, p);
> + fdinsert(p->p_fd, epp->ep_fd, 0, fp);
> + fdpunlock(p->p_fd);
> + FRELE(fp, p);
> }
>
> /* set up the parameters for the recursive check_exec() call */
> Index: kern/kern_descrip.c
> ===
> RCS file: /cvs/src/sys/kern/kern_descrip.c,v
> retrieving revision 1.159
> diff -u -p -r1.159 kern_descrip.c
> --- kern/kern_descrip.c 28 May 2018 08:55:11 - 1.159
> +++ kern/kern_descrip.c 28 May 2018 09:41:16 -
> @@ -144,6 +144,17 @@ find_last_set(struct filedesc *fd, int l
> return i;
> }
>
> +static __inline int
> +fd_inuse(struct filedesc *fdp, int fd)
> +{
> + u_int off = fd >> NDENTRYSHIFT;
> +
> + if (fdp->fd_lomap[off] & (1 << (fd & NDENTRYMASK)))
> + return 1;
> +
> + return 0;
> +}
> +
> static __inline void
> fd_used(struct filedesc *fdp, int fd)
> {
> @@ -190,7 +201,7 @@ fd_iterfile(struct file *fp, struct proc
> nfp = LIST_NEXT(fp, f_list);
>
> /* don't FREF when f_count == 0 to avoid race in fdrop() */
> - while (nfp != NULL && (nfp->f_count == 0 || !FILE_IS_USABLE(nfp)))
> + while (nfp != NULL && nfp->f_count == 0)
> nfp = LIST_NEXT(nfp, f_list);
> if (nfp != NULL)
> FREF(nfp);
> @@ -209,9 +220,6 @@ fd_getfile(struct filedesc *fdp, int fd)
> if ((u_int)fd >= fdp->fd_nfiles || (fp = fdp->fd_ofiles[fd]) == NULL)
> return (NULL);
>
> - if (!FILE_IS_USABLE(fp))
> - return (NULL);
> -
> FREF(fp);
> return (fp);
> }
> @@ -634,19 +642,17 @@ finishdup(struct proc *p, struct file *f
> return (EDEADLK);
> }
>
> - oldfp = fdp->fd_ofiles[new];
> - if (oldfp != NULL) {
> -
Re: bgpd ignore aspath with to large attributes
On Wed, May 30, 2018 at 03:18:45PM +0200, Claudio Jeker wrote: > This adds a protection to handle aspaths overly large attributes in > bgpd. The main idea is to protect other bgp routes downstream for > hitting the limit with is often not well catched. I am not sure this is sound logic. The BGP UPDATE limit has been 4096 for decades, and protocol developers are even looking into extending it beyond 4096. If a downstream router cannot handle the legal limit that is on them, they need to fix their non-compliance. > The limit is currently a bit arbitarily set to 4096 - 1024 which is > afaik the same limit that bird uses. Arbitrary limits are arbitrary, not sure I agree with how BIRD handles this either. Kind regards, Job
Re: [patch] Add kvm_close in mib_hrsystemprocs function
Hi Gerhard,
Thanks for your reply!
Yes, if no "kvm_close(kd);", there will be resource (memory, file
descriptor) leak. So hope you can commit it, thanks!
On 5/30/2018 4:49 PM, Gerhard Roth wrote:
> On Wed, 30 May 2018 16:25:55 +0800 Nan Xiao wrote:
>> Hi tech@,
>>
>> Maybe kvm_close is needed if kvm_getprocs returns NULL here? Sorry if I
>> am wrong, thanks!
>>
>> Index: mib.c
>> ===
>> RCS file: /cvs/src/usr.sbin/snmpd/mib.c,v
>> retrieving revision 1.87
>> diff -u -p -r1.87 mib.c
>> --- mib.c25 May 2018 08:23:15 - 1.87
>> +++ mib.c30 May 2018 08:15:19 -
>> @@ -516,8 +516,10 @@ mib_hrsystemprocs(struct oid *oid, struc
>> return (-1);
>>
>> if (kvm_getprocs(kd, KERN_PROC_ALL, 0,
>> -sizeof(struct kinfo_proc), &val) == NULL)
>> +sizeof(struct kinfo_proc), &val) == NULL) {
>> +kvm_close(kd);
>> return (-1);
>> +}
>>
>> *elm = ber_add_integer(*elm, val);
>> ber_set_header(*elm, BER_CLASS_APPLICATION, SNMP_T_GAUGE32);
>>
>
>
> Looks reasonable.
>
--
Best Regards
Nan Xiao
in_ioctl: ifr vs ifra
It turns out that the split is now such that 'ifra' is only used once in
in_ioctl() and 'ifr' is only used once in in_ioctl_change_ifaddr().
Is there a reason not to do this?
Index: sys/netinet/in.c
===
RCS file: /cvs/src/sys/netinet/in.c,v
retrieving revision 1.154
diff -u -p -r1.154 in.c
--- sys/netinet/in.c31 May 2018 07:39:19 - 1.154
+++ sys/netinet/in.c31 May 2018 07:47:23 -
@@ -214,7 +214,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
struct ifreq *ifr = (struct ifreq *)data;
struct ifaddr *ifa;
struct in_ifaddr *ia = NULL;
- struct in_aliasreq *ifra = (struct in_aliasreq *)data;
struct sockaddr_in oldaddr;
int error = 0;
@@ -305,7 +304,7 @@ in_ioctl(u_long cmd, caddr_t data, struc
}
ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr =
- ifra->ifra_addr.sin_addr.s_addr;
+ satosin(&ifr->ifr_addr)->sin_addr.s_addr;
break;
}
@@ -317,7 +316,6 @@ int
in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp,
int privileged)
{
- struct ifreq *ifr = (struct ifreq *)data;
struct ifaddr *ifa;
struct in_ifaddr *ia = NULL;
struct in_aliasreq *ifra = (struct in_aliasreq *)data;
@@ -369,7 +367,7 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
newifaddr = 0;
in_ifscrub(ifp, ia);
- error = in_ifinit(ifp, ia, satosin(&ifr->ifr_addr), newifaddr);
+ error = in_ifinit(ifp, ia, &ifra->ifra_addr, newifaddr);
if (error)
break;
dohooks(ifp->if_addrhooks, 0);
