Re: Port httpd(8) 'strip' directive to relayd(8)
Le Fri, Dec 11, 2020 at 10:53:56AM +, Olivier Cherrier a écrit :
>
> Hello tech@,
>
> Is there any interest for this feature to be commited?
> I find it very useful. Thank you Denis!
>
Here is an up to date diff, looking for OKs.
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.250
diff -u -p -r1.250 parse.y
--- parse.y 29 Dec 2020 19:48:06 - 1.250
+++ parse.y 3 Jan 2021 10:38:26 -
@@ -175,7 +175,7 @@ typedef struct {
%token LOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT PATH
%token PFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY REMOVE
%token REQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK SCRIPT SEND
-%token SESSION SOCKET SPLICE SSL STICKYADDR STYLE TABLE TAG TAGGED TCP
+%token SESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG TAGGED TCP
%token TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE
%token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD ECDHE
%token EDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES CHECKS
@@ -1549,6 +1549,20 @@ ruleopts : METHOD STRING
{
rule->rule_kv[keytype].kv_option = $2;
rule->rule_kv[keytype].kv_type = keytype;
}
+ | PATH STRIP NUMBER {
+ char*strip = NULL;
+
+ if ($3 < 0 || $3 > INT_MAX) {
+ yyerror("invalid strip number");
+ YYERROR;
+ }
+ if (asprintf(&strip, "%lld", $3) <= 0)
+ fatal("can't parse strip");
+ keytype = KEY_TYPE_PATH;
+ rule->rule_kv[keytype].kv_option = KEY_OPTION_STRIP;
+ rule->rule_kv[keytype].kv_value = strip;
+ rule->rule_kv[keytype].kv_type = keytype;
+ }
| QUERYSTR key_option STRING value {
switch ($2) {
case KEY_OPTION_APPEND:
@@ -2481,6 +2495,7 @@ lookup(char *s)
{ "ssl",SSL },
{ "state", STATE },
{ "sticky-address", STICKYADDR },
+ { "strip", STRIP },
{ "style", STYLE },
{ "table", TABLE },
{ "tag",TAG },
Index: relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.251
diff -u -p -r1.251 relay.c
--- relay.c 14 May 2020 17:27:38 - 1.251
+++ relay.c 3 Jan 2021 10:38:27 -
@@ -214,6 +214,9 @@ relay_ruledebug(struct relay_rule *rule)
case KEY_OPTION_LOG:
fprintf(stderr, "log ");
break;
+ case KEY_OPTION_STRIP:
+ fprintf(stderr, "strip ");
+ break;
case KEY_OPTION_NONE:
break;
}
@@ -227,13 +230,15 @@ relay_ruledebug(struct relay_rule *rule)
break;
}
+ int kvv = (kv->kv_option == KEY_OPTION_STRIP ||
+kv->kv_value == NULL);
fprintf(stderr, "%s%s%s%s%s%s ",
kv->kv_key == NULL ? "" : "\"",
kv->kv_key == NULL ? "" : kv->kv_key,
kv->kv_key == NULL ? "" : "\"",
- kv->kv_value == NULL ? "" : " value \"",
+ kvv ? "" : " value \"",
kv->kv_value == NULL ? "" : kv->kv_value,
- kv->kv_value == NULL ? "" : "\"");
+ kvv ? "" : "\"");
}
if (rule->rule_tablename[0])
Index: relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.79
diff -u -p -r1.79 relay_http.c
--- relay_http.c4 Sep 2020 13:09:14 - 1.79
+++ relay_http.c3 Jan 2021 10:38:27 -
@@ -77,6 +77,7 @@ intrelay_match_actions(struct ctl_rel
struct relay_rule *, struct kvlist *, struct kvlist *,
struct relay_table **);
voidrelay_httpdesc_free(struct http_descriptor *);
+char * server_root_strip(char *, int);
static struct relayd *env = NULL;
@@ -1421,14 +1422,16 @@ relay_httppath_test(struct ctl_relay_eve
if (cre->dir == RELAY_DIR_RESPONSE || kv->kv_type != KEY_TYPE_PATH)
return (0);
- else if (kv->kv_key == NULL)
- return (0);
- else if (fnmatch(kv->kv_key, desc->http_path, 0) == FNM_NOMATCH)
-
Re: acme-client(1): backup certs
On 2021/01/02 17:10, Chris Bennett wrote: > On Sat, Jan 02, 2021 at 05:23:11PM +0100, Florian Obser wrote: > > > > Create .1 backup files when acme-client is going to overwrite a > > certificate file. > > > > This files are not terribly big and it's convenient to keep one > > previous file around for example if one adds or removes domains to the > > certificate and then wants to revoke the previous one. > > > > (Note that it's kinda difficult to revoke the old certificate with > > acme-client currently. The whole revoke machinery needs to be > > overhauled. I have ideas...) > > > > Comments, OKs? > > > > Wait, I can have multiple, active certificates? One's that are in fact > different, such as domain.xxx and then add www.domain.xxx in another > certificate? > > If that's the case, then couldn't someone steal the old or new one and > use that to cause problems? What are you thinking would be stolen? The certificates themselves are public knowledge anyway - they are sent in full whenever someone connects to your TLS-based service and are available from Certificate Transparency log servers (https://crt.sh etc) - but they are useless without the private key. > Especially since DNS servers can take up to 48 hours to propagate changes > So getting rid of www.domain.xxx might not show up quickly enough. > And if I change IP addresses and they don't get propagated soon enough, > wouldn't someone be able to briefly spoof my site? letsencrypt (and I think probably all CAs) do uncached lookups from the authoritative servers for the domain, following the chain from the root servers, the usual problem with DNS servers returning outdated records is with bad recursive servers. If you have problems getting the authoritative servers giving out current information then that needs fixing, and isn't really a problem specific to CA validation.
Re: [PATCH] faq13, Using a Webcam
Stefan Hagen wrote: > If there are new developments, like a video group or a sysctl video.record > knob, I'll send delta patches to describe the behavior. I've updated the patch to cover the kern.video.record knob. I also used # instead of $ doas in code blocks. Index: faq/faq13.html === RCS file: /cvs/www/faq/faq13.html,v retrieving revision 1.221 diff -u -p -u -p -r1.221 faq13.html --- faq/faq13.html 26 Sep 2020 22:11:00 - 1.221 +++ faq/faq13.html 3 Jan 2021 11:49:50 - @@ -66,6 +66,7 @@ FAQ - Multimedia Choosing the Default Audio Device Debugging Audio Problems Using MIDI Instruments + Using a Webcam
Re: Thread local data setup and destruct
On Thu, Dec 31, 2020 at 05:54:06PM +0100, Alexander Bluhm wrote:
> On Tue, Dec 29, 2020 at 04:07:19PM +0100, Otto Moerbeek wrote:
> > This workds better, checking the flags does not work if the thread is
> > already on the road to desctruction.
>
> This diff survived a full regress run on amd64.
>
> bluhm
anybody wants to ok?
-Otto
>
> > Index: asr/asr.c
> > ===
> > RCS file: /cvs/src/lib/libc/asr/asr.c,v
> > retrieving revision 1.64
> > diff -u -p -r1.64 asr.c
> > --- asr/asr.c 6 Jul 2020 13:33:05 - 1.64
> > +++ asr/asr.c 29 Dec 2020 15:05:45 -
> > @@ -117,7 +117,7 @@ _asr_resolver_done(void *arg)
> > _asr_ctx_unref(ac);
> > return;
> > } else {
> > - priv = _THREAD_PRIVATE(_asr, _asr, &_asr);
> > + priv = _THREAD_PRIVATE_DT(_asr, _asr, NULL, &_asr);
> > if (*priv == NULL)
> > return;
> > asr = *priv;
> > @@ -128,6 +128,23 @@ _asr_resolver_done(void *arg)
> > free(asr);
> > }
> >
> > +static void
> > +_asr_resolver_done_tp(void *arg)
> > +{
> > + char buf[100];
> > + int len;
> > + struct asr **priv = arg;
> > + struct asr *asr;
> > +
> > + if (*priv == NULL)
> > + return;
> > + asr = *priv;
> > +
> > + _asr_ctx_unref(asr->a_ctx);
> > + free(asr);
> > + free(priv);
> > +}
> > +
> > void *
> > asr_resolver_from_string(const char *str)
> > {
> > @@ -349,7 +366,8 @@ _asr_use_resolver(void *arg)
> > }
> > else {
> > DPRINT("using thread-local resolver\n");
> > - priv = _THREAD_PRIVATE(_asr, _asr, &_asr);
> > + priv = _THREAD_PRIVATE_DT(_asr, _asr, _asr_resolver_done_tp,
> > + &_asr);
> > if (*priv == NULL) {
> > DPRINT("setting up thread-local resolver\n");
> > *priv = _asr_resolver();
> > Index: include/thread_private.h
> > ===
> > RCS file: /cvs/src/lib/libc/include/thread_private.h,v
> > retrieving revision 1.35
> > diff -u -p -r1.35 thread_private.h
> > --- include/thread_private.h13 Feb 2019 13:22:14 - 1.35
> > +++ include/thread_private.h29 Dec 2020 15:05:45 -
> > @@ -98,7 +98,8 @@ struct thread_callbacks {
> > void(*tc_mutex_destroy)(void **);
> > void(*tc_tag_lock)(void **);
> > void(*tc_tag_unlock)(void **);
> > - void*(*tc_tag_storage)(void **, void *, size_t, void *);
> > + void*(*tc_tag_storage)(void **, void *, size_t, void (*)(void *),
> > + void *);
> > __pid_t (*tc_fork)(void);
> > __pid_t (*tc_vfork)(void);
> > void(*tc_thread_release)(struct pthread *);
> > @@ -142,6 +143,7 @@ __END_HIDDEN_DECLS
> > #define _THREAD_PRIVATE_MUTEX_LOCK(name) do {} while (0)
> > #define _THREAD_PRIVATE_MUTEX_UNLOCK(name) do {} while (0)
> > #define _THREAD_PRIVATE(keyname, storage, error) &(storage)
> > +#define _THREAD_PRIVATE_DT(keyname, storage, dt, error)&(storage)
> > #define _MUTEX_LOCK(mutex) do {} while (0)
> > #define _MUTEX_UNLOCK(mutex) do {} while (0)
> > #define _MUTEX_DESTROY(mutex) do {} while (0)
> > @@ -168,7 +170,12 @@ __END_HIDDEN_DECLS
> > #define _THREAD_PRIVATE(keyname, storage, error) \
> > (_thread_cb.tc_tag_storage == NULL ? &(storage) : \
> > _thread_cb.tc_tag_storage(&(__THREAD_NAME(keyname)),\
> > - &(storage), sizeof(storage), error))
> > + &(storage), sizeof(storage), NULL, (error)))
> > +
> > +#define _THREAD_PRIVATE_DT(keyname, storage, dt, error)
> > \
> > + (_thread_cb.tc_tag_storage == NULL ? &(storage) : \
> > + _thread_cb.tc_tag_storage(&(__THREAD_NAME(keyname)),\
> > + &(storage), sizeof(storage), (dt), (error)))
> >
> > /*
> > * Macros used in libc to access mutexes.
> > Index: thread/rthread_cb.h
> > ===
> > RCS file: /cvs/src/lib/libc/thread/rthread_cb.h,v
> > retrieving revision 1.2
> > diff -u -p -r1.2 rthread_cb.h
> > --- thread/rthread_cb.h 5 Sep 2017 02:40:54 - 1.2
> > +++ thread/rthread_cb.h 29 Dec 2020 15:05:45 -
> > @@ -35,5 +35,5 @@ void _thread_mutex_unlock(void **);
> > void _thread_mutex_destroy(void **);
> > void _thread_tag_lock(void **);
> > void _thread_tag_unlock(void **);
> > -void *_thread_tag_storage(void **, void *, size_t, void *);
> > +void *_thread_tag_storage(void **, void *, size_t, void (*)(void*),
> > void *);
> > __END_HIDDEN_DECLS
> > Index: thread/rthread_libc.c
> > ===
> > RCS file: /cvs/src/lib/libc/thread/rthread_libc.c,v
>
snmpd(8) make traphandler more secure
Right now snmpd's traphandler_parse and traphandler_v1translate are just
weird to me and do very little ASN1 checking. Since I want to remove the
traphandler process anyway and the current code structue is in the way,
this diff is a logical first step towards that goal.
The new traphandler_v1translate should now be properly build around
RFC3584 section 3.1 and I've choosen to disable the proxy addition,
since we don't forward it to another host, but it does work for me and
can be enabled for this scenario if people feel strong about it.
Another major change people should be aware of is when running snmpd
with -N the sysUpTime and snmpTrapOID parameters will now be numerical
(where these were hardcode printed before).
A minor thing people should be aware of is that the new code is a lot
more strict on its ASN1 validation, so if you use it with crappy
SNMP entities that send broken packets, you're out of luck.
Also, traphandler_parse is now called only once, which should help in my
next step in removing the traphandler process altogether.
OK?
martijn@
Index: traphandler.c
===
RCS file: /cvs/src/usr.sbin/snmpd/traphandler.c,v
retrieving revision 1.18
diff -u -p -r1.18 traphandler.c
--- traphandler.c 6 Sep 2020 15:51:28 - 1.18
+++ traphandler.c 3 Jan 2021 14:20:17 -
@@ -28,6 +28,7 @@
#include
#include
+#include
#include
#include
#include
@@ -50,13 +51,12 @@ int traphandler_bind(struct address *);
voidtraphandler_recvmsg(int, short, void *);
int traphandler_priv_recvmsg(struct privsep_proc *, struct imsg *);
int traphandler_fork_handler(struct privsep_proc *, struct imsg *);
-int traphandler_parse(char *, size_t, struct ber_element **,
- struct ber_element **, u_int *, struct ber_oid *);
-voidtraphandler_v1translate(struct ber_oid *, u_int, u_int);
-
+int traphandler_parse(struct ber_element *, char *, struct sockaddr *);
+struct ber_element *
+traphandler_v1translate(struct ber_element *, char *, int);
int trapcmd_cmp(struct trapcmd *, struct trapcmd *);
voidtrapcmd_exec(struct trapcmd *, struct sockaddr *,
- struct ber_element *, char *, u_int);
+ struct ber_element *);
char *traphandler_hostname(struct sockaddr *, int);
@@ -102,7 +102,7 @@ traphandler_init(struct privsep *ps, str
/* listen for SNMP trap messages */
TAILQ_FOREACH(h, &env->sc_addresses, entry) {
event_set(&h->ev, h->fd, EV_READ|EV_PERSIST,
- traphandler_recvmsg, ps);
+ traphandler_recvmsg, NULL);
event_add(&h->ev, NULL);
}
}
@@ -180,104 +180,216 @@ snmpd_dispatch_traphandler(int fd, struc
void
traphandler_recvmsg(int fd, short events, void *arg)
{
- struct privsep *ps = arg;
+ struct ber ber = {0};
+ struct ber_element *msg = NULL, *pdu;
char buf[8196];
- struct iovec iov[2];
struct sockaddr_storage ss;
socklen_tslen;
ssize_t n;
- struct ber_element *req, *iter;
- struct ber_oid trapoid;
- u_intuptime;
+ int vers;
+ char*community;
slen = sizeof(ss);
if ((n = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&ss,
&slen)) == -1)
return;
- if (traphandler_parse(buf, n, &req, &iter, &uptime, &trapoid) == -1)
- goto done;
+ ober_set_application(&ber, smi_application);
+ ober_set_readbuf(&ber, buf, n);
- iov[0].iov_base = &ss;
- iov[0].iov_len = ss.ss_len;
- iov[1].iov_base = buf;
- iov[1].iov_len = n;
+ if ((msg = ober_read_elements(&ber, NULL)) == NULL)
+ goto parsefail;
- /* Forward it to the parent process */
- if (proc_composev(ps, PROC_PARENT, IMSG_ALERT, iov, 2) == -1)
- goto done;
+ if (ober_scanf_elements(msg, "{dse", &vers, &community, &pdu) == -1)
+ goto parsefail;
- done:
- if (req != NULL)
- ober_free_elements(req);
- return;
+ switch (vers) {
+ case SNMP_V1:
+ if (pdu->be_type != SNMP_C_TRAP)
+ goto parsefail;
+ break;
+ case SNMP_V2:
+ if (pdu->be_type != SNMP_C_TRAPV2)
+ goto parsefail;
+ break;
+ default:
+ goto parsefail;
+ }
+
+ (void)traphandler_parse(pdu, community, (struct sockaddr *)&ss);
+
+parsefail:
+ ober_free(&ber);
+ if (msg != NULL)
+ ober_free_elements(msg);
}
/*
* Validate received message
*/
int
-traphandler_parse(char *buf, size_t n, struct ber_element **req,
-struct ber_element **vbinds, u_int *
Re: libc/regex: turn unsafe macros to inline functions
> Is there a reason not to do
>
> return (cs->ptr[(uch)c] & cs->mask) != 0;
>
> This would allow us to get rid of the !! construct in regcomp.c
Why not. What about that?
Index: regcomp.c
===
RCS file: /OpenBSD/src/lib/libc/regex/regcomp.c,v
retrieving revision 1.41
diff -u -p -r1.41 regcomp.c
--- regcomp.c 31 Dec 2020 17:24:05 - 1.41
+++ regcomp.c 3 Jan 2021 16:43:50 -
@@ -1101,7 +1099,7 @@ freezeset(struct parse *p, cset *cs)
if (cs2->hash == h && cs2 != cs) {
/* maybe */
for (i = 0; i < css; i++)
- if (!!CHIN(cs2, i) != !!CHIN(cs, i))
+ if (CHIN(cs2, i) != CHIN(cs, i))
break; /* no */
if (i == css)
break; /* yes */
Index: regex2.h
===
RCS file: /OpenBSD/src/lib/libc/regex/regex2.h,v
retrieving revision 1.10
diff -u -p -r1.10 regex2.h
--- regex2.h31 Dec 2020 17:20:19 - 1.10
+++ regex2.h3 Jan 2021 16:43:50 -
@@ -107,10 +107,26 @@ typedef struct {
uch mask; /* bit within array */
uch hash; /* hash code */
} cset;
-/* note that CHadd and CHsub are unsafe, and CHIN doesn't yield 0/1 */
-#defineCHadd(cs, c)((cs)->ptr[(uch)(c)] |= (cs)->mask, (cs)->hash
+= (c))
-#defineCHsub(cs, c)((cs)->ptr[(uch)(c)] &= ~(cs)->mask, (cs)->hash
-= (c))
-#defineCHIN(cs, c) ((cs)->ptr[(uch)(c)] & (cs)->mask)
+
+static inline void
+CHadd(cset *cs, char c)
+{
+ cs->ptr[(uch)c] |= cs->mask;
+ cs->hash += c;
+}
+
+static inline void
+CHsub(cset *cs, char c)
+{
+ cs->ptr[(uch)c] &= ~cs->mask;
+ cs->hash -= c;
+}
+
+static inline int
+CHIN(const cset *cs, char c)
+{
+ return (cs->ptr[(uch)c] & cs->mask) != 0;
+}
/*
* main compiled-expression structure
Re: libc/regex: turn unsafe macros to inline functions
On Sun, Jan 03, 2021 at 04:45:30PM +, Miod Vallat wrote:
> > Is there a reason not to do
> >
> > return (cs->ptr[(uch)c] & cs->mask) != 0;
> >
> > This would allow us to get rid of the !! construct in regcomp.c
>
> Why not. What about that?
Thanks. Here's the diff rebased on top of -current. This is
ok tb
Index: regcomp.c
===
RCS file: /cvs/src/lib/libc/regex/regcomp.c,v
retrieving revision 1.42
diff -u -p -r1.42 regcomp.c
--- regcomp.c 2 Jan 2021 20:42:01 - 1.42
+++ regcomp.c 3 Jan 2021 16:48:59 -
@@ -1099,7 +1099,7 @@ freezeset(struct parse *p, cset *cs)
if (cs2->hash == h && cs2 != cs) {
/* maybe */
for (i = 0; i < css; i++)
- if (!!CHIN(cs2, i) != !!CHIN(cs, i))
+ if (CHIN(cs2, i) != CHIN(cs, i))
break; /* no */
if (i == css)
break; /* yes */
Index: regex2.h
===
RCS file: /cvs/src/lib/libc/regex/regex2.h,v
retrieving revision 1.11
diff -u -p -r1.11 regex2.h
--- regex2.h3 Jan 2021 10:50:02 - 1.11
+++ regex2.h3 Jan 2021 16:49:20 -
@@ -122,10 +122,10 @@ CHsub(cset *cs, char c)
cs->hash -= c;
}
-static inline uch
+static inline int
CHIN(const cset *cs, char c)
{
- return cs->ptr[(uch)c] & cs->mask;
+ return (cs->ptr[(uch)c] & cs->mask) != 0;
}
/*
Re: libc/regex: turn unsafe macros to inline functions
On Sun, 03 Jan 2021 17:51:56 +0100, Theo Buehler wrote: > Thanks. Here's the diff rebased on top of -current. This is > > ok tb OK millert@ as well if you'd like to do the honors. - todd
Re: pf route-to issues
On Sun, Jan 03, 2021 at 02:00:00PM +1000, David Gwynne wrote:
> On Tue, Oct 20, 2020 at 09:27:09AM +1000, David Gwynne wrote:
> We've been running this diff in production for the last couple of
> months, and it's been solid for us so far. Ignoring the fixes for
> crashes, I personally find it a lot more usable than the current
> route-to rules too.
>
> Can I commit it?
The diff is quite large and does multiple things at a time.
In general I also did not understand why I have to say em0@10.0.0.1
for routing and it took me a while to figure out what to put into
pf.conf. I use this syntax in /usr/src/regress/sys/net/pf_forward/pf.conf.
This has to be fixed after this goes in. I will care about regress
as this test is quite complex an needs several machines to setup.
I am currently running a full regress to find more fallout.
I do not use pfsync, so I cannot say what the consequences of the
change are in this area. Also I don't know why pf-route interfaces
were designed in such a strange way.
>From a user perspective it is not clear, why route-to should not
work together with no-state. So we should either fix it or document
it and add a check in the parser. Is fixing hard?
Are we losing any other features apart from this strange arp reuse
you described in your mail?
There is some refactoring in your diff. I splitted it to make
review easier. I think this should go in first. Note that the
pf_state variable is called st in if_pfsync.c. Can we be consistent
here? Is the pfsync_state properly aligned? During import it comes
from an mbuf.
Is there anything else that can be split out easily?
bluhm
Index: net/if_pfsync.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
retrieving revision 1.279
diff -u -p -r1.279 if_pfsync.c
--- net/if_pfsync.c 12 Dec 2020 11:49:02 - 1.279
+++ net/if_pfsync.c 3 Jan 2021 17:16:55 -
@@ -612,7 +612,7 @@ pfsync_state_import(struct pfsync_state
st->rtableid[PF_SK_STACK] = ntohl(sp->rtableid[PF_SK_STACK]);
/* copy to state */
- bcopy(&sp->rt_addr, &st->rt_addr, sizeof(st->rt_addr));
+ st->rt_addr = sp->rt_addr;
st->creation = getuptime() - ntohl(sp->creation);
st->expire = getuptime();
if (ntohl(sp->expire)) {
@@ -1843,6 +1843,7 @@ pfsync_undefer(struct pfsync_deferral *p
{
struct pfsync_softc *sc = pfsyncif;
struct pf_pdesc pdesc;
+ struct pf_state *st = pd->pd_st;
NET_ASSERT_LOCKED();
@@ -1852,35 +1853,32 @@ pfsync_undefer(struct pfsync_deferral *p
TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
sc->sc_deferred--;
- CLR(pd->pd_st->state_flags, PFSTATE_ACK);
+ CLR(st->state_flags, PFSTATE_ACK);
if (drop)
m_freem(pd->pd_m);
else {
- if (pd->pd_st->rule.ptr->rt == PF_ROUTETO) {
- if (pf_setup_pdesc(&pdesc,
- pd->pd_st->key[PF_SK_WIRE]->af,
- pd->pd_st->direction, pd->pd_st->rt_kif,
- pd->pd_m, NULL) != PF_PASS) {
+ if (st->rule.ptr->rt == PF_ROUTETO) {
+ if (pf_setup_pdesc(&pdesc, st->key[PF_SK_WIRE]->af,
+ st->direction, st->kif, pd->pd_m, NULL) !=
+ PF_PASS) {
m_freem(pd->pd_m);
goto out;
}
- switch (pd->pd_st->key[PF_SK_WIRE]->af) {
+ switch (st->key[PF_SK_WIRE]->af) {
case AF_INET:
- pf_route(&pdesc,
- pd->pd_st->rule.ptr, pd->pd_st);
+ pf_route(&pdesc, st->rule.ptr, st);
break;
#ifdef INET6
case AF_INET6:
- pf_route6(&pdesc,
- pd->pd_st->rule.ptr, pd->pd_st);
+ pf_route6(&pdesc, st->rule.ptr, st);
break;
#endif /* INET6 */
default:
- unhandled_af(pd->pd_st->key[PF_SK_WIRE]->af);
+ unhandled_af(st->key[PF_SK_WIRE]->af);
}
pd->pd_m = pdesc.m;
} else {
- switch (pd->pd_st->key[PF_SK_WIRE]->af) {
+ switch (st->key[PF_SK_WIRE]->af) {
case AF_INET:
ip_output(pd->pd_m, NULL, NULL, 0, NULL, NULL,
0);
@@ -1892,12 +1890,12 @@ pfsync_undefer(struct pfsync_deferral *p
break;
#endif /* INET6 */
default:
- unhandled_af(pd->pd_st->key[PF_SK_WIRE
libc/regex: drop debug helpers
regex(3) documents non-standard extensions REG_ITOA and REG_ATOI to
regerror(). In the OpenBSD tree, the only use of them is by the regress
test, so why not move that specific code to the regress test and shrink
libc a bit - remember that this code is present in the installation
media through grep(1).
Assuming there are no objections against this, it would be worth trying
a ports build with this diff to confirm there are no 3rd-party users of
these extensions.
Index: include/regex.h
===
RCS file: /OpenBSD/src/include/regex.h,v
retrieving revision 1.7
diff -u -p -r1.7 regex.h
--- include/regex.h 5 Dec 2012 23:19:57 - 1.7
+++ include/regex.h 3 Jan 2021 17:48:03 -
@@ -83,8 +83,6 @@ typedef struct {
#defineREG_EMPTY 14
#defineREG_ASSERT 15
#defineREG_INVARG 16
-#defineREG_ATOI255 /* convert name to number (!) */
-#defineREG_ITOA0400/* convert number to name (!) */
/* regexec() flags */
#defineREG_NOTBOL 1
Index: lib/libc/regex/regerror.c
===
RCS file: /OpenBSD/src/lib/libc/regex/regerror.c,v
retrieving revision 1.15
diff -u -p -r1.15 regerror.c
--- lib/libc/regex/regerror.c 30 Dec 2020 08:56:38 - 1.15
+++ lib/libc/regex/regerror.c 3 Jan 2021 17:48:03 -
@@ -48,26 +48,25 @@ static const char *regatoi(const regex_t
static const struct rerr {
int code;
- const char *name;
const char *explain;
} rerrs[] = {
- { REG_NOMATCH, "REG_NOMATCH", "regexec() failed to match" },
- { REG_BADPAT, "REG_BADPAT", "invalid regular expression" },
- { REG_ECOLLATE, "REG_ECOLLATE", "invalid collating element" },
- { REG_ECTYPE, "REG_ECTYPE", "invalid character class" },
- { REG_EESCAPE, "REG_EESCAPE", "trailing backslash (\\)" },
- { REG_ESUBREG, "REG_ESUBREG", "invalid backreference number" },
- { REG_EBRACK, "REG_EBRACK", "brackets ([ ]) not balanced" },
- { REG_EPAREN, "REG_EPAREN", "parentheses not balanced" },
- { REG_EBRACE, "REG_EBRACE", "braces not balanced" },
- { REG_BADBR,"REG_BADBR","invalid repetition count(s)" },
- { REG_ERANGE, "REG_ERANGE", "invalid character range" },
- { REG_ESPACE, "REG_ESPACE", "out of memory" },
- { REG_BADRPT, "REG_BADRPT", "repetition-operator operand invalid" },
- { REG_EMPTY,"REG_EMPTY","empty (sub)expression" },
- { REG_ASSERT, "REG_ASSERT", "\"can't happen\" -- you found a bug" },
- { REG_INVARG, "REG_INVARG", "invalid argument to regex routine" },
- { 0,"", "*** unknown regexp error code ***" }
+ { REG_NOMATCH, "regexec() failed to match" },
+ { REG_BADPAT, "invalid regular expression" },
+ { REG_ECOLLATE, "invalid collating element" },
+ { REG_ECTYPE, "invalid character class" },
+ { REG_EESCAPE, "trailing backslash (\\)" },
+ { REG_ESUBREG, "invalid backreference number" },
+ { REG_EBRACK, "brackets ([ ]) not balanced" },
+ { REG_EPAREN, "parentheses not balanced" },
+ { REG_EBRACE, "braces not balanced" },
+ { REG_BADBR,"invalid repetition count(s)" },
+ { REG_ERANGE, "invalid character range" },
+ { REG_ESPACE, "out of memory" },
+ { REG_BADRPT, "repetition-operator operand invalid" },
+ { REG_EMPTY,"empty (sub)expression" },
+ { REG_ASSERT, "\"can't happen\" -- you found a bug" },
+ { REG_INVARG, "invalid argument to regex routine" },
+ { 0,"unknown regexp error code ***" }
};
/*
@@ -79,51 +78,15 @@ regerror(int errcode, const regex_t *pre
{
const struct rerr *r;
size_t len;
- int target = errcode &~ REG_ITOA;
- const char *s;
- char convbuf[50];
- if (errcode == REG_ATOI)
- s = regatoi(preg, convbuf, sizeof convbuf);
- else {
- for (r = rerrs; r->code != 0; r++)
- if (r->code == target)
- break;
-
- if (errcode®_ITOA) {
- if (r->code != 0) {
- assert(strlen(r->name) < sizeof(convbuf));
- (void) strlcpy(convbuf, r->name, sizeof
convbuf);
- } else
- (void)snprintf(convbuf, sizeof convbuf,
- "REG_0x%x", target);
- s = convbuf;
- } else
- s = r->explain;
- }
+ for (r = rerrs; r->code != 0; r++)
+ if (r->code == errcode)
+ break;
if (errbuf_size != 0)
- len = strlcpy(errbuf, s, errbuf_size);
+ len = strlcpy(err
Re: compress sparc64 bsd.rd
On Wed, Dec 30, 2020 at 08:37:49PM +, Miod Vallat wrote:
> Up until 6.5, sparc64 bsd.rd were gzipped kernels. This got lost during
> the Great Installation Media Unification of the 6.6 release cycle, and
> since then bsd.rd are uncompressed.
> The following diff ought to fix this and bring back sparc64 netboot
> times down to acceptable times.
Since this change went in, bsd.rd doesn't boot unless I uncompress it first.
upgrade detected: switching to /bsd.upgrade
Trying /bsd.upgrade...
NOTE: random seed is being reused.
Booting /pci@400/pci@2/pci@0/pci@c/nvme@0/disk@1:a/bsd.upgrade
4246528@0x100+5120@0x140cc00+3248796@0x1c0+945508@0x1f1929c
OF_map_phys(ff84,8192,fee5,-1) failed
no space for symbol table
Program terminated
--Kurt
> Index: miniroot/Makefile
> ===
> RCS file: /OpenBSD/src/distrib/sparc64/miniroot/Makefile,v
> retrieving revision 1.23
> diff -u -p -r1.23 Makefile
> --- miniroot/Makefile 18 May 2020 06:20:44 - 1.23
> +++ miniroot/Makefile 30 Dec 2020 20:35:55 -
> @@ -100,7 +100,7 @@ unconfig:
>
> .ifdef RELEASEDIR
> install:
> - cp bsd.rd ${RELEASEDIR}/bsd.rd
> + cp bsd.gz ${RELEASEDIR}/bsd.rd
> chmod a+r ${RELEASEDIR}/bsd.rd
> cp ${FS} ${RELEASEDIR}
> cp ${CDROM} ${RELEASEDIR}
>
Re: compress sparc64 bsd.rd
> Since this change went in, bsd.rd doesn't boot unless I uncompress it first. > > upgrade detected: switching to /bsd.upgrade > Trying /bsd.upgrade... > NOTE: random seed is being reused. > Booting /pci@400/pci@2/pci@0/pci@c/nvme@0/disk@1:a/bsd.upgrade > 4246528@0x100+5120@0x140cc00+3248796@0x1c0+945508@0x1f1929c > OF_map_phys(ff84,8192,fee5,-1) failed > no space for symbol table > Program terminated What machine and OpenBoot version are you using? gzipped kernels work here (as they used to) on: Sun Ultra 1 UPA/SBus (UltraSPARC 167MHz), No Keyboard OpenBoot 3.1, 128 MB memory installed, Serial #8592590. Ethernet address 8:0:20:83:1c:ce, Host ID: 80831cce. Rebooting with command: boot bsd.rd.gz Boot device: /sbus/SUNW,fas@e,880/sd@0,0 File and args: bsd.rd.gz OpenBSD IEEE 1275 Bootblock 2.1 ..>> OpenBSD BOOT 1.21 Booting /sbus@1f,0/SUNW,fas@e,880/sd@0,0:a/bsd.rd.gz 4249968@0x100+1680@0x140d970+3249820@0x1c0+944484@0x1f1969c symbols @ 0xffe88400 249148+165+369384+224195 start=0x100 console is /sbus@1f,0/zs@f,110:a Copyright (c) 1982, 1986, 1989, 1991, 1993 The Regents of the University of California. All rights reserved. Copyright (c) 1995-2020 OpenBSD. All rights reserved. https://www.OpenBSD.org (etc, etc)
Re: acme-client(1): backup certs
ok
Florian Obser(flor...@openbsd.org) on 2021.01.02 17:23:11 +0100:
>
> Create .1 backup files when acme-client is going to overwrite a
> certificate file.
>
> This files are not terribly big and it's convenient to keep one
> previous file around for example if one adds or removes domains to the
> certificate and then wants to revoke the previous one.
>
> (Note that it's kinda difficult to revoke the old certificate with
> acme-client currently. The whole revoke machinery needs to be
> overhauled. I have ideas...)
>
> Comments, OKs?
>
> diff --git acme-client.conf.5 acme-client.conf.5
> index 3c5fd1c2362..3fdd40a5eb0 100644
> --- acme-client.conf.5
> +++ acme-client.conf.5
> @@ -149,6 +149,11 @@ The filename of the certificate that will be issued.
> This is optional if
> .Ar domain full chain certificate
> is specified.
> +A backup with name
> +.Ar file.1
> +is created if
> +.Ar file
> +exists.
> .It Ic domain chain certificate Ar file
> The filename in which to store the certificate chain
> that will be returned by the certificate authority.
> @@ -156,6 +161,11 @@ It needs to be in the same directory as the
> .Ar domain certificate
> (or in a subdirectory) and can be specified as a relative or absolute path.
> This setting is optional.
> +A backup with name
> +.Ar file.1
> +is created if
> +.Ar file
> +exists.
> .It Ic domain full chain certificate Ar file
> The filename in which to store the full certificate chain
> that will be returned by the certificate authority.
> @@ -170,6 +180,11 @@ in one file, and is required by most browsers.
> This is optional if
> .Ar domain certificate
> is specified.
> +A backup with name
> +.Ar file.1
> +is created if
> +.Ar file
> +exists.
> .It Ic sign with Ar authority
> The certificate authority (as declared above in the
> .Sx AUTHORITIES
> diff --git fileproc.c fileproc.c
> index b7cdff5525d..cc3aa293712 100644
> --- fileproc.c
> +++ fileproc.c
> @@ -34,6 +34,19 @@ serialise(const char *real, const char *v, size_t vsz,
> const char *v2, size_t v2
> int fd;
> char *tmp;
>
> + /* create backup hardlink */
> + if (asprintf(&tmp, "%s.1", real) == -1) {
> + warn("asprintf");
> + return 0;
> + }
> + (void) unlink(tmp);
> + if (link(real, tmp) == -1 && errno != ENOENT) {
> + warn("link");
> + free(tmp);
> + return 0;
> + }
> + free(tmp);
> +
> /*
>* Write into backup location, overwriting.
>* Then atomically do the rename.
>
> --
> I'm not entirely sure you are real.
>
Re: compress sparc64 bsd.rd
On Sun, Jan 03, 2021 at 07:19:51PM +, Miod Vallat wrote: > > Since this change went in, bsd.rd doesn't boot unless I uncompress it first. > > upgrade detected: switching to /bsd.upgrade > > Trying /bsd.upgrade... > > NOTE: random seed is being reused. > > Booting /pci@400/pci@2/pci@0/pci@c/nvme@0/disk@1:a/bsd.upgrade > > 4246528@0x100+5120@0x140cc00+3248796@0x1c0+945508@0x1f1929c > > OF_map_phys(ff84,8192,fee5,-1) failed > > no space for symbol table > > Program terminated > What machine and OpenBoot version are you using? > gzipped kernels work here (as they used to) on: > Sun Ultra 1 UPA/SBus (UltraSPARC 167MHz), No Keyboard > OpenBoot 3.1, 128 MB memory installed, Serial #8592590. > Ethernet address 8:0:20:83:1c:ce, Host ID: 80831cce. T4-1 LDOM. One of the build cluster. SPARC T4-1, No Keyboard Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights reserved. OpenBoot 4.38.9, 7.5000 GB memory available, Serial #83498844. Ethernet address 0:14:4f:fa:17:5c, Host ID: 84fa175c. Boot device: /pci@400/pci@1/pci@0/pci@0/nvme@0/disk@1 File and args: OpenBSD IEEE 1275 Bootblock 2.1 ..>> OpenBSD BOOT 1.21 ERROR: /iscsi-hba: No iscsi-network-bootpath property upgrade detected: switching to /bsd.upgrade Trying /bsd.upgrade... NOTE: random seed is being reused. Booting /pci@400/pci@1/pci@0/pci@0/nvme@0/disk@1:a/bsd.upgrade 4246528@0x100+5120@0x140cc00+3248796@0x1c0+945508@0x1f1929c OF_map_phys(3f842000,8192,fee52000,-1) failed no space for symbol table Program terminated > Rebooting with command: boot bsd.rd.gz This is interesting. The change has it just being named bsd.rd, without the .gz. --Kurt > Boot device: /sbus/SUNW,fas@e,880/sd@0,0 File and args: bsd.rd.gz > OpenBSD IEEE 1275 Bootblock 2.1 > ..>> OpenBSD BOOT 1.21 > Booting /sbus@1f,0/SUNW,fas@e,880/sd@0,0:a/bsd.rd.gz > 4249968@0x100+1680@0x140d970+3249820@0x1c0+944484@0x1f1969c > symbols @ 0xffe88400 249148+165+369384+224195 start=0x100 > console is /sbus@1f,0/zs@f,110:a > Copyright (c) 1982, 1986, 1989, 1991, 1993 > The Regents of the University of California. All rights reserved. > Copyright (c) 1995-2020 OpenBSD. All rights reserved. https://www.OpenBSD.org > (etc, etc)
pipex(4): remove unused `pipex_iface_context' struct
Index: sys/net/pipex.h
===
RCS file: /cvs/src/sys/net/pipex.h,v
retrieving revision 1.29
diff -u -p -r1.29 pipex.h
--- sys/net/pipex.h 2 Jan 2021 13:15:15 - 1.29
+++ sys/net/pipex.h 3 Jan 2021 20:10:47 -
@@ -177,22 +177,8 @@ extern int pipex_enable;
struct pipex_session;
-/* pipex context for a interface
- *
- * Locks used to protect struct members:
- * I immutable after creation
- * N net lock
- */
-struct pipex_iface_context {
- u_int ifindex;/* [I] outer interface index */
- u_int pipexmode; /* [N] pipex mode */
- /* [I] virtual pipex_session entry for multicast routing */
- struct pipex_session *multicast_session;
-};
-
__BEGIN_DECLS
void pipex_init (void);
-void pipex_iface_fini (struct pipex_iface_context *);
struct pipex_session *pipex_pppoe_lookup_session (struct mbuf *);
struct mbuf *pipex_pppoe_input (struct mbuf *, struct pipex_session
*);
Index: sys/net/pipex_local.h
===
RCS file: /cvs/src/sys/net/pipex_local.h,v
retrieving revision 1.40
diff -u -p -r1.40 pipex_local.h
--- sys/net/pipex_local.h 27 Aug 2020 10:47:52 - 1.40
+++ sys/net/pipex_local.h 3 Jan 2021 20:10:47 -
@@ -384,8 +384,6 @@ void pipex_rele_session
int pipex_link_session(struct pipex_session *,
struct ifnet *, void *);
void pipex_unlink_session(struct pipex_session *);
-int pipex_close_session (struct pipex_session_close_req *,
- struct pipex_iface_context *);
int pipex_config_session (struct pipex_session_config_req *,
void *);
int pipex_get_stat (struct pipex_session_stat_req *,
Re: compress sparc64 bsd.rd
> > Rebooting with command: boot bsd.rd.gz > > This is interesting. The change has it just being named bsd.rd, without the > .gz. That's just me testing a compressed bsd.rd.
Re: libc/regex: safer pointer arithmetic
> regcomp.c uses the "start + count < end" idiom to check that there are > "count" bytes available in an array of char "start" and "end" both point > to. > > This is fine, unless "start + count" goes beyond the last element of the > array. In this case, pedantic interpretation of the C standard makes > the comparison of such a pointer against "end" undefined, and optimizers > from hell will happily remove as much code as possible because of this. I am only noticing now that llvm contains a copy of OpenBSD's libc regex code (with an llvm_ prefix to the public interfaces) in its llvmSupport library. I am thus surprised that one of their sanitizers did not expose that wrong construct already. I'll report this to the llvm project tomorrow. In the meantime, under OpenBSD, it might be worth investigating shutting that copy and having llvm_re* aliases of libc's re* functions, if only to make the code smaller.
Re: snmp - remove BER_TYPE_BOOLEAN
ping
On Mon, 2020-12-14 at 12:13 +0100, Martijn van Duren wrote:
> I can't find any reference in RFC2578 for a boolean type, nor have I
> seen it in the wild and the TruthValue diff I just committed give me a
> strong indication that this was added without any real reason.
>
> OK to remove?
>
> martijn@
>
> Index: usr.bin/snmp/smi.c
> ===
> RCS file: /cvs/src/usr.bin/snmp/smi.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 smi.c
> --- usr.bin/snmp/smi.c 14 Dec 2020 07:44:26 - 1.13
> +++ usr.bin/snmp/smi.c 14 Dec 2020 11:12:29 -
> @@ -95,9 +95,6 @@ smi_debug_elements(struct ber_element *r
> case BER_TYPE_EOC:
> fprintf(stderr, "end-of-content");
> break;
> - case BER_TYPE_BOOLEAN:
> - fprintf(stderr, "boolean");
> - break;
> case BER_TYPE_INTEGER:
> fprintf(stderr, "integer");
> break;
> @@ -196,9 +193,6 @@ smi_debug_elements(struct ber_element *r
> goto invalid;
>
> switch (root->be_encoding) {
> - case BER_TYPE_BOOLEAN:
> - fprintf(stderr, "%s", value);
> - break;
> case BER_TYPE_INTEGER:
> case BER_TYPE_ENUMERATED:
> fprintf(stderr, "value %s", value);
> @@ -255,7 +249,6 @@ smi_print_element(struct ber_oid *oid, s
> struct textconv tckey;
> size_t len, i, slen;
> long long v, ticks;
> - int d;
> int is_hex = 0, ret;
> struct ber_oid o;
> char strbuf[BUFSIZ];
> @@ -277,17 +270,6 @@ smi_print_element(struct ber_oid *oid, s
> }
>
> switch (root->be_encoding) {
> - case BER_TYPE_BOOLEAN:
> - if (ober_get_boolean(root, &d) == -1)
> - goto fail;
> - if (print_hint) {
> - if (asprintf(&str, "INTEGER: %s(%d)",
> - d ? "true" : "false", d) == -1)
> - goto fail;
> - } else
> - if (asprintf(&str, "%s", d ? "true" : "false") == -1)
> - goto fail;
> - break;
> case BER_TYPE_INTEGER:
> case BER_TYPE_ENUMERATED:
> if (ober_get_integer(root, &v) == -1)
> Index: usr.sbin/snmpd/smi.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/smi.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 smi.c
> --- usr.sbin/snmpd/smi.c24 Oct 2019 12:39:27 - 1.27
> +++ usr.sbin/snmpd/smi.c14 Dec 2020 11:12:29 -
> @@ -317,9 +317,6 @@ smi_debug_elements(struct ber_element *r
> case BER_TYPE_EOC:
> fprintf(stderr, "end-of-content");
> break;
> - case BER_TYPE_BOOLEAN:
> - fprintf(stderr, "boolean");
> - break;
> case BER_TYPE_INTEGER:
> fprintf(stderr, "integer");
> break;
> @@ -417,9 +414,6 @@ smi_debug_elements(struct ber_element *r
> goto invalid;
>
> switch (root->be_encoding) {
> - case BER_TYPE_BOOLEAN:
> - fprintf(stderr, "%s", value);
> - break;
> case BER_TYPE_INTEGER:
> case BER_TYPE_ENUMERATED:
> fprintf(stderr, "value %s", value);
> @@ -473,17 +467,10 @@ smi_print_element(struct ber_element *ro
> char*str = NULL, *buf, *p;
> size_t len, i;
> long long v;
> - int d;
> struct ber_oid o;
> char strbuf[BUFSIZ];
>
> switch (root->be_encoding) {
> - case BER_TYPE_BOOLEAN:
> - if (ober_get_boolean(root, &d) == -1)
> - goto fail;
> - if (asprintf(&str, "%s(%d)", d ? "true" : "false", d) == -1)
> - goto fail;
> - break;
> case BER_TYPE_INTEGER:
> case BER_TYPE_ENUMERATED:
> if (ober_get_integer(root, &v) == -1)
>
>
Re: pipex(4): remove unused `pipex_iface_context' struct
OK kn
Re: pppoe: input without kernel lock
On Wed, Dec 30, 2020 at 01:10:33AM +0300, Vitaliy Makkoveev wrote:
> For me these if_{get,put}(9) dances are useless. This `ph_ifidx’ is
> related to ethernet device and used to find pppoe(4) related software
> context. pppoe_send_padt() will get this `ifp’ as `eth_if’ by itself
> and correctly handle the case where this ethernet interface was gone.
Indeed, I concur after going through the code once again.
> Also `if_list’ modifications are simultaneously protected by kernel
> lock and netlock.
Right, but if_var.h only says
TAILQ_ENTRY(ifnet) if_list; /* [K] all struct ifnets are chained */
so that's what I worked with.
I'm confident now that pppoe(4)'s input can run without the kernel lock
as is.
OK?
Index: if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.621
diff -u -p -r1.621 if.c
--- if.c15 Dec 2020 03:43:34 - 1.621
+++ if.c28 Dec 2020 18:13:02 -
@@ -907,9 +907,7 @@ if_netisr(void *unused)
#endif
#if NPPPOE > 0
if (n & (1 << NETISR_PPPOE)) {
- KERNEL_LOCK();
pppoeintr();
- KERNEL_UNLOCK();
}
#endif
t |= n;
Re: pppoe: input without kernel lock
ok mvs@
> On 4 Jan 2021, at 00:23, Klemens Nanni wrote:
>
> On Wed, Dec 30, 2020 at 01:10:33AM +0300, Vitaliy Makkoveev wrote:
>> For me these if_{get,put}(9) dances are useless. This `ph_ifidx’ is
>> related to ethernet device and used to find pppoe(4) related software
>> context. pppoe_send_padt() will get this `ifp’ as `eth_if’ by itself
>> and correctly handle the case where this ethernet interface was gone.
> Indeed, I concur after going through the code once again.
>
>> Also `if_list’ modifications are simultaneously protected by kernel
>> lock and netlock.
> Right, but if_var.h only says
>TAILQ_ENTRY(ifnet) if_list; /* [K] all struct ifnets are chained */
> so that's what I worked with.
>
> I'm confident now that pppoe(4)'s input can run without the kernel lock
> as is.
>
> OK?
>
> Index: if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.621
> diff -u -p -r1.621 if.c
> --- if.c 15 Dec 2020 03:43:34 - 1.621
> +++ if.c 28 Dec 2020 18:13:02 -
> @@ -907,9 +907,7 @@ if_netisr(void *unused)
> #endif
> #if NPPPOE > 0
> if (n & (1 << NETISR_PPPOE)) {
> - KERNEL_LOCK();
> pppoeintr();
> - KERNEL_UNLOCK();
> }
> #endif
> t |= n;
Re: acme-client(1): backup certs
On Sun, Jan 03, 2021 at 11:16:00AM +, Stuart Henderson wrote: > What are you thinking would be stolen? The certificates themselves > are public knowledge anyway - they are sent in full whenever someone > connects to your TLS-based service and are available from Certificate > Transparency log servers (https://crt.sh etc) - but they are useless > without the private key. That's exactly what concerns me. I rent servers. Physical access always breaks security if someone really wants to. If it wasn't so insane in the Big Tech companies right now, I would only place my paranoia with some bad guy in the server room. But I have two sites that just have copies of the US and Texas Declarations of Independence, The US Constitution, Hammarabi's legal code and just stuff like that. Nothing with any opinions. I also walk past small shops permanently out of business every day, so I find it tough not to be a little paranoid. I do keep all my sites with DNSSEC. Except this one. As I tried to move it, I found all kinds of restrictions on sites with endings like .us IMO, really stupid, but oh well. Going to try to move it again next couple of days. I really don't maintain bennettconstruction.us, it's just sentimental value for me and what was. Chris > > > Especially since DNS servers can take up to 48 hours to propagate changes > > So getting rid of www.domain.xxx might not show up quickly enough. > > And if I change IP addresses and they don't get propagated soon enough, > > wouldn't someone be able to briefly spoof my site? > > letsencrypt (and I think probably all CAs) do uncached lookups from the > authoritative servers for the domain, following the chain from the root > servers, the usual problem with DNS servers returning outdated records > is with bad recursive servers. > > If you have problems getting the authoritative servers giving out current > information then that needs fixing, and isn't really a problem specific > to CA validation. >
Re: pf route-to issues
On Sun, Jan 03, 2021 at 06:56:20PM +0100, Alexander Bluhm wrote: > I am currently running a full regress to find more fallout. These regress tests fail: sys/net/pf_forward sys/net/pf_fragment sbin/pfctl The first two are easy to fix. That means my tests using route-to work fine with your diff. Just remove the @interface as below. pfctl tests pfail8 and pf13 use very strange routespec syntax. You might want to take a look at what that meant before and what should be valid now. bluhm Index: regress/sys/net/pf_forward/pf.conf === RCS file: /mount/openbsd/cvs/src/regress/sys/net/pf_forward/pf.conf,v retrieving revision 1.5 diff -u -p -r1.5 pf.conf --- regress/sys/net/pf_forward/pf.conf 11 Jan 2018 03:23:16 - 1.5 +++ regress/sys/net/pf_forward/pf.conf 3 Jan 2021 23:26:54 - @@ -17,22 +17,22 @@ pass out inet6 pass in to $AF_IN6/64 af-to inet from $PF_OUT to $ECO_IN/24 tag af pass out inettagged af -pass in to $RTT_IN/24 route-to $RT_IN@$PF_IFOUT tag rttin -pass out tagged rttin -pass in to $RTT_IN6/64 route-to $RT_IN6@$PF_IFOUT tag rttin -pass out tagged rttin +pass in to $RTT_IN/24 route-to $RT_IN tag rttin +pass out tagged rttin +pass in to $RTT_IN6/64 route-to $RT_IN6 tag rttin +pass out tagged rttin -pass in to $RTT_OUT/24 tag rttout -pass out route-to $RT_IN@$PF_IFOUT tagged rttout -pass in to $RTT_OUT6/64tag rttout -pass out route-to $RT_IN6@$PF_IFOUT tagged rttout +pass in to $RTT_OUT/24 tag rttout +pass out route-to $RT_IN tagged rttout +pass in to $RTT_OUT6/64 tag rttout +pass out route-to $RT_IN6 tagged rttout -pass in from $RPT_IN/24 reply-to $SRC_OUT@$PF_IFIN tag rptin -pass out tagged rptin -pass in from $RPT_IN6/64 reply-to $SRC_OUT6@$PF_IFIN tag rptin -pass out tagged rptin +pass in from $RPT_IN/24 reply-to $SRC_OUT tag rptin +pass out tagged rptin +pass in from $RPT_IN6/64 reply-to $SRC_OUT6 tag rptin +pass out tagged rptin -pass in from $RPT_OUT/24 tag rptout -pass out reply-to $SRC_OUT@$PF_IFIN tagged rptout -pass in from $RPT_OUT6/64 tag rptout -pass out reply-to $SRC_OUT6@$PF_IFIN tagged rptout +pass in from $RPT_OUT/24 tag rptout +pass out reply-to $SRC_OUT tagged rptout +pass in from $RPT_OUT6/64tag rptout +pass out reply-to $SRC_OUT6 tagged rptout Index: regress/sys/net/pf_fragment/pf.conf === RCS file: /mount/openbsd/cvs/src/regress/sys/net/pf_fragment/pf.conf,v retrieving revision 1.5 diff -u -p -r1.5 pf.conf --- regress/sys/net/pf_fragment/pf.conf 7 Jun 2017 20:09:07 - 1.5 +++ regress/sys/net/pf_fragment/pf.conf 3 Jan 2021 23:28:07 - @@ -10,7 +10,7 @@ pass outnat-to $PF_OUT pass in to $RDR_IN6/64 rdr-to $ECO_IN6 allow-opts tag rdr pass outnat-to $PF_OUT6 allow-opts tagged rdr -pass in to $RTT_IN/24 allow-opts tag rtt -pass outroute-to $RT_IN@$PF_IFOUT allow-opts tagged rtt -pass in to $RTT_IN6/64allow-opts tag rtt -pass outroute-to $RT_IN6@$PF_IFOUT allow-opts tagged rtt +pass in to $RTT_IN/24 allow-opts tag rtt +pass outroute-to $RT_IN allow-opts tagged rtt +pass in to $RTT_IN6/64 allow-opts tag rtt +pass outroute-to $RT_IN6 allow-opts tagged rtt
convert i386 fix_f00f() uvm_km_zalloc
I don't have a real 586, but I can tell qemu to pretend to be one, which at least executes this code. Using kd_waitok here seems suspect, because if we're out of memory this early I can't see anything else freeing any up, but uvm_km_zalloc() will also sleep rather than return failure. Should this use kd_nowait and panic if the allocation fails instead? ok? Index: arch/i386/i386/machdep.c === RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v retrieving revision 1.642 diff -u -p -u -p -r1.642 machdep.c --- arch/i386/i386/machdep.c28 Dec 2020 14:02:07 - 1.642 +++ arch/i386/i386/machdep.c3 Jan 2021 23:01:34 - @@ -3100,7 +3100,7 @@ fix_f00f(void) void *p; /* Allocate two new pages */ - va = uvm_km_zalloc(kernel_map, NBPG*2); + va = (vaddr_t)km_alloc(NBPG*2, &kv_any, &kp_zero, &kd_waitok); p = (void *)(va + NBPG - 7*sizeof(*idt)); /* Copy over old IDT */
Re: convert i386 fix_f00f() uvm_km_zalloc
On Mon, Jan 04, 2021 at 10:00:25AM +1000, Jonathan Matthew wrote: > I don't have a real 586, but I can tell qemu to pretend to be one, > which at least executes this code. You can run regress/sys/arch/i386/f00f/ . > Using kd_waitok here seems suspect, because if we're out of memory > this early I can't see anything else freeing any up, but > uvm_km_zalloc() will also sleep rather than return failure. > Should this use kd_nowait and panic if the allocation fails instead? Calling malloc(9) with M_WAITOK during boot is the correct way. It will always succeed or panic in malloc() if it tries to sleep during boot. Although km_alloc() does not have this check, I would also call it with kd_waitok. I don't think we will trigger sleeping during boot. But if there is concern, better put a similar check into km_alloc() instead of checks in every caller. > ok? OK bluhm@ > Index: arch/i386/i386/machdep.c > === > RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v > retrieving revision 1.642 > diff -u -p -u -p -r1.642 machdep.c > --- arch/i386/i386/machdep.c 28 Dec 2020 14:02:07 - 1.642 > +++ arch/i386/i386/machdep.c 3 Jan 2021 23:01:34 - > @@ -3100,7 +3100,7 @@ fix_f00f(void) > void *p; > > /* Allocate two new pages */ > - va = uvm_km_zalloc(kernel_map, NBPG*2); > + va = (vaddr_t)km_alloc(NBPG*2, &kv_any, &kp_zero, &kd_waitok); > p = (void *)(va + NBPG - 7*sizeof(*idt)); > > /* Copy over old IDT */
Re: pppoe: input without kernel lock
On Tue, Dec 29, 2020 at 11:18:26PM +0100, Claudio Jeker wrote:
> Generally I would prefer to go for direct dispatch and not use netisr.
> This removes a queue and a scheduling point and should help reduce the
> latency in processing pppoe packages.
>
> This does not mean that I'm against this change. I just think it may be
> benefitial to move one step further.
Here's a diff that removes the kernel lock and calls input routines
directly instead of (de)queuing through netisr.
Previously, if_netisr() handled the net lock around those calls, now
if_input_process() does it before calling ether_input(), so no need to
add or remove NET_*LOCK() anywhere.
I'm running this on my home router without any regression so far.
Feedback? Objections? OK?
NB: I want to commit this separately modulo the previous diff.
Index: if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.621
diff -u -p -r1.621 if.c
--- if.c15 Dec 2020 03:43:34 - 1.621
+++ if.c3 Jan 2021 21:38:57 -
@@ -68,7 +68,6 @@
#include "pf.h"
#include "pfsync.h"
#include "ppp.h"
-#include "pppoe.h"
#include "switch.h"
#include "if_wg.h"
@@ -902,13 +901,6 @@ if_netisr(void *unused)
if (n & (1 << NETISR_SWITCH)) {
KERNEL_LOCK();
switchintr();
- KERNEL_UNLOCK();
- }
-#endif
-#if NPPPOE > 0
- if (n & (1 << NETISR_PPPOE)) {
- KERNEL_LOCK();
- pppoeintr();
KERNEL_UNLOCK();
}
#endif
Index: if_ethersubr.c
===
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.267
diff -u -p -r1.267 if_ethersubr.c
--- if_ethersubr.c 1 Oct 2020 05:14:10 - 1.267
+++ if_ethersubr.c 3 Jan 2021 21:41:17 -
@@ -532,9 +532,9 @@ ether_input(struct ifnet *ifp, struct mb
}
#endif
if (etype == ETHERTYPE_PPPOEDISC)
- niq_enqueue(&pppoediscinq, m);
+ pppoe_disc_input(m);
else
- niq_enqueue(&pppoeinq, m);
+ pppoe_data_input(m);
return;
#endif
#ifdef MPLS
Index: if_pppoe.c
===
RCS file: /cvs/src/sys/net/if_pppoe.c,v
retrieving revision 1.75
diff -u -p -r1.75 if_pppoe.c
--- if_pppoe.c 30 Dec 2020 13:18:07 - 1.75
+++ if_pppoe.c 4 Jan 2021 00:14:30 -
@@ -143,14 +143,8 @@ struct pppoe_softc {
struct timeval sc_session_time; /* [N] time the session was established
*/
};
-/* incoming traffic will be queued here */
-struct niqueue pppoediscinq = NIQUEUE_INITIALIZER(IFQ_MAXLEN, NETISR_PPPOE);
-struct niqueue pppoeinq = NIQUEUE_INITIALIZER(IFQ_MAXLEN, NETISR_PPPOE);
-
/* input routines */
-static void pppoe_disc_input(struct mbuf *);
static void pppoe_dispatch_disc_pkt(struct mbuf *);
-static void pppoe_data_input(struct mbuf *);
/* management routines */
void pppoeattach(int);
@@ -341,21 +335,6 @@ pppoe_find_softc_by_hunique(u_int8_t *to
return (sc);
}
-/* Interface interrupt handler routine. */
-void
-pppoeintr(void)
-{
- struct mbuf *m;
-
- NET_ASSERT_LOCKED();
-
- while ((m = niq_dequeue(&pppoediscinq)) != NULL)
- pppoe_disc_input(m);
-
- while ((m = niq_dequeue(&pppoeinq)) != NULL)
- pppoe_data_input(m);
-}
-
/* Analyze and handle a single received packet while not in session state. */
static void
pppoe_dispatch_disc_pkt(struct mbuf *m)
@@ -649,7 +628,7 @@ done:
}
/* Input function for discovery packets. */
-static void
+void
pppoe_disc_input(struct mbuf *m)
{
/* avoid error messages if there is not a single pppoe instance */
@@ -661,7 +640,7 @@ pppoe_disc_input(struct mbuf *m)
}
/* Input function for data packets */
-static void
+void
pppoe_data_input(struct mbuf *m)
{
struct pppoe_softc *sc;
Index: if_pppoe.h
===
RCS file: /cvs/src/sys/net/if_pppoe.h,v
retrieving revision 1.6
diff -u -p -r1.6 if_pppoe.h
--- if_pppoe.h 10 Apr 2015 13:58:20 - 1.6
+++ if_pppoe.h 3 Jan 2021 21:46:11 -
@@ -66,10 +66,8 @@ struct pppoeconnectionstate {
#ifdef _KERNEL
-extern struct niqueue pppoediscinq;
-extern struct niqueue pppoeinq;
-
-void pppoeintr(void);
+void pppoe_disc_input(struct mbuf *);
+void pppoe_data_input(struct mbuf *);
#endif /* _KERNEL */
#endif /* _NET_IF_PPPOE_H_ */
Index: netisr.h
===
RCS file: /cvs/src/sys/net/netisr.h,v
retrieving revision 1.53
diff -u -p -r1.53 netisr.h
--- netisr.h6 Aug 2020 12:00:46 - 1.53
+++ netisr.h3 Jan 2021 21:38:37 -
@@ -45,7 +45,6 @@
#defineNETISR_A
Re: pf route-to issues
On Sun, Jan 03, 2021 at 06:56:20PM +0100, Alexander Bluhm wrote:
> On Sun, Jan 03, 2021 at 02:00:00PM +1000, David Gwynne wrote:
> > On Tue, Oct 20, 2020 at 09:27:09AM +1000, David Gwynne wrote:
> > We've been running this diff in production for the last couple of
> > months, and it's been solid for us so far. Ignoring the fixes for
> > crashes, I personally find it a lot more usable than the current
> > route-to rules too.
> >
> > Can I commit it?
>
> The diff is quite large and does multiple things at a time.
In hindsight I agree. It was hard to see while being so close to it.
> In general I also did not understand why I have to say em0@10.0.0.1
> for routing and it took me a while to figure out what to put into
> pf.conf. I use this syntax in /usr/src/regress/sys/net/pf_forward/pf.conf.
> This has to be fixed after this goes in. I will care about regress
> as this test is quite complex an needs several machines to setup.
> I am currently running a full regress to find more fallout.
>
> I do not use pfsync, so I cannot say what the consequences of the
> change are in this area. Also I don't know why pf-route interfaces
> were designed in such a strange way.
we do use pfsync, and not being able to use it with route-to has been a
point of friction for us for years.
as for the design, i think it was copied (imperfectly) from ipfilter.
look for "Policy Based Routing" in
https://www.freebsd.org/cgi/man.cgi?query=ipf&sektion=5.
> From a user perspective it is not clear, why route-to should not
> work together with no-state. So we should either fix it or document
> it and add a check in the parser. Is fixing hard?
pf_route only takes a state now, so i'd say it is non-trivial. for now
i'll go with documentation and a check in the parser..
> Are we losing any other features apart from this strange arp reuse
> you described in your mail?
i wouldn't say the arp reuse is a feature.
> There is some refactoring in your diff. I splitted it to make
> review easier. I think this should go in first. Note that the
> pf_state variable is called st in if_pfsync.c. Can we be consistent
> here? Is the pfsync_state properly aligned? During import it comes
> from an mbuf.
the stack should provide it on a 4 byte boundary, but it has uint64_t
members. however, it is also __packed, so the compiler makes no
assumptions about alignment.
> Is there anything else that can be split out easily?
let's put this in and then i'll have a look. ok by me.
>
> bluhm
>
> Index: net/if_pfsync.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_pfsync.c,v
> retrieving revision 1.279
> diff -u -p -r1.279 if_pfsync.c
> --- net/if_pfsync.c 12 Dec 2020 11:49:02 - 1.279
> +++ net/if_pfsync.c 3 Jan 2021 17:16:55 -
> @@ -612,7 +612,7 @@ pfsync_state_import(struct pfsync_state
> st->rtableid[PF_SK_STACK] = ntohl(sp->rtableid[PF_SK_STACK]);
>
> /* copy to state */
> - bcopy(&sp->rt_addr, &st->rt_addr, sizeof(st->rt_addr));
> + st->rt_addr = sp->rt_addr;
> st->creation = getuptime() - ntohl(sp->creation);
> st->expire = getuptime();
> if (ntohl(sp->expire)) {
> @@ -1843,6 +1843,7 @@ pfsync_undefer(struct pfsync_deferral *p
> {
> struct pfsync_softc *sc = pfsyncif;
> struct pf_pdesc pdesc;
> + struct pf_state *st = pd->pd_st;
>
> NET_ASSERT_LOCKED();
>
> @@ -1852,35 +1853,32 @@ pfsync_undefer(struct pfsync_deferral *p
> TAILQ_REMOVE(&sc->sc_deferrals, pd, pd_entry);
> sc->sc_deferred--;
>
> - CLR(pd->pd_st->state_flags, PFSTATE_ACK);
> + CLR(st->state_flags, PFSTATE_ACK);
> if (drop)
> m_freem(pd->pd_m);
> else {
> - if (pd->pd_st->rule.ptr->rt == PF_ROUTETO) {
> - if (pf_setup_pdesc(&pdesc,
> - pd->pd_st->key[PF_SK_WIRE]->af,
> - pd->pd_st->direction, pd->pd_st->rt_kif,
> - pd->pd_m, NULL) != PF_PASS) {
> + if (st->rule.ptr->rt == PF_ROUTETO) {
> + if (pf_setup_pdesc(&pdesc, st->key[PF_SK_WIRE]->af,
> + st->direction, st->kif, pd->pd_m, NULL) !=
> + PF_PASS) {
> m_freem(pd->pd_m);
> goto out;
> }
> - switch (pd->pd_st->key[PF_SK_WIRE]->af) {
> + switch (st->key[PF_SK_WIRE]->af) {
> case AF_INET:
> - pf_route(&pdesc,
> - pd->pd_st->rule.ptr, pd->pd_st);
> + pf_route(&pdesc, st->rule.ptr, st);
> break;
> #ifdef INET6
> case AF_INET6:
> - pf_route6(&pdesc,
> - pd->pd_st->rule.ptr, pd->pd_st);
> +
Re: pf route-to issues
On Mon, Jan 04, 2021 at 12:58:17AM +0100, Alexander Bluhm wrote: > On Sun, Jan 03, 2021 at 06:56:20PM +0100, Alexander Bluhm wrote: > > I am currently running a full regress to find more fallout. > > These regress tests fail: > > sys/net/pf_forward > sys/net/pf_fragment > sbin/pfctl > > The first two are easy to fix. That means my tests using route-to > work fine with your diff. Just remove the @interface as below. pretty much, yes. > pfctl tests pfail8 and pf13 use very strange routespec syntax. You > might want to take a look at what that meant before and what should > be valid now. this is another syntax which we seem to agree is confusing. this makes me more convinced that it needs to be changed. pfail8.in and pf13.in should be modified to route-to an IP address instead of an interface. these regress tests are a bit confusing because they're just testing the parser and the addresses that they're using aren't configured anywhere. pfail8.ok shows that pfctl should generate some more specific error messages, which is easily fixed. > > bluhm > > Index: regress/sys/net/pf_forward/pf.conf > === > RCS file: /mount/openbsd/cvs/src/regress/sys/net/pf_forward/pf.conf,v > retrieving revision 1.5 > diff -u -p -r1.5 pf.conf > --- regress/sys/net/pf_forward/pf.conf11 Jan 2018 03:23:16 - > 1.5 > +++ regress/sys/net/pf_forward/pf.conf3 Jan 2021 23:26:54 - > @@ -17,22 +17,22 @@ pass out inet6 > pass in to $AF_IN6/64 af-to inet from $PF_OUT to $ECO_IN/24 tag af > pass out inettagged af > > -pass in to $RTT_IN/24 route-to $RT_IN@$PF_IFOUT tag rttin > -pass out tagged rttin > -pass in to $RTT_IN6/64 route-to $RT_IN6@$PF_IFOUT tag rttin > -pass out tagged rttin > +pass in to $RTT_IN/24 route-to $RT_IN tag rttin > +pass out tagged rttin > +pass in to $RTT_IN6/64 route-to $RT_IN6 tag rttin > +pass out tagged rttin > > -pass in to $RTT_OUT/24 tag rttout > -pass out route-to $RT_IN@$PF_IFOUT tagged rttout > -pass in to $RTT_OUT6/64tag rttout > -pass out route-to $RT_IN6@$PF_IFOUT tagged rttout > +pass in to $RTT_OUT/24 tag rttout > +pass out route-to $RT_IN tagged rttout > +pass in to $RTT_OUT6/64 tag rttout > +pass out route-to $RT_IN6 tagged rttout > > -pass in from $RPT_IN/24 reply-to $SRC_OUT@$PF_IFIN tag rptin > -pass out tagged rptin > -pass in from $RPT_IN6/64 reply-to $SRC_OUT6@$PF_IFIN tag rptin > -pass out tagged rptin > +pass in from $RPT_IN/24 reply-to $SRC_OUT tag rptin > +pass out tagged rptin > +pass in from $RPT_IN6/64 reply-to $SRC_OUT6 tag rptin > +pass out tagged rptin > > -pass in from $RPT_OUT/24 tag rptout > -pass out reply-to $SRC_OUT@$PF_IFIN tagged rptout > -pass in from $RPT_OUT6/64 tag rptout > -pass out reply-to $SRC_OUT6@$PF_IFIN tagged rptout > +pass in from $RPT_OUT/24 tag rptout > +pass out reply-to $SRC_OUT tagged rptout > +pass in from $RPT_OUT6/64tag rptout > +pass out reply-to $SRC_OUT6 tagged rptout > Index: regress/sys/net/pf_fragment/pf.conf > === > RCS file: /mount/openbsd/cvs/src/regress/sys/net/pf_fragment/pf.conf,v > retrieving revision 1.5 > diff -u -p -r1.5 pf.conf > --- regress/sys/net/pf_fragment/pf.conf 7 Jun 2017 20:09:07 - > 1.5 > +++ regress/sys/net/pf_fragment/pf.conf 3 Jan 2021 23:28:07 - > @@ -10,7 +10,7 @@ pass outnat-to $PF_OUT > pass in to $RDR_IN6/64 rdr-to $ECO_IN6 allow-opts tag rdr > pass outnat-to $PF_OUT6 allow-opts tagged rdr > > -pass in to $RTT_IN/24 allow-opts tag rtt > -pass outroute-to $RT_IN@$PF_IFOUT allow-opts tagged rtt > -pass in to $RTT_IN6/64allow-opts tag rtt > -pass outroute-to $RT_IN6@$PF_IFOUT allow-opts tagged rtt > +pass in to $RTT_IN/24 allow-opts tag rtt > +pass outroute-to $RT_IN allow-opts tagged rtt > +pass in to $RTT_IN6/64 allow-opts tag rtt > +pass outroute-to $RT_IN6 allow-opts tagged rtt >
