Re: [External] : Re: move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()
Hello,
On Tue, May 10, 2022 at 12:18:15AM +0200, Alexander Bluhm wrote:
> On Mon, May 09, 2022 at 11:11:03PM +0200, Alexandr Nedvedicky wrote:
> > > ... and then we insert a destroyed p
> >
> > yes. you are right. new diff addresses that with change as follows:
> >
> > @@ -1542,9 +1542,8 @@ pfr_add_tables(struct pfr_table ...)
> > pfr_destroy_ktable(p, 0);
> > break;
> > }
> > + SLIST_INSERT_HEAD(, p, pfrkt_workq);
> This inserts p each time you run over q list.
>
> Should we do it like this? It is similar to your solution at the
> other loop.
>
> SLIST_FOREACH(q, , pfrkt_workq) {
> if (!pfr_ktable_compare(p, q)) {
> /*
> * We need no lock here, because `p` is empty,
> * there are no rules or shadow tables
> * attached.
> */
> pfr_destroy_ktable(p->pfrkt_root, 0);
> p->pfrkt_root = NULL;
> pfr_destroy_ktable(p, 0);
> break;
> }
> }
> if (q != NULL)
> continue;
> SLIST_INSERT_HEAD(, p, pfrkt_workq);
yes, this is exactly code I want.
>
>
> > > I compared the old and new code to see if it is equivalent.
> > > Before the condtion looked like this.
> >
> > very good point. I think this what needs to be done:
> >
> > @@ -1558,7 +1558,8 @@ pfr_add_tables(struct pfr_table *tbl, ...)
> > if (p == NULL) {
> > SLIST_REMOVE(, n, pfr_ktable, pfrkt_workq);
> > SLIST_INSERT_HEAD(, n, pfrkt_workq);
> > - } else if (!(flags & PFR_FLAG_DUMMY)) {
> > + } else if (!(flags & PFR_FLAG_DUMMY) &&
> I guess PFR_FLAG_DUMMY check is an optimization.
not really. I don't want to alter any flags on existing tables,
when running 'pfctl -n ...'. the '-n' sets PFR_FLAG_DUMMY.
> > + !(p->pfrkt_flags & PFR_TFLAG_ACTIVE)) {
> Indent should be one tab to the left.
> > p->pfrkt_nflags = (p->pfrkt_flags &
> > ~PFR_TFLAG_USRMASK) | key.pfrkt_flags;
> > SLIST_INSERT_HEAD(, p, pfrkt_workq);
>
> Old code had this to avoid duplicate entries. Do we need it?
> SLIST_FOREACH(q, , pfrkt_workq)
> if (!pfr_ktable_compare(, q))
> goto _skip;
the duplicate entries are removed in the first loop, which copies
data in. So we don't need this check here.
>
> > > This continue goes to the r list, but I think you want to continue p list.
> > > > }
> > > > }
> >
> > yes, exactly. we want to continue with outer loop if we break from inner
> > one. this is what I want to do:
> >
> > @@ -1617,9 +1618,12 @@ pfr_add_tables(struct pfr_table *tbl, ...)
> > p->pfrkt_root = r;
> > SLIST_INSERT_HEAD(, q,
> > pfrkt_workq);
> > - continue;
> > + break;
> > }
> > }
> > + if (r != SLIST_END())
> Could you use if (r != NULL) ? Noone uses SLIST_END macros.
> > + continue;
> > +
sure.
updated diff is below.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 8315b115474..1f036e1368f 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -2217,12 +2217,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
error = ENODEV;
goto fail;
}
- NET_LOCK();
- PF_LOCK();
error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
>pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
- NET_UNLOCK();
break;
}
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index fb23bcabe04..d0e42ca62ba 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -189,6 +189,7 @@ void pfr_clstats_ktable(struct
pfr_ktable *, time_t, int);
struct pfr_ktable *pfr_create_ktable(struct pfr_table *, time_t, int,
int);
void
Re: [External] : Re: move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()
On Mon, May 09, 2022 at 11:11:03PM +0200, Alexandr Nedvedicky wrote:
> > ... and then we insert a destroyed p
>
> yes. you are right. new diff addresses that with change as follows:
>
> @@ -1542,9 +1542,8 @@ pfr_add_tables(struct pfr_table ...)
> pfr_destroy_ktable(p, 0);
> break;
> }
> + SLIST_INSERT_HEAD(, p, pfrkt_workq);
This inserts p each time you run over q list.
> }
> -
> - SLIST_INSERT_HEAD(, p, pfrkt_workq);
> }
Should we do it like this? It is similar to your solution at the
other loop.
SLIST_FOREACH(q, , pfrkt_workq) {
if (!pfr_ktable_compare(p, q)) {
/*
* We need no lock here, because `p` is empty,
* there are no rules or shadow tables
* attached.
*/
pfr_destroy_ktable(p->pfrkt_root, 0);
p->pfrkt_root = NULL;
pfr_destroy_ktable(p, 0);
break;
}
}
if (q != NULL)
continue;
SLIST_INSERT_HEAD(, p, pfrkt_workq);
> > I compared the old and new code to see if it is equivalent.
> > Before the condtion looked like this.
>
> very good point. I think this what needs to be done:
>
> @@ -1558,7 +1558,8 @@ pfr_add_tables(struct pfr_table *tbl, ...)
> if (p == NULL) {
> SLIST_REMOVE(, n, pfr_ktable, pfrkt_workq);
> SLIST_INSERT_HEAD(, n, pfrkt_workq);
> - } else if (!(flags & PFR_FLAG_DUMMY)) {
> + } else if (!(flags & PFR_FLAG_DUMMY) &&
I guess PFR_FLAG_DUMMY check is an optimization.
> + !(p->pfrkt_flags & PFR_TFLAG_ACTIVE)) {
Indent should be one tab to the left.
> p->pfrkt_nflags = (p->pfrkt_flags &
> ~PFR_TFLAG_USRMASK) | key.pfrkt_flags;
> SLIST_INSERT_HEAD(, p, pfrkt_workq);
Old code had this to avoid duplicate entries. Do we need it?
SLIST_FOREACH(q, , pfrkt_workq)
if (!pfr_ktable_compare(, q))
goto _skip;
> > This continue goes to the r list, but I think you want to continue p list.
> > > }
> > > }
>
> yes, exactly. we want to continue with outer loop if we break from inner
> one. this is what I want to do:
>
> @@ -1617,9 +1618,12 @@ pfr_add_tables(struct pfr_table *tbl, ...)
> p->pfrkt_root = r;
> SLIST_INSERT_HEAD(, q,
> pfrkt_workq);
> - continue;
> + break;
> }
> }
> + if (r != SLIST_END())
Could you use if (r != NULL) ? Noone uses SLIST_END macros.
> + continue;
> +
bluhm
Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling
On 2022/05/09 23:16, Alexandr Nedvedicky wrote: > Hello, > > I'm sorry I was too fast with commit. I've just committed > what's been suggested by bluhm@: That's totally ok, my diff is on top and wasn't written until you committed yours :-) > @@ -2186,6 +2186,7 @@ It cannot be used with > .Cm modulate state > or > .Cm synproxy state . > +With this option ICMP replies can create states. > .It Ar timeout seconds > Changes the > .Ar timeout > > > > This is helpful, but because it's so surprising that "pass proto icmp" > > doesn't pass all icmp traffic, I think it would help to mention it where > > "proto icmp" is described too. > > > > Also, the top of the text about "sloppy" just talks about the sloppy > > TCP connection tracker, I think perhaps it would be better to lead > > with something that suggests it has multiple functions for different > > protocols? > > I don't object to any of your enhancements. > > reads OK sashan Thanks.
Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling
On Mon, May 09, 2022 at 10:08:24PM +0100, Stuart Henderson wrote: > This is helpful, but because it's so surprising that "pass proto icmp" > doesn't pass all icmp traffic, I think it would help to mention it where > "proto icmp" is described too. > > Also, the top of the text about "sloppy" just talks about the sloppy > TCP connection tracker, I think perhaps it would be better to lead > with something that suggests it has multiple functions for different > protocols? OK bluhm@ > Index: man5/pf.conf.5 > === > RCS file: /cvs/src/share/man/man5/pf.conf.5,v > retrieving revision 1.594 > diff -u -p -r1.594 pf.conf.5 > --- man5/pf.conf.59 May 2022 20:29:23 - 1.594 > +++ man5/pf.conf.59 May 2022 21:05:48 - > @@ -594,6 +594,13 @@ or > .Pc > must match. > .Pp > +ICMP responses are not permitted unless they either match an > +existing request, or unless > +.Cm no state > +or > +.Cm keep state (sloppy) > +is specified. > +.Pp > .It Cm label Ar string > Adds a label to the rule, which can be used to identify the rule. > For instance, > @@ -2177,7 +2184,7 @@ States created by this rule are exported > .Xr pflow 4 > interface. > .It Cm sloppy > -Uses a sloppy TCP connection tracker that does not check sequence > +For TCP, uses a sloppy connection tracker that does not check sequence > numbers at all, which makes insertion and ICMP teardown attacks way > easier. > This is intended to be used in situations where one does not see all > @@ -2186,7 +2193,8 @@ It cannot be used with > .Cm modulate state > or > .Cm synproxy state . > -With this option ICMP replies can create states. > +For ICMP, this option allows states to be created from replies, > +not just requests. > .It Ar timeout seconds > Changes the > .Ar timeout
Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling
Hello, I'm sorry I was too fast with commit. I've just committed what's been suggested by bluhm@: @@ -2186,6 +2186,7 @@ It cannot be used with .Cm modulate state or .Cm synproxy state . +With this option ICMP replies can create states. .It Ar timeout seconds Changes the .Ar timeout > This is helpful, but because it's so surprising that "pass proto icmp" > doesn't pass all icmp traffic, I think it would help to mention it where > "proto icmp" is described too. > > Also, the top of the text about "sloppy" just talks about the sloppy > TCP connection tracker, I think perhaps it would be better to lead > with something that suggests it has multiple functions for different > protocols? I don't object to any of your enhancements. reads OK sashan > > Index: man5/pf.conf.5 > === > RCS file: /cvs/src/share/man/man5/pf.conf.5,v > retrieving revision 1.594 > diff -u -p -r1.594 pf.conf.5 > --- man5/pf.conf.59 May 2022 20:29:23 - 1.594 > +++ man5/pf.conf.59 May 2022 21:05:48 - > @@ -594,6 +594,13 @@ or > .Pc > must match. > .Pp > +ICMP responses are not permitted unless they either match an > +existing request, or unless > +.Cm no state > +or > +.Cm keep state (sloppy) > +is specified. > +.Pp > .It Cm label Ar string > Adds a label to the rule, which can be used to identify the rule. > For instance, > @@ -2177,7 +2184,7 @@ States created by this rule are exported > .Xr pflow 4 > interface. > .It Cm sloppy > -Uses a sloppy TCP connection tracker that does not check sequence > +For TCP, uses a sloppy connection tracker that does not check sequence > numbers at all, which makes insertion and ICMP teardown attacks way > easier. > This is intended to be used in situations where one does not see all > @@ -2186,7 +2193,8 @@ It cannot be used with > .Cm modulate state > or > .Cm synproxy state . > -With this option ICMP replies can create states. > +For ICMP, this option allows states to be created from replies, > +not just requests. > .It Ar timeout seconds > Changes the > .Ar timeout >
Re: [External] : Re: move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()
Hello,
thanks for taking a look.
> > + SLIST_FOREACH(q, , pfrkt_workq) {
> > + if (!pfr_ktable_compare(p, q)) {
> > + /*
> > +* We need no lock here, because `p` is empty,
> > +* there are no rules or shadow tables
> > +* attached.
> > +*/
> > + pfr_destroy_ktable(p->pfrkt_root, 0);
> > + p->pfrkt_root = NULL;
> > + pfr_destroy_ktable(p, 0);
> > + break;
> This break...
> > + }
> > + }
> ... end here
> > +
> >
> ... and then we insert a destroyed p
yes. you are right. new diff addresses that with change as follows:
@@ -1542,9 +1542,8 @@ pfr_add_tables(struct pfr_table ...)
pfr_destroy_ktable(p, 0);
break;
}
+ SLIST_INSERT_HEAD(, p, pfrkt_workq);
}
-
- SLIST_INSERT_HEAD(, p, pfrkt_workq);
}
/*
> > + SLIST_REMOVE(, n, pfr_ktable, pfrkt_workq);
> > + SLIST_INSERT_HEAD(, n, pfrkt_workq);
> > + } else if (!(flags & PFR_FLAG_DUMMY)) {
>
> I compared the old and new code to see if it is equivalent.
> Before the condtion looked like this.
very good point. I think this what needs to be done:
@@ -1558,7 +1558,8 @@ pfr_add_tables(struct pfr_table *tbl, ...)
if (p == NULL) {
SLIST_REMOVE(, n, pfr_ktable, pfrkt_workq);
SLIST_INSERT_HEAD(, n, pfrkt_workq);
- } else if (!(flags & PFR_FLAG_DUMMY)) {
+ } else if (!(flags & PFR_FLAG_DUMMY) &&
+ !(p->pfrkt_flags & PFR_TFLAG_ACTIVE)) {
p->pfrkt_nflags = (p->pfrkt_flags &
~PFR_TFLAG_USRMASK) | key.pfrkt_flags;
SLIST_INSERT_HEAD(, p, pfrkt_workq);
> > + SLIST_FOREACH(r, , pfrkt_workq) {
> > + if (!pfr_ktable_compare(r, q)) {
> > + /*
> > +* `r` is our root table we've found
> > +* earlier, `q` can get dropped.
> > +*/
> > + p->pfrkt_root = r;
> > + SLIST_INSERT_HEAD(, q,
> > + pfrkt_workq);
> > + continue;
> This continue goes to the r list, but I think you want to continue p list.
> > }
> > }
yes, exactly. we want to continue with outer loop if we break from inner
one. this is what I want to do:
@@ -1617,9 +1618,12 @@ pfr_add_tables(struct pfr_table *tbl, ...)
p->pfrkt_root = r;
SLIST_INSERT_HEAD(, q,
pfrkt_workq);
- continue;
+ break;
}
}
+ if (r != SLIST_END())
+ continue;
+
q->pfrkt_rs = pf_find_or_create_ruleset(
q->pfrkt_root->pfrkt_anchor);
/*
updated diff is below.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index 8315b115474..1f036e1368f 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -2217,12 +2217,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int flags,
struct proc *p)
error = ENODEV;
goto fail;
}
- NET_LOCK();
- PF_LOCK();
error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
>pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
- PF_UNLOCK();
- NET_UNLOCK();
break;
}
diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
index fb23bcabe04..4dd49ea3550 100644
--- a/sys/net/pf_table.c
+++ b/sys/net/pf_table.c
@@ -189,6 +189,7 @@ void pfr_clstats_ktable(struct
pfr_ktable *, time_t, int);
struct pfr_ktable *pfr_create_ktable(struct pfr_table *, time_t, int,
int);
voidpfr_destroy_ktables(struct pfr_ktableworkq *, int);
+voidpfr_destroy_ktables_aux(struct pfr_ktableworkq *);
Re: [External] : Re: pf.conf(5) clarify ICMP sloppy state handling
This is helpful, but because it's so surprising that "pass proto icmp" doesn't pass all icmp traffic, I think it would help to mention it where "proto icmp" is described too. Also, the top of the text about "sloppy" just talks about the sloppy TCP connection tracker, I think perhaps it would be better to lead with something that suggests it has multiple functions for different protocols? Index: man5/pf.conf.5 === RCS file: /cvs/src/share/man/man5/pf.conf.5,v retrieving revision 1.594 diff -u -p -r1.594 pf.conf.5 --- man5/pf.conf.5 9 May 2022 20:29:23 - 1.594 +++ man5/pf.conf.5 9 May 2022 21:05:48 - @@ -594,6 +594,13 @@ or .Pc must match. .Pp +ICMP responses are not permitted unless they either match an +existing request, or unless +.Cm no state +or +.Cm keep state (sloppy) +is specified. +.Pp .It Cm label Ar string Adds a label to the rule, which can be used to identify the rule. For instance, @@ -2177,7 +2184,7 @@ States created by this rule are exported .Xr pflow 4 interface. .It Cm sloppy -Uses a sloppy TCP connection tracker that does not check sequence +For TCP, uses a sloppy connection tracker that does not check sequence numbers at all, which makes insertion and ICMP teardown attacks way easier. This is intended to be used in situations where one does not see all @@ -2186,7 +2193,8 @@ It cannot be used with .Cm modulate state or .Cm synproxy state . -With this option ICMP replies can create states. +For ICMP, this option allows states to be created from replies, +not just requests. .It Ar timeout seconds Changes the .Ar timeout
Re: ssh-add(1): fix NULL in fprintf
On Mon, May 09, 2022 at 10:42:29AM -0600, Theo de Raadt wrote:
> Martin Vahlensieck wrote:
>
> > if (!qflag) {
> > - fprintf(stderr, "Identity removed: %s %s (%s)\n", path,
> > - sshkey_type(key), comment);
> > + fprintf(stderr, "Identity removed: %s %s%s%s%s\n", path,
> > + sshkey_type(key), comment ? " (" : "",
> > + comment ? comment : "", comment ? ")" : "");
>
> this is probably better as something like
>
> > - fprintf(stderr, "Identity removed: %s %s (%s)\n", path,
> > - sshkey_type(key), comment ? comment : "no comment");
>
> Which has a minor ambiguity, but probably harms noone.
>
Index: ssh-add.c
===
RCS file: /cvs/src/usr.bin/ssh/ssh-add.c,v
retrieving revision 1.165
diff -u -p -r1.165 ssh-add.c
--- ssh-add.c 4 Feb 2022 02:49:17 - 1.165
+++ ssh-add.c 9 May 2022 18:36:54 -
@@ -118,7 +118,7 @@ delete_one(int agent_fd, const struct ss
}
if (!qflag) {
fprintf(stderr, "Identity removed: %s %s (%s)\n", path,
- sshkey_type(key), comment);
+ sshkey_type(key), comment ? comment : "no comment");
}
return 0;
}
@@ -392,7 +392,7 @@ add_file(int agent_fd, const char *filen
certpath, filename);
sshkey_free(cert);
goto out;
- }
+ }
/* Graft with private bits */
if ((r = sshkey_to_certified(private)) != 0) {
Re: move memory allocation in pfr_add_tables() outside of NET_LOCK()/PF_LOCK()
On Sun, May 08, 2022 at 06:15:49PM +0200, Alexandr Nedvedicky wrote:
> OK ?
3 comments inline
> 8<---8<---8<--8<
> diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
> index 8315b115474..1f036e1368f 100644
> --- a/sys/net/pf_ioctl.c
> +++ b/sys/net/pf_ioctl.c
> @@ -2217,12 +2217,8 @@ pfioctl(dev_t dev, u_long cmd, caddr_t addr, int
> flags, struct proc *p)
> error = ENODEV;
> goto fail;
> }
> - NET_LOCK();
> - PF_LOCK();
> error = pfr_add_tables(io->pfrio_buffer, io->pfrio_size,
> >pfrio_nadd, io->pfrio_flags | PFR_FLAG_USERIOCTL);
> - PF_UNLOCK();
> - NET_UNLOCK();
> break;
> }
>
> diff --git a/sys/net/pf_table.c b/sys/net/pf_table.c
> index fb23bcabe04..79fd3e0447b 100644
> --- a/sys/net/pf_table.c
> +++ b/sys/net/pf_table.c
> @@ -189,6 +189,7 @@ void pfr_clstats_ktable(struct
> pfr_ktable *, time_t, int);
> struct pfr_ktable*pfr_create_ktable(struct pfr_table *, time_t, int,
> int);
> void pfr_destroy_ktables(struct pfr_ktableworkq *, int);
> +void pfr_destroy_ktables_aux(struct pfr_ktableworkq *);
> void pfr_destroy_ktable(struct pfr_ktable *, int);
> int pfr_ktable_compare(struct pfr_ktable *,
> struct pfr_ktable *);
> @@ -1493,14 +1494,16 @@ pfr_clr_tables(struct pfr_table *filter, int *ndel,
> int flags)
> int
> pfr_add_tables(struct pfr_table *tbl, int size, int *nadd, int flags)
> {
> - struct pfr_ktableworkq addq, changeq;
> - struct pfr_ktable *p, *q, *r, key;
> + struct pfr_ktableworkq addq, changeq, auxq;
> + struct pfr_ktable *p, *q, *r, *n, *w, key;
> int i, rv, xadd = 0;
> time_t tzero = gettime();
>
> ACCEPT_FLAGS(flags, PFR_FLAG_DUMMY);
> SLIST_INIT();
> SLIST_INIT();
> + SLIST_INIT();
> + /* pre-allocate all memory outside of locks */
> for (i = 0; i < size; i++) {
> YIELD(flags & PFR_FLAG_USERIOCTL);
> if (COPYIN(tbl+i, _t, sizeof(key.pfrkt_t), flags))
> @@ -1509,65 +1512,142 @@ pfr_add_tables(struct pfr_table *tbl, int size, int
> *nadd, int flags)
> flags & PFR_FLAG_USERIOCTL))
> senderr(EINVAL);
> key.pfrkt_flags |= PFR_TFLAG_ACTIVE;
> - p = RB_FIND(pfr_ktablehead, _ktables, );
> + p = pfr_create_ktable(_t, tzero, 0,
> + !(flags & PFR_FLAG_USERIOCTL));
> + if (p == NULL)
> + senderr(ENOMEM);
> +
> + /*
> + * Note: we also pre-allocate a root table here. We keep it
> + * at ->pfrkt_root, which we must not forget about.
> + */
> + key.pfrkt_flags = 0;
> + memset(key.pfrkt_anchor, 0, sizeof(key.pfrkt_anchor));
> + p->pfrkt_root = pfr_create_ktable(_t, 0, 0,
> + !(flags & PFR_FLAG_USERIOCTL));
> + if (p->pfrkt_root == NULL) {
> + pfr_destroy_ktable(p, 0);
> + senderr(ENOMEM);
> + }
> +
> + SLIST_FOREACH(q, , pfrkt_workq) {
> + if (!pfr_ktable_compare(p, q)) {
> + /*
> + * We need no lock here, because `p` is empty,
> + * there are no rules or shadow tables
> + * attached.
> + */
> + pfr_destroy_ktable(p->pfrkt_root, 0);
> + p->pfrkt_root = NULL;
> + pfr_destroy_ktable(p, 0);
> + break;
This break...
> + }
> + }
... end here
> +
>
... and then we insert a destroyed p
> + SLIST_INSERT_HEAD(, p, pfrkt_workq);
> + }
> +
> + /*
> + * auxq contains freshly allocated tables with no dups.
> + * also note there are no rulesets attached, because
> + * the attach operation requires PF_LOCK().
> + */
> + NET_LOCK();
> + PF_LOCK();
> + SLIST_FOREACH_SAFE(n, , pfrkt_workq, w) {
> + p = RB_FIND(pfr_ktablehead, _ktables, n);
> if (p == NULL) {
> - p = pfr_create_ktable(_t, tzero, 1,
> - !(flags & PFR_FLAG_USERIOCTL));
> - if (p == NULL)
> - senderr(ENOMEM);
> - SLIST_FOREACH(q, , pfrkt_workq) {
> - if (!pfr_ktable_compare(p, q)) {
> - pfr_destroy_ktable(p, 0);
> - goto _skip;
> -
Re: ssh-add(1): fix NULL in fprintf
Martin Vahlensieck wrote:
> if (!qflag) {
> - fprintf(stderr, "Identity removed: %s %s (%s)\n", path,
> - sshkey_type(key), comment);
> + fprintf(stderr, "Identity removed: %s %s%s%s%s\n", path,
> + sshkey_type(key), comment ? " (" : "",
> + comment ? comment : "", comment ? ")" : "");
this is probably better as something like
> - fprintf(stderr, "Identity removed: %s %s (%s)\n", path,
> - sshkey_type(key), comment ? comment : "no comment");
Which has a minor ambiguity, but probably harms noone.
ssh-add(1): fix NULL in fprintf
Hi
When removing an identity from the agent using the private key file,
ssh-add first tries to find the public key file. If that fails,
it loads the public key from the private key file, but no comment
is loaded. This means comment is NULL when it is used inside
delete_one to print `Identity removed: ...'
Below is a diff which only prints the braces and the comment if it
is not NULL. Something similar is done in ssh-keygen.c line
2423-2425.
So with the following setup:
$ ssh-keygen -t ed25519 -f demo -C demo -N ''
$ mv demo.pub demo_pub
$ ssh-add demo
Identity added: demo (demo)
Before:
$ ssh-add -d demo
Identity removed: demo ED25519 ((null))
$ tail -n 1 /var/log/messages
May 9 18:15:53 demo ssh-add: vfprintf %s NULL in "Identity removed: %s %s
(%s) "
After:
$ ssh-add -d demo
Identity removed: demo ED25519
Best,
Martin
P.S.: While here remove a trailing space as well.
Index: ssh-add.c
===
RCS file: /cvs/src/usr.bin/ssh/ssh-add.c,v
retrieving revision 1.165
diff -u -p -r1.165 ssh-add.c
--- ssh-add.c 4 Feb 2022 02:49:17 - 1.165
+++ ssh-add.c 9 May 2022 16:04:14 -
@@ -117,8 +117,9 @@ delete_one(int agent_fd, const struct ss
return r;
}
if (!qflag) {
- fprintf(stderr, "Identity removed: %s %s (%s)\n", path,
- sshkey_type(key), comment);
+ fprintf(stderr, "Identity removed: %s %s%s%s%s\n", path,
+ sshkey_type(key), comment ? " (" : "",
+ comment ? comment : "", comment ? ")" : "");
}
return 0;
}
@@ -392,7 +393,7 @@ add_file(int agent_fd, const char *filen
certpath, filename);
sshkey_free(cert);
goto out;
- }
+ }
/* Graft with private bits */
if ((r = sshkey_to_certified(private)) != 0) {
Re: [External] : net lock priority
Hello, On Mon, May 09, 2022 at 04:34:00PM +0200, Alexander Bluhm wrote: > On Sun, May 08, 2022 at 10:54:01PM +0200, Alexandr Nedvedicky wrote: > > what bothers me is the situation where there are > > more than one reader. The line 350 is executed by > > the first reader which drops the lock. So the process > > woken up by wakeup(rwl) are going to find out the > > lock is still occupied by remaining readers. > > wakeup() activates all sleepers. They should check and sleep again. > Maybe a little bit of wasted resources, but I don't see a severe > problem. I came to the same conclusion. > > I did a little digging in history. In rev 1.3 of kern_rwlock.c > we had case RW_READ: need_wait = RWLOCK_WRLOCK | RWLOCK_WRWANT; > and the commet "Let writers through before obtaining read lock." > > The comment > * RW_READ RWLOCK_WRLOCK|RWLOCK_WRWANT may not be set. We increment > * with RWLOCK_READ_INCR. RWLOCK_WAIT while waiting. > is still there, just the RWLOCK_WRWANT got lost from the condition. > > So I think we should get back the original reader writer priority. > > Regarding the race in rw_do_exit() that sashan@ found I also found > a comment in rev 1.7. > > /* > * Potential MP race here. If the owner had WRWANT set we cleared > * it and a reader can sneak in before a writer. Do we care? > */ > > I do not want to change anything to that behavior now. There is > no easy fix and I did not see the problem during testing. But we > can put the comment back to clarify the situation. > > ok? this certainly works for me. OK sashan
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Mon, May 09, 2022 at 05:08:53PM +0200, Theo Buehler wrote: > On Mon, May 09, 2022 at 02:59:06PM +0200, Claudio Jeker wrote: > > On Mon, May 09, 2022 at 12:53:05PM +0200, Theo Buehler wrote: > > > Regarding the spec: > > > > > > * isn't it a bit unfortunate that the ResourceBlock contains an > > > ipAddrBlocks > > > member which isn't an IPAddrBlocks as in RFC 3779 but rather an IPList? > > > > > > The (somewhat) subtle difference is that an IPAddressFamilyItem has > > > an iPAddressOrRange where an IPAddressFamily has an IPAddressChoice. > > > I assume this was done because inheritance is excluded in this draft. > > > > Uh, yes, it would be good to be able to use RFC 3779 parser functions on > > can limit the usage of inheritance after parsing the objects. > > Indeed. > > After some refactoring we may be able to reduce the low level ASN.1 > bashing in rsc.c somewhat, but I fear the RFC 3779 parser functions are > only of limited help due to these unfortunate discrepancies. The asID > can't be deserialized directly for similar reasons. > > That's also why I think it's important that the spec is more specific on > how the ResourceBlock is encoded. There's ambiguity there (see the quoted > bit below). It should at least say that analogous rules as in RFC 3779 > apply and preferably be more precise. It would be good if you share this insight with sidr...@ietf.org :-) Kind regards, Job
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Mon, May 09, 2022 at 01:07:17PM +, Job Snijders wrote:
> On Mon, May 09, 2022 at 12:11:22PM +0200, Claudio Jeker wrote:
> > why does the draft allow for optional filenames? What the heck is the
> > digest then covering some random gunk?
>
> Yes, that is entirely possible. Some folks in the working group
> requested the filename to be optional, I abided. In inter-business
> workflows that are less filesystem centric, I imagine it is possible
> that one party challenges the other party to produce a RSC signing a
> specific (random) SHA256 hash, and then validating the RSC to see if the
> desired hash showed up.
>
> > This will make it close to impossible to actually fully validate a RSC
> > (which would include the validation of all SHA256 signatures). Because
> > of this RSC validation in rpki-client will be close to impossible
> > because how do you implement:
> >
> > To verify a set of digital objects with an RSC:
> >
> >* The message digest of each referenced digital object, using the
> > digest algorithm specified in the the digestAlgorithm field, MUST
> > be calculated and MUST match the value given in the messageDigest
> > field of the associated FileNameAndHash, for the digital object to
> > be considered as verified with the RSC.
> >
> > When there is no filename to reference an object?
>
> I think we should print the hash(es) where the filename was left out,
> and leave it up to the relying party how to proceed and what to compare
> against.
>
> > > + case CERT_IP_INHERIT:
> > > + warnx("%s: RSC ResourceBlock: illegal inherit", fn);
> > > + break;
> >
> > I'm not sure if this works the way you think it does. The switch case is
> > only triggers when valid_ip() fails but that may not be the case if
> > rsc->ips[i].type == CERT_IP_INHERIT.
> >
> > I think the proper way to check this is above the valid_ip() call to make
> > sure CERT_IP_INHERIT can't be used. The parse kind of makes sure of this
> > (it does not handle NULL ipAddrOrRange objects so it will bork on that
> > with an error).
>
> done.
>
> > Actually this function is not used anyway because the only place it was
> > called was in the parse.c code but that code is no needed.
>
> I'm not entirely sure I follow? valid_rsc() is called in
> proc_parser_rsc(), which is called in parse_entity().
It's only reachable outside of filemode:
if (!filemode)
proc_parser() -> parse_entity().
>
> > Also this code should probably verify the fileandhash list if we are
> > serious about full RSC validation. rpki-client -f currently skips some
> > other validation steps so it mostly depends on proper x509 validation.
>
> which other steps are skipped?
In filemode you only call rsc_parse(), valid_rsc() isn't called. Other
file types are similar. In particular, nothing will be checking that the
EE cert covers the same resources as the rsc file.
I haven't looked at the JSON issue. If claudio is happy with that, I'm
ok with the diff.
In detail:
> Index: usr.sbin/rpki-client/Makefile
ok
> Index: usr.sbin/rpki-client/extern.h
[...]
> @@ -450,6 +482,12 @@ void gbr_free(struct gbr *);
> struct gbr *gbr_parse(X509 **, const char *, const unsigned char *,
> size_t);
>
> +void rsc_buffer(struct ibuf *, const struct rsc *);
> +void rsc_free(struct rsc *);
> +struct rsc *rsc_parse(X509 **, const char *, const unsigned char *,
> + size_t);
> +struct rsc *rsc_read(struct ibuf *);
Drop the now unused rsc_buffer() and rsc_read() prototypes
> +
> /* crl.c */
> struct crl *crl_parse(const char *, const unsigned char *, size_t);
> struct crl *crl_get(struct crl_tree *, const struct auth *);
> @@ -470,6 +508,7 @@ intvalid_uri(const char *, size_t, co
> int valid_origin(const char *, const char *);
> int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
> struct crl *, int);
> +int valid_rsc(const char *, struct auth *, struct rsc *);
valid_rsc() is currently not used either.
>
> /* Working with CMS. */
> unsigned char*cms_parse_validate(X509 **, const char *,
> @@ -608,6 +647,7 @@ void crl_print(const struct crl *);
> void mft_print(const X509 *, const struct mft *);
> void roa_print(const X509 *, const struct roa *);
> void gbr_print(const X509 *, const struct gbr *);
> +void rsc_print(const X509 *, const struct rsc *);
>
> /* Output! */
>
> Index: usr.sbin/rpki-client/filemode.c
ok
> Index: usr.sbin/rpki-client/mft.c
ok
> Index: usr.sbin/rpki-client/parser.c
As discussed, leave this out.
> Index: usr.sbin/rpki-client/print.c
ok if claudio is happy with the changes in JSON
> Index: usr.sbin/rpki-client/rpki-client.8
ok
> Index: usr.sbin/rpki-client/rsc.c
[...]
> +void
> +rsc_free(struct rsc *p)
> +{
> + size_t i;
> +
> +
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Mon, May 09, 2022 at 02:59:06PM +0200, Claudio Jeker wrote: > On Mon, May 09, 2022 at 12:53:05PM +0200, Theo Buehler wrote: > > Regarding the spec: > > > > * isn't it a bit unfortunate that the ResourceBlock contains an ipAddrBlocks > > member which isn't an IPAddrBlocks as in RFC 3779 but rather an IPList? > > > > The (somewhat) subtle difference is that an IPAddressFamilyItem has > > an iPAddressOrRange where an IPAddressFamily has an IPAddressChoice. > > I assume this was done because inheritance is excluded in this draft. > > Uh, yes, it would be good to be able to use RFC 3779 parser functions on > can limit the usage of inheritance after parsing the objects. Indeed. After some refactoring we may be able to reduce the low level ASN.1 bashing in rsc.c somewhat, but I fear the RFC 3779 parser functions are only of limited help due to these unfortunate discrepancies. The asID can't be deserialized directly for similar reasons. That's also why I think it's important that the spec is more specific on how the ResourceBlock is encoded. There's ambiguity there (see the quoted bit below). It should at least say that analogous rules as in RFC 3779 apply and preferably be more precise. > I really have the goal to replace all the ASN.1 fumbling around ASid and > IPAddress in rpki-client using not the low-level OpenSSL ASN.1 functions. I need to dust off my diffs. I held them back since I wasn't sure if we wanted to introduce a dependency on LibreSSL 3.5 already. With this diff that's a done deal, so there's no more reason for me to wait. > > * 4.2 requires that the resources in the checklist match the EE cert's > > resources. > > > > In your code there is the implicit assumption that the asID and > > ipAddrBlocks > > are subject to the rules in RFC 3779 (sorted, no overlap, no adjacency), > > specifically the rules in 3.2.3.4 and 2.2.3.6. That isn't really spelled > > out > > in the spec as far as I can see. Also, multiple IPLists per AFI seem a > > priori > > permitted (which is not allowed in RFC 3779 2.2.3.3). > > > > I think something should explicitly be said to tighten the requirements > > on the > > resource lists.
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Mon, May 09, 2022 at 01:07:17PM +, Job Snijders wrote:
> On Mon, May 09, 2022 at 12:11:22PM +0200, Claudio Jeker wrote:
> > why does the draft allow for optional filenames? What the heck is the
> > digest then covering some random gunk?
>
> Yes, that is entirely possible. Some folks in the working group
> requested the filename to be optional, I abided. In inter-business
> workflows that are less filesystem centric, I imagine it is possible
> that one party challenges the other party to produce a RSC signing a
> specific (random) SHA256 hash, and then validating the RSC to see if the
> desired hash showed up.
>
> > This will make it close to impossible to actually fully validate a RSC
> > (which would include the validation of all SHA256 signatures). Because
> > of this RSC validation in rpki-client will be close to impossible
> > because how do you implement:
> >
> > To verify a set of digital objects with an RSC:
> >
> >* The message digest of each referenced digital object, using the
> > digest algorithm specified in the the digestAlgorithm field, MUST
> > be calculated and MUST match the value given in the messageDigest
> > field of the associated FileNameAndHash, for the digital object to
> > be considered as verified with the RSC.
> >
> > When there is no filename to reference an object?
>
> I think we should print the hash(es) where the filename was left out,
> and leave it up to the relying party how to proceed and what to compare
> against.
>
> > > + case CERT_IP_INHERIT:
> > > + warnx("%s: RSC ResourceBlock: illegal inherit", fn);
> > > + break;
> >
> > I'm not sure if this works the way you think it does. The switch case is
> > only triggers when valid_ip() fails but that may not be the case if
> > rsc->ips[i].type == CERT_IP_INHERIT.
> >
> > I think the proper way to check this is above the valid_ip() call to make
> > sure CERT_IP_INHERIT can't be used. The parse kind of makes sure of this
> > (it does not handle NULL ipAddrOrRange objects so it will bork on that
> > with an error).
>
> done.
>
> > Actually this function is not used anyway because the only place it was
> > called was in the parse.c code but that code is no needed.
>
> I'm not entirely sure I follow? valid_rsc() is called in
> proc_parser_rsc(), which is called in parse_entity().
proc_parser_rsc() is not needed. The parser is only used to validate the
RPKI repositories. RSC files are NOT allowed to be in the repository so
there should never be a call to proc_parser_rsc().
With proc_parser_rsc() gone, valid_rsc() is no longer called
(proc_parser_rsc() was the only consumer).
> > Also this code should probably verify the fileandhash list if we are
> > serious about full RSC validation. rpki-client -f currently skips some
> > other validation steps so it mostly depends on proper x509 validation.
>
> which other steps are skipped?
filemode.c code currently fails to call valid_roa, I'm not sure if
valid_cert and valid_ta is called for the passed certificate.
On MFT the file and hash are just printed but valid_filehash() is not
called to validate that the contents of the MFT are actually present.
I think for an MFT this is fine but I would prefer if for RSC files the
files are validated via valid_filehash() before telling the world that
this is OK.
> Below is the latest patch based on feedback from you and Theo.
A few comments inline, with those applied I think this can go in and
be further adjusted there. OK claudio@
> Kind regards,
>
> Job
>
>
> Index: usr.sbin/rpki-client/Makefile
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- usr.sbin/rpki-client/Makefile 21 Apr 2022 09:53:07 - 1.24
> +++ usr.sbin/rpki-client/Makefile 9 May 2022 13:04:16 -
> @@ -5,7 +5,7 @@ SRCS= as.c cert.c cms.c crl.c encoding.c
> log.c main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
> output-csv.c output-json.c parser.c print.c repo.c roa.c rrdp.c \
> rrdp_delta.c rrdp_notification.c rrdp_snapshot.c rrdp_util.c \
> - rsync.c tal.c validate.c x509.c
> + rsc.c rsync.c tal.c validate.c x509.c
> MAN= rpki-client.8
>
> LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
> Index: usr.sbin/rpki-client/extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.132
> diff -u -p -r1.132 extern.h
> --- usr.sbin/rpki-client/extern.h 21 Apr 2022 12:59:03 - 1.132
> +++ usr.sbin/rpki-client/extern.h 9 May 2022 13:04:16 -
> @@ -24,6 +24,14 @@
> #include
> #include
>
> +/*
> + * Enumeration for ASN.1 explicit tags in RSC eContent
> + */
> +enum rsc_resourceblock_tag {
> + RSRCBLK_TYPE_ASID,
> + RSRCBLK_TYPE_IPADDRBLK,
> +};
> +
> enum cert_as_type {
>
Re: [External] : net lock priority
On Sun, May 08, 2022 at 10:54:01PM +0200, Alexandr Nedvedicky wrote:
> what bothers me is the situation where there are
> more than one reader. The line 350 is executed by
> the first reader which drops the lock. So the process
> woken up by wakeup(rwl) are going to find out the
> lock is still occupied by remaining readers.
wakeup() activates all sleepers. They should check and sleep again.
Maybe a little bit of wasted resources, but I don't see a severe
problem.
I did a little digging in history. In rev 1.3 of kern_rwlock.c
we had case RW_READ: need_wait = RWLOCK_WRLOCK | RWLOCK_WRWANT;
and the commet "Let writers through before obtaining read lock."
The comment
* RW_READ RWLOCK_WRLOCK|RWLOCK_WRWANT may not be set. We increment
* with RWLOCK_READ_INCR. RWLOCK_WAIT while waiting.
is still there, just the RWLOCK_WRWANT got lost from the condition.
So I think we should get back the original reader writer priority.
Regarding the race in rw_do_exit() that sashan@ found I also found
a comment in rev 1.7.
/*
* Potential MP race here. If the owner had WRWANT set we cleared
* it and a reader can sneak in before a writer. Do we care?
*/
I do not want to change anything to that behavior now. There is
no easy fix and I did not see the problem during testing. But we
can put the comment back to clarify the situation.
ok?
bluhm
Index: kern/kern_rwlock.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.47
diff -u -p -r1.47 kern_rwlock.c
--- kern/kern_rwlock.c 8 Feb 2021 08:18:45 - 1.47
+++ kern/kern_rwlock.c 9 May 2022 07:36:35 -
@@ -81,7 +81,7 @@ static const struct rwlock_op {
},
{ /* RW_READ */
RWLOCK_READ_INCR,
- RWLOCK_WRLOCK,
+ RWLOCK_WRLOCK | RWLOCK_WRWANT,
RWLOCK_WAIT,
0,
PLOCK
@@ -103,7 +103,7 @@ rw_enter_read(struct rwlock *rwl)
{
unsigned long owner = rwl->rwl_owner;
- if (__predict_false((owner & RWLOCK_WRLOCK) ||
+ if (__predict_false((owner & (RWLOCK_WRLOCK | RWLOCK_WRWANT)) ||
rw_cas(>rwl_owner, owner, owner + RWLOCK_READ_INCR)))
rw_enter(rwl, RW_READ);
else {
@@ -340,6 +340,11 @@ rw_do_exit(struct rwlock *rwl, unsigned
do {
owner = rwl->rwl_owner;
+ /*
+* Potential MP race here. If the owner had WRWANT set we
+* cleared it and a reader can sneak in before a writer.
+* Do we care?
+*/
if (wrlock)
set = 0;
else
vmm: give a lonely enum a friend, fixing `vmctl receive`
tech@,
Another vmm/vmd update: fix `vmctl receive` on Intel hosts by adding
another fault enum value to disambiguate fault reasons.
It's expected that the guest will trigger nested page faults after being
received by vmd. When you connect to the vm using `vmctl console` and
interact with the guest, it generates both a page fault and interrupt.
This combo is special because while the page fault will be handled by
vmm via uvm_fault(9), it will still exit to userland/vmd to handle the
interrupt.
vmd always checks the vm-exit reason after the return from vmm before
looping around and servicing interrupts before re-entering vmm. vmd has
a single userland handler for nested page faults for when we have a
protection fault. In this case, it reboots the vm. :-(
Since the enum we used for the fault type flag has only one value, vmm
isn't able to properly convey the type of nested fault. In this case, I
chose to add a "VEE_FAULT_HANDLED" value to indicate the fault has
already been handled by vmm and no userland assist is needed. (And
HANDLED is the same num of characters of PROTECT.)
This prevents the ambiguity and vm happily skips rebooting the vm.
It's possible this reboot could occur at any point in a vm's lifetime,
though I think the probability is low, so this is worth fixing
regardless.
ok?
-dv
Index: sys/arch/amd64/amd64/vmm.c
===
RCS file: /opt/cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.308
diff -u -p -r1.308 vmm.c
--- sys/arch/amd64/amd64/vmm.c 4 May 2022 02:24:26 - 1.308
+++ sys/arch/amd64/amd64/vmm.c 9 May 2022 13:45:02 -
@@ -5732,14 +5732,16 @@ vmx_fault_page(struct vcpu *vcpu, paddr_
int fault_type, ret;
fault_type = vmx_get_guest_faulttype();
- if (fault_type == -1) {
+ switch (fault_type) {
+ case -1:
printf("%s: invalid fault type\n", __func__);
return (EINVAL);
- }
-
- if (fault_type == VM_FAULT_PROTECT) {
+ case VM_FAULT_PROTECT:
vcpu->vc_exit.vee.vee_fault_type = VEE_FAULT_PROTECT;
return (EAGAIN);
+ default:
+ vcpu->vc_exit.vee.vee_fault_type = VEE_FAULT_HANDLED;
+ break;
}
/* We may sleep during uvm_fault(9), so reload VMCS. */
Index: sys/arch/amd64/include/vmmvar.h
===
RCS file: /opt/cvs/src/sys/arch/amd64/include/vmmvar.h,v
retrieving revision 1.75
diff -u -p -r1.75 vmmvar.h
--- sys/arch/amd64/include/vmmvar.h 3 May 2022 21:39:19 - 1.75
+++ sys/arch/amd64/include/vmmvar.h 9 May 2022 13:38:18 -
@@ -324,7 +324,8 @@ enum {
};
enum {
- VEE_FAULT_PROTECT
+ VEE_FAULT_HANDLED,
+ VEE_FAULT_PROTECT,
};
enum {
add support for AX210/AX211 devices to iwx(4)
This patch adds support for AX210/AX211 devices to iwx(4).
While this patch attempts to make a couple of devices work which
are part of this device family, so far only one specific AX210
device has been tested:
iwx0 at pci4 dev 0 function 0 "Intel Wi-Fi 6 AX210" rev 0x1a, msix
iwx0: hw rev 0x420, fw 67.8f59b80b.0, pnvm dda57f4f, address ba:d0:f1:95:2b:a0
Problems are not entirely unexpected when testing on other devices.
Hopefully, any AX210/AX211 devices will work. But since several variants
of firmware are involved and each firmware variant requires a specific
device, I cannot test everything by myself.
If your device is not working please enable 'ifconfig iwx0 debug' and
include the additional information this prints to /var/log/messages
in your test report.
I have tested this patch on AX200 and AX201 devices as well. I do not
see any regressions but additional testing would be welcome.
AX210 device firmware is already available via fw_update.
The required firmware package version is iwx-firmware-20220110.
Please update your tree before applying this patch. I made a commit
to sys/dev/pci/pcidevs earlier today which this patch depends on.
It will fail to compile otherwise.
diff 9d011d1bb76b879a432878d921f2f89e1153b973 refs/heads/ax210
blob - ba8e3352346b081628ac1b4999fc7681b48eb695
blob + cad4b806a5bb3626af3b395da4ab8bd55f8cd17f
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -175,6 +175,7 @@ static const uint8_t iwx_nvm_channels_uhb[] = {
};
#define IWX_NUM_2GHZ_CHANNELS 14
+#define IWX_NUM_5GHZ_CHANNELS 37
const struct iwx_rate {
uint16_t rate;
@@ -239,7 +240,10 @@ intiwx_store_cscheme(struct iwx_softc *, uint8_t
*, s
intiwx_alloc_fw_monitor_block(struct iwx_softc *, uint8_t, uint8_t);
intiwx_alloc_fw_monitor(struct iwx_softc *, uint8_t);
intiwx_apply_debug_destination(struct iwx_softc *);
+void iwx_set_ltr(struct iwx_softc *);
intiwx_ctxt_info_init(struct iwx_softc *, const struct iwx_fw_sects *);
+intiwx_ctxt_info_gen3_init(struct iwx_softc *,
+ const struct iwx_fw_sects *);
void iwx_ctxt_info_free_fw_img(struct iwx_softc *);
void iwx_ctxt_info_free_paging(struct iwx_softc *);
intiwx_init_fw_sec(struct iwx_softc *, const struct iwx_fw_sects *,
@@ -250,10 +254,15 @@ int iwx_firmware_store_section(struct iwx_softc *,
enu
intiwx_set_default_calib(struct iwx_softc *, const void *);
void iwx_fw_info_free(struct iwx_fw_info *);
intiwx_read_firmware(struct iwx_softc *);
+uint32_t iwx_prph_addr_mask(struct iwx_softc *);
uint32_t iwx_read_prph_unlocked(struct iwx_softc *, uint32_t);
uint32_t iwx_read_prph(struct iwx_softc *, uint32_t);
void iwx_write_prph_unlocked(struct iwx_softc *, uint32_t, uint32_t);
void iwx_write_prph(struct iwx_softc *, uint32_t, uint32_t);
+uint32_t iwx_read_umac_prph_unlocked(struct iwx_softc *, uint32_t);
+uint32_t iwx_read_umac_prph(struct iwx_softc *, uint32_t);
+void iwx_write_umac_prph_unlocked(struct iwx_softc *, uint32_t, uint32_t);
+void iwx_write_umac_prph(struct iwx_softc *, uint32_t, uint32_t);
intiwx_read_mem(struct iwx_softc *, uint32_t, void *, int);
intiwx_write_mem(struct iwx_softc *, uint32_t, const void *, int);
intiwx_write_mem32(struct iwx_softc *, uint32_t, uint32_t);
@@ -337,6 +346,10 @@ intiwx_is_valid_mac_addr(const uint8_t *);
intiwx_nvm_get(struct iwx_softc *);
intiwx_load_firmware(struct iwx_softc *);
intiwx_start_fw(struct iwx_softc *);
+intiwx_pnvm_handle_section(struct iwx_softc *, const uint8_t *, size_t);
+intiwx_pnvm_parse(struct iwx_softc *, const uint8_t *, size_t);
+void iwx_ctxt_info_gen3_set_pnvm(struct iwx_softc *);
+intiwx_load_pnvm(struct iwx_softc *);
intiwx_send_tx_ant_cfg(struct iwx_softc *, uint8_t);
intiwx_send_phy_cfg_cmd(struct iwx_softc *);
intiwx_load_ucode_wait_alive(struct iwx_softc *);
@@ -357,7 +370,7 @@ voidiwx_rx_frame(struct iwx_softc *, struct mbuf *,
i
uint32_t, struct ieee80211_rxinfo *, struct mbuf_list *);
void iwx_clear_tx_desc(struct iwx_softc *, struct iwx_tx_ring *, int);
void iwx_txd_done(struct iwx_softc *, struct iwx_tx_data *);
-void iwx_txq_advance(struct iwx_softc *, struct iwx_tx_ring *, int);
+void iwx_txq_advance(struct iwx_softc *, struct iwx_tx_ring *, uint16_t);
void iwx_rx_tx_cmd(struct iwx_softc *, struct iwx_rx_packet *,
struct iwx_rx_data *);
void iwx_clear_oactive(struct iwx_softc *, struct iwx_tx_ring *);
@@ -381,8 +394,9 @@ int iwx_send_cmd_pdu_status(struct iwx_softc *, uint32
void iwx_free_resp(struct iwx_softc *, struct iwx_host_cmd *);
void iwx_cmd_done(struct iwx_softc *, int, int, int);
const struct iwx_rate *iwx_tx_fill_cmd(struct iwx_softc *, struct iwx_node *,
- struct ieee80211_frame *, struct iwx_tx_cmd_gen2 *);
-void iwx_tx_update_byte_tbl(struct iwx_tx_ring *, int, uint16_t, uint16_t);
+ struct ieee80211_frame *, uint16_t *,
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Mon, May 09, 2022 at 12:11:22PM +0200, Claudio Jeker wrote:
> why does the draft allow for optional filenames? What the heck is the
> digest then covering some random gunk?
Yes, that is entirely possible. Some folks in the working group
requested the filename to be optional, I abided. In inter-business
workflows that are less filesystem centric, I imagine it is possible
that one party challenges the other party to produce a RSC signing a
specific (random) SHA256 hash, and then validating the RSC to see if the
desired hash showed up.
> This will make it close to impossible to actually fully validate a RSC
> (which would include the validation of all SHA256 signatures). Because
> of this RSC validation in rpki-client will be close to impossible
> because how do you implement:
>
> To verify a set of digital objects with an RSC:
>
>* The message digest of each referenced digital object, using the
> digest algorithm specified in the the digestAlgorithm field, MUST
> be calculated and MUST match the value given in the messageDigest
> field of the associated FileNameAndHash, for the digital object to
> be considered as verified with the RSC.
>
> When there is no filename to reference an object?
I think we should print the hash(es) where the filename was left out,
and leave it up to the relying party how to proceed and what to compare
against.
> > + case CERT_IP_INHERIT:
> > + warnx("%s: RSC ResourceBlock: illegal inherit", fn);
> > + break;
>
> I'm not sure if this works the way you think it does. The switch case is
> only triggers when valid_ip() fails but that may not be the case if
> rsc->ips[i].type == CERT_IP_INHERIT.
>
> I think the proper way to check this is above the valid_ip() call to make
> sure CERT_IP_INHERIT can't be used. The parse kind of makes sure of this
> (it does not handle NULL ipAddrOrRange objects so it will bork on that
> with an error).
done.
> Actually this function is not used anyway because the only place it was
> called was in the parse.c code but that code is no needed.
I'm not entirely sure I follow? valid_rsc() is called in
proc_parser_rsc(), which is called in parse_entity().
> Also this code should probably verify the fileandhash list if we are
> serious about full RSC validation. rpki-client -f currently skips some
> other validation steps so it mostly depends on proper x509 validation.
which other steps are skipped?
Below is the latest patch based on feedback from you and Theo.
Kind regards,
Job
Index: usr.sbin/rpki-client/Makefile
===
RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
retrieving revision 1.24
diff -u -p -r1.24 Makefile
--- usr.sbin/rpki-client/Makefile 21 Apr 2022 09:53:07 - 1.24
+++ usr.sbin/rpki-client/Makefile 9 May 2022 13:04:16 -
@@ -5,7 +5,7 @@ SRCS= as.c cert.c cms.c crl.c encoding.c
log.c main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
output-csv.c output-json.c parser.c print.c repo.c roa.c rrdp.c \
rrdp_delta.c rrdp_notification.c rrdp_snapshot.c rrdp_util.c \
- rsync.c tal.c validate.c x509.c
+ rsc.c rsync.c tal.c validate.c x509.c
MAN= rpki-client.8
LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
Index: usr.sbin/rpki-client/extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.132
diff -u -p -r1.132 extern.h
--- usr.sbin/rpki-client/extern.h 21 Apr 2022 12:59:03 - 1.132
+++ usr.sbin/rpki-client/extern.h 9 May 2022 13:04:16 -
@@ -24,6 +24,14 @@
#include
#include
+/*
+ * Enumeration for ASN.1 explicit tags in RSC eContent
+ */
+enum rsc_resourceblock_tag {
+ RSRCBLK_TYPE_ASID,
+ RSRCBLK_TYPE_IPADDRBLK,
+};
+
enum cert_as_type {
CERT_AS_ID, /* single identifier */
CERT_AS_INHERIT, /* inherit from parent */
@@ -164,6 +172,7 @@ enum rtype {
RTYPE_ASPA,
RTYPE_REPO,
RTYPE_FILE,
+ RTYPE_RSC,
};
enum location {
@@ -232,6 +241,29 @@ struct roa {
time_t expires; /* do not use after */
};
+struct rscfile {
+ char*filename; /* an optional filename on the checklist */
+ unsigned charhash[SHA256_DIGEST_LENGTH]; /* the digest */
+};
+
+/*
+ * A Signed Checklist (RSC)
+ */
+struct rsc {
+ int talid; /* RSC covered by what TAL */
+ int valid; /* eContent resources covered by EE's 3779? */
+ struct cert_ip *ips; /* IP prefixes */
+ size_t ipsz; /* number of IP prefixes */
+ struct cert_as *as; /* AS resources */
+ size_t asz; /* number of AS resources */
+ struct rscfile *files; /* FileAndHashes in the RSC */
+ size_t filesz; /* number of FileAndHashes */
+ char*aia; /*
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Mon, May 09, 2022 at 12:53:05PM +0200, Theo Buehler wrote:
> > As the various same-named-but-different 'parse' structs are not easily
> > interchangeable without more refactoring, I marked them "XXX:". Perhaps
> > we can work on that in tree?
>
> I'm fine with fixing that in-tree. Sorry about this mistake, I made it
> many times. I wish the various 'struct parse' were explicitly part of
> the per-file namespace (e.g., cert_parse, rsc_parse, ...).
>
> I have more comments. Most of what I had was covered by claudio's
> review, so I left that out. What remains inline are a few minor nits
> and one important thing regarding mktime().
>
> The openssl11 regress needs a small fix (diff at the end)
>
> At the moment the code only parses the filenames and hashes. They aren't
> used (except for printing). I assume the actual verification of files
> against the hashes will be a separate diff?
>
> What's the purpose of naked hashes (where the optional filename is
> omitted)?
>
> During a normal rpki-client run: what checks that the EE cert's
> resources match the ones listed in the .sig file? How is the .sig file
> handed to rpki-client?
During a normal rpki-client no RSC .sig files should be encountered. If
they show up that is an error and the files should be ignored.
RSC validation only makes sense with rpki-client -f but we need to make
sure that we only mark the output as valid if it actually is fully valid.
See my comment on this in myt previous mail.
> Regarding the spec:
>
> * isn't it a bit unfortunate that the ResourceBlock contains an ipAddrBlocks
> member which isn't an IPAddrBlocks as in RFC 3779 but rather an IPList?
>
> The (somewhat) subtle difference is that an IPAddressFamilyItem has
> an iPAddressOrRange where an IPAddressFamily has an IPAddressChoice.
> I assume this was done because inheritance is excluded in this draft.
Uh, yes, it would be good to be able to use RFC 3779 parser functions on
can limit the usage of inheritance after parsing the objects.
I really have the goal to replace all the ASN.1 fumbling around ASid and
IPAddress in rpki-client using not the low-level OpenSSL ASN.1 functions.
> * 4.2 requires that the resources in the checklist match the EE cert's
> resources.
>
> In your code there is the implicit assumption that the asID and ipAddrBlocks
> are subject to the rules in RFC 3779 (sorted, no overlap, no adjacency),
> specifically the rules in 3.2.3.4 and 2.2.3.6. That isn't really spelled out
> in the spec as far as I can see. Also, multiple IPLists per AFI seem a
> priori
> permitted (which is not allowed in RFC 3779 2.2.3.3).
>
> I think something should explicitly be said to tighten the requirements on
> the
> resource lists.
>
> * There's a misplaced HTML entity '' in the draft (page 5, end of l -14):
>
> IPAddressFamilyItem ::= SEQUENCE {-- AFI optional SAFI --
>
> * It is very unclear to me why filenames are OPTIONAL and what to do if they
> are actually left out. Go look for anything that matches?
Both of us wonder about this, I guess this is something the draft should
either remove or cover in a proper way :)
I really dislike the idea of "go look for possible matches".
> > Index: usr.sbin/rpki-client/rsc.c
>
> [...]
>
> > +static int
> > +rsc_check_digesttype(struct parse *p, const unsigned char *d, size_t dsz)
> > +{
> > + X509_ALGOR *alg;
> > +const ASN1_OBJECT *obj;
> > +int type, nid;
> > +int rc = 0;
>
> Use tabs instead of 8 spaces in the previous three lines.
>
> [...]
>
> > +static int
> > +rsc_parse_ipaddrfamitem(struct parse *p, const ASN1_OCTET_STRING *os)
> > +{
> > + ASN1_OCTET_STRING *aos = NULL;
> > + IPAddressOrRange*aor = NULL;
> > + int tag;
> > + const unsigned char *cnt = os->data;
> > + long cntsz;
> > + const unsigned char *d;
> > + size_t dsz = os->length;
>
> Please drop dsz and use os->length directly below...
>
> > + struct cert_ip ip;
> > + int rc = 0;
> > +
> > + memset(, 0, sizeof(struct cert_ip));
> > +
> > + /*
> > +* IPAddressFamilyItem is a sequence containing an addressFamily and
> > +* an IPAddressOrRange.
> > +*/
> > + if (!ASN1_frame(p->fn, dsz, , , )) {
>
> ...like this:
>
> if (!ASN1_frame(p->fn, os->length, , , )) {
>
> [...]
>
> > + at = X509_get0_notAfter(*x509);
> > + if (at == NULL) {
> > + warnx("%s: X509_get0_notAfter failed", fn);
> > + goto out;
> > + }
>
> We have a helper for the next few lines (and using mktime() is wrong here):
> https://ftp.openbsd.org/pub/OpenBSD/patches/7.0/common/020_rpki.patch.sig
>
> > + memset(_tm, 0, sizeof(expires_tm));
> > + if (ASN1_time_parse(at->data, at->length, _tm, 0) == -1) {
> > + warnx("%s: ASN1_time_parse failed", fn);
> > +
Re: uvm: Consider BUFPAGES_DEFICIT in swap_shortage
On 05/05/22(Thu) 10:56, Bob Beck wrote:
> On Thu, May 05, 2022 at 10:16:23AM -0600, Bob Beck wrote:
> > Ugh. You???re digging in the most perilous parts of the pile.
> >
> > I will go look with you??? sigh. (This is not yet an ok for that.)
> >
> > > On May 5, 2022, at 7:53 AM, Martin Pieuchot wrote:
> > >
> > > When considering the amount of free pages in the page daemon a small
> > > amount is always kept for the buffer cache... except in one place.
> > >
> > > The diff below gets rid of this exception. This is important because
> > > uvmpd_scan() is called conditionally using the following check:
> > >
> > > if (uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) {
> > >...
> > > }
> > >
> > > So in case of swap shortage we might end up freeing fewer pages than
> > > wanted.
>
> So a bit of backgroud.
>
> I am pretty much of the belief that this "low water mark" for pages is
> nonsense now. I was in the midst of trying to prove that
> to myself and therefore rip down some of the crazy accounting and
> very arbitrary limits in the buffer cache and got distracted.
>
> Maybe something like this to start? (buf failing that I think
> your current diff is probably ok).
Thanks. I'll commit my diff then to make the current code coherent and
let me progress in my refactoring. Then we can consider changing this
magic.
> Index: sys/sys/mount.h
> ===
> RCS file: /cvs/src/sys/sys/mount.h,v
> retrieving revision 1.148
> diff -u -p -u -p -r1.148 mount.h
> --- sys/sys/mount.h 6 Apr 2021 14:17:35 - 1.148
> +++ sys/sys/mount.h 5 May 2022 16:50:50 -
> @@ -488,10 +488,8 @@ struct bcachestats {
> #ifdef _KERNEL
> extern struct bcachestats bcstats;
> extern long buflowpages, bufhighpages, bufbackpages;
> -#define BUFPAGES_DEFICIT (((buflowpages - bcstats.numbufpages) < 0) ? 0 \
> -: buflowpages - bcstats.numbufpages)
> -#define BUFPAGES_INACT (((bcstats.numcleanpages - buflowpages) < 0) ? 0 \
> -: bcstats.numcleanpages - buflowpages)
> +#define BUFPAGES_DEFICIT 0
> +#define BUFPAGES_INACT bcstats.numcleanpages
> extern int bufcachepercent;
> extern void bufadjust(int);
> struct uvm_constraint_range;
>
>
> > >
> > > ok?
> > >
> > > Index: uvm/uvm_pdaemon.c
> > > ===
> > > RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> > > retrieving revision 1.98
> > > diff -u -p -r1.98 uvm_pdaemon.c
> > > --- uvm/uvm_pdaemon.c 4 May 2022 14:58:26 - 1.98
> > > +++ uvm/uvm_pdaemon.c 5 May 2022 13:40:28 -
> > > @@ -923,12 +923,13 @@ uvmpd_scan(void)
> > >* detect if we're not going to be able to page anything out
> > >* until we free some swap resources from active pages.
> > >*/
> > > + free = uvmexp.free - BUFPAGES_DEFICIT;
> > > swap_shortage = 0;
> > > - if (uvmexp.free < uvmexp.freetarg &&
> > > + if (free < uvmexp.freetarg &&
> > > uvmexp.swpginuse == uvmexp.swpages &&
> > > !uvm_swapisfull() &&
> > > pages_freed == 0) {
> > > - swap_shortage = uvmexp.freetarg - uvmexp.free;
> > > + swap_shortage = uvmexp.freetarg - free;
> > > }
> > >
> > > for (p = TAILQ_FIRST(_active);
> > >
> >
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
> As the various same-named-but-different 'parse' structs are not easily
> interchangeable without more refactoring, I marked them "XXX:". Perhaps
> we can work on that in tree?
I'm fine with fixing that in-tree. Sorry about this mistake, I made it
many times. I wish the various 'struct parse' were explicitly part of
the per-file namespace (e.g., cert_parse, rsc_parse, ...).
I have more comments. Most of what I had was covered by claudio's
review, so I left that out. What remains inline are a few minor nits
and one important thing regarding mktime().
The openssl11 regress needs a small fix (diff at the end)
At the moment the code only parses the filenames and hashes. They aren't
used (except for printing). I assume the actual verification of files
against the hashes will be a separate diff?
What's the purpose of naked hashes (where the optional filename is
omitted)?
During a normal rpki-client run: what checks that the EE cert's
resources match the ones listed in the .sig file? How is the .sig file
handed to rpki-client?
Regarding the spec:
* isn't it a bit unfortunate that the ResourceBlock contains an ipAddrBlocks
member which isn't an IPAddrBlocks as in RFC 3779 but rather an IPList?
The (somewhat) subtle difference is that an IPAddressFamilyItem has
an iPAddressOrRange where an IPAddressFamily has an IPAddressChoice.
I assume this was done because inheritance is excluded in this draft.
* 4.2 requires that the resources in the checklist match the EE cert's
resources.
In your code there is the implicit assumption that the asID and ipAddrBlocks
are subject to the rules in RFC 3779 (sorted, no overlap, no adjacency),
specifically the rules in 3.2.3.4 and 2.2.3.6. That isn't really spelled out
in the spec as far as I can see. Also, multiple IPLists per AFI seem a priori
permitted (which is not allowed in RFC 3779 2.2.3.3).
I think something should explicitly be said to tighten the requirements on the
resource lists.
* There's a misplaced HTML entity '' in the draft (page 5, end of l -14):
IPAddressFamilyItem ::= SEQUENCE {-- AFI optional SAFI --
* It is very unclear to me why filenames are OPTIONAL and what to do if they
are actually left out. Go look for anything that matches?
> Index: usr.sbin/rpki-client/rsc.c
[...]
> +static int
> +rsc_check_digesttype(struct parse *p, const unsigned char *d, size_t dsz)
> +{
> + X509_ALGOR *alg;
> +const ASN1_OBJECT*obj;
> +int type, nid;
> +int rc = 0;
Use tabs instead of 8 spaces in the previous three lines.
[...]
> +static int
> +rsc_parse_ipaddrfamitem(struct parse *p, const ASN1_OCTET_STRING *os)
> +{
> + ASN1_OCTET_STRING *aos = NULL;
> + IPAddressOrRange*aor = NULL;
> + int tag;
> + const unsigned char *cnt = os->data;
> + long cntsz;
> + const unsigned char *d;
> + size_t dsz = os->length;
Please drop dsz and use os->length directly below...
> + struct cert_ip ip;
> + int rc = 0;
> +
> + memset(, 0, sizeof(struct cert_ip));
> +
> + /*
> + * IPAddressFamilyItem is a sequence containing an addressFamily and
> + * an IPAddressOrRange.
> + */
> + if (!ASN1_frame(p->fn, dsz, , , )) {
...like this:
if (!ASN1_frame(p->fn, os->length, , , )) {
[...]
> + at = X509_get0_notAfter(*x509);
> + if (at == NULL) {
> + warnx("%s: X509_get0_notAfter failed", fn);
> + goto out;
> + }
We have a helper for the next few lines (and using mktime() is wrong here):
https://ftp.openbsd.org/pub/OpenBSD/patches/7.0/common/020_rpki.patch.sig
> + memset(_tm, 0, sizeof(expires_tm));
> + if (ASN1_time_parse(at->data, at->length, _tm, 0) == -1) {
> + warnx("%s: ASN1_time_parse failed", fn);
> + goto out;
> + }
> + if ((expires = mktime(_tm)) == -1)
> + errx(1, "mktime failed");
> +
> + p.res->expires = expires;
Replace the above with:
if (x509_get_time(at, >expires) == -1) {
warnx("%s: ASN1_time_parse failed", fn);
goto out;
}
[...]
For the openssl11 test, you'll need the following diff:
Index: openssl11/Makefile
===
RCS file: /cvs/src/regress/usr.sbin/rpki-client/openssl11/Makefile,v
retrieving revision 1.7
diff -u -p -r1.7 Makefile
--- openssl11/Makefile 20 Apr 2022 17:37:53 - 1.7
+++ openssl11/Makefile 9 May 2022 08:25:59 -
@@ -26,6 +26,7 @@ SRCS_test-gbr = a_time_tm_gen.c o_time.
SRCS_test-tal =a_time_tm_gen.c o_time.c
SRCS_test-bgpsec = a_time_tm_gen.c o_time.c
SRCS_test-rrdp = a_time_tm_gen.c o_time.c
+SRCS_test-rsc =a_time_tm_gen.c o_time.c
CFLAGS +=
Re: rpki-client: add support for draft-ietf-sidrops-rpki-rsc in filemode
On Sun, May 08, 2022 at 08:05:08PM +, Job Snijders wrote:
> Dear Theo, fellow developers,
>
> Many thanks for the first review pass, much appreciated.
>
> > This is a good first step. I have a few initial comments inline. Once you
> > fix
> > those, review of the rest will be easier.
> >
> > The main thing is: please remove all the copy-pasted functions that had no
> > changes whatsoever or only very minor changes.
>
> As the various same-named-but-different 'parse' structs are not easily
> interchangeable without more refactoring, I marked them "XXX:". Perhaps
> we can work on that in tree?
>
> > [snip]
>
> The following is a list of changes compared to previous changeset
> proposal:
>
> * removed the __func__ references
> * added copyright lines
> * use d2i_X509_ALGOR() instead of deserializing by hand
> * removed all 'else if'
> * fixed spaces vs tabs
> * renamed elemz to elemsz, and made the check more readable
> * added default case in switch (aor->type)
> * removed superfluous parenthesis
See inline comments
> Index: usr.sbin/rpki-client/Makefile
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- usr.sbin/rpki-client/Makefile 21 Apr 2022 09:53:07 - 1.24
> +++ usr.sbin/rpki-client/Makefile 8 May 2022 20:01:25 -
> @@ -5,7 +5,7 @@ SRCS= as.c cert.c cms.c crl.c encoding.c
> log.c main.c mft.c mkdir.c output.c output-bgpd.c output-bird.c \
> output-csv.c output-json.c parser.c print.c repo.c roa.c rrdp.c \
> rrdp_delta.c rrdp_notification.c rrdp_snapshot.c rrdp_util.c \
> - rsync.c tal.c validate.c x509.c
> + rsc.c rsync.c tal.c validate.c x509.c
> MAN= rpki-client.8
>
> LDADD+= -lexpat -ltls -lssl -lcrypto -lutil
> Index: usr.sbin/rpki-client/extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.132
> diff -u -p -r1.132 extern.h
> --- usr.sbin/rpki-client/extern.h 21 Apr 2022 12:59:03 - 1.132
> +++ usr.sbin/rpki-client/extern.h 8 May 2022 20:01:25 -
> @@ -24,6 +24,14 @@
> #include
> #include
>
> +/*
> + * Enumeration for ASN.1 explicit tags in RSC eContent
> + */
> +enum rsc_resourceblock_tag {
> + RSRCBLK_TYPE_ASID,
> + RSRCBLK_TYPE_IPADDRBLK,
> +};
> +
> enum cert_as_type {
> CERT_AS_ID, /* single identifier */
> CERT_AS_INHERIT, /* inherit from parent */
> @@ -164,6 +172,7 @@ enum rtype {
> RTYPE_ASPA,
> RTYPE_REPO,
> RTYPE_FILE,
> + RTYPE_RSC,
> };
>
> enum location {
> @@ -232,6 +241,29 @@ struct roa {
> time_t expires; /* do not use after */
> };
>
> +struct rscfile {
> + char*filename; /* an optional filename on the checklist */
> + unsigned charhash[SHA256_DIGEST_LENGTH]; /* the digest */
> +};
> +
> +/*
> + * A Signed Checklist (RSC)
> + */
> +struct rsc {
> + int talid; /* RSC covered by what TAL */
> + int valid; /* eContent resources covered by EE's 3779? */
> + struct cert_ip *ips; /* IP prefixes */
> + size_t ipsz; /* number of IP prefixes */
> + struct cert_as *as; /* AS resources */
> + size_t asz; /* number of AS resources */
> + struct rscfile *files; /* FileAndHashes in the RSC */
> + size_t filesz; /* number of FileAndHashes */
> + char*aia; /* AIA */
> + char*aki; /* AKI */
> + char*ski; /* SKI */
> + time_t expires; /* Not After of the RSC EE */
> +};
> +
> /*
> * A single Ghostbuster record
> */
> @@ -450,6 +482,12 @@ void gbr_free(struct gbr *);
> struct gbr *gbr_parse(X509 **, const char *, const unsigned char *,
> size_t);
>
> +void rsc_buffer(struct ibuf *, const struct rsc *);
> +void rsc_free(struct rsc *);
> +struct rsc *rsc_parse(X509 **, const char *, const unsigned char *,
> + size_t);
> +struct rsc *rsc_read(struct ibuf *);
> +
Both rsc_buffer and rsc_read can be dropped here. These functions will not
be needed since .sig files are not part of a cache parse. See further
down.
> /* crl.c */
> struct crl *crl_parse(const char *, const unsigned char *, size_t);
> struct crl *crl_get(struct crl_tree *, const struct auth *);
> @@ -470,6 +508,7 @@ intvalid_uri(const char *, size_t, co
> int valid_origin(const char *, const char *);
> int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
> struct crl *, int);
> +int valid_rsc(const char *, struct auth *, struct rsc *);
>
> /* Working with CMS. */
> unsigned char*cms_parse_validate(X509 **, const char *,
> @@ -608,6 +647,7 @@ void crl_print(const
