Re: Unlock umask(2)
> On 13 May 2022, at 00:02, Alexander Bluhm wrote:
>
> On Wed, May 11, 2022 at 11:20:15AM +0300, Vitaliy Makkoveev wrote:
>> sys_umask() only modifies `fd_cmask', which modification is already
>> protected by `fd_lock' rwlock(9).
>
> I did run full regress on amd64.
>
> OK bluhm@
>
Thanks! I’ll wait couple of days before commit it.
>> Index: sys/kern/syscalls.master
>> ===
>> RCS file: /cvs/src/sys/kern/syscalls.master,v
>> retrieving revision 1.223
>> diff -u -p -r1.223 syscalls.master
>> --- sys/kern/syscalls.master 24 Feb 2022 07:41:51 - 1.223
>> +++ sys/kern/syscalls.master 11 May 2022 08:14:59 -
>> @@ -146,7 +146,7 @@
>> char *buf, size_t count); }
>> 59 STD { int sys_execve(const char *path, \
>> char * const *argp, char * const *envp); }
>> -60 STD { mode_t sys_umask(mode_t newmask); }
>> +60 STD NOLOCK { mode_t sys_umask(mode_t newmask); }
>> 61 STD { int sys_chroot(const char *path); }
>> 62 STD { int sys_getfsstat(struct statfs *buf, size_t bufsize,
>> \
>> int flags); }
>
Re: changelist: add /etc/login.conf.d/*
Hello, I take this is an ok by deraadt@ Regards, Raf On Thu, May 12, 2022 at 01:40:40PM BST, Theo de Raadt wrote: > Yep > > Raf Czlonka wrote: > > > On Thu, May 12, 2022 at 11:58:22AM BST, Stuart Henderson wrote: > > > changelist already has /etc/login.conf, but I think files in the .d > > > directory should be checked too, both so we have notification of changes > > > (as it can set environment variables this is a very powerful file), and > > > also so we keep old versions in /var/backup. > > > > The directory itself should probably also go into /etc/mtree/special. > > > > Regards, > > > > Raf > > > > Index: etc/mtree/special > > === > > RCS file: /cvs/src/etc/mtree/special,v > > retrieving revision 1.127 > > diff -u -p -r1.127 special > > --- etc/mtree/special 13 Sep 2020 10:03:46 - 1.127 > > +++ etc/mtree/special 12 May 2022 11:30:54 - > > @@ -46,6 +46,8 @@ isakmpd.policytype=file mode=0600 uname > > ldapd.conf type=file mode=0600 uname=root gname=wheel optional > > ldpd.conf type=file mode=0600 uname=root gname=wheel optional > > login.conf type=file mode=0644 uname=root gname=wheel > > +login.conf.d type=dir mode=0755 uname=root gname=wheel > > +.. #login.conf.d > > login_ldap.conftype=file mode=0640 uname=root gname=auth optional > > mail.rctype=file mode=0644 uname=root gname=wheel > > mailer.conftype=file mode=0644 uname=root gname=wheel > >
libcrypto/err_prn.c: skip BIO*
Hi
As far as I can tell, this ends up calling vprintf eventually, so
skip the steps inbetween.
Best,
Martin
Index: err_prn.c
===
RCS file: /home/reposync/cvs/src/lib/libcrypto/err/err_prn.c,v
retrieving revision 1.19
diff -u -p -r1.19 err_prn.c
--- err_prn.c 7 Jan 2022 09:02:18 - 1.19
+++ err_prn.c 7 Jan 2022 16:13:48 -
@@ -92,12 +92,7 @@ ERR_print_errors_cb(int (*cb)(const char
static int
print_fp(const char *str, size_t len, void *fp)
{
- BIO bio;
-
- BIO_set(, BIO_s_file());
- BIO_set_fp(, fp, BIO_NOCLOSE);
-
- return BIO_printf(, "%s", str);
+ return fprintf(fp, "%s", str);
}
void
Re: rpki-client: ASN.1 bit string flag errors
On Thu, May 12, 2022 at 07:44:51PM +0200, Theo Buehler wrote:
> ip_addr_parse() sticks its fingers into undocumented API surface of
> libcrypto. What is true is that the unused bit count is in the lower
> three bits of p->flags, provided that ASN1_STRING_FLAG_BITS_LEFT is set.
> This is "documented" in asn1.h
>
> #define ASN1_STRING_FLAG_BITS_LEFT 0x08 /* Set if 0x07 has bits left value */
>
> What the flags beyond 0x08 mean is mostly unspecified. They a priori
> don't have anything to do with the bit string's unused bits, so the
> checks and errors make no sense.
>
> Maybe we should add a check of the form
>
> if (p->flags & ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07)) {
> warnx("%s: unexpected flags set in address bit string", fn);
> return 0;
> }
>
> since equivalent checks have been working so far, but I'm not sure.
> This complains about an implementation detail, not on anything directly
> related to the (mis)parsing of a cert, roa, or ...
Yes, this seems reasonable. Diff is OK claudio@
Not sure about the extra paranoia check. Since unused is now masked with
0x07 extra bits wont hurt anymore.
> Index: ip.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/ip.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 ip.c
> --- ip.c 11 May 2022 18:48:35 - 1.22
> +++ ip.c 12 May 2022 11:20:52 -
> @@ -189,17 +189,9 @@ ip_addr_parse(const ASN1_BIT_STRING *p,
> /* Weird OpenSSL-ism to get unused bit count. */
>
> if ((p->flags & ASN1_STRING_FLAG_BITS_LEFT))
> - unused = p->flags & ~ASN1_STRING_FLAG_BITS_LEFT;
> + unused = p->flags & 0x07;
>
> - if (unused < 0) {
> - warnx("%s: RFC 3779 section 2.2.3.8: "
> - "unused bit count must be non-negative", fn);
> - return 0;
> - } else if (unused >= 8) {
> - warnx("%s: RFC 3779 section 2.2.3.8: "
> - "unused bit count must mask an unsigned char", fn);
> - return 0;
> - } else if (p->length == 0 && unused != 0) {
> + if (p->length == 0 && unused != 0) {
> warnx("%s: RFC 3779 section 2.2.3.8: "
> "unused bit count must be zero if length is zero", fn);
> return 0;
>
--
:wq Claudio
apply(1): constify two arguments
Index: apply.c
===
RCS file: /cvs/src/usr.bin/apply/apply.c,v
retrieving revision 1.29
diff -u -p -r1.29 apply.c
--- apply.c 1 Apr 2018 17:45:05 - 1.29
+++ apply.c 12 May 2022 21:14:04 -
@@ -54,7 +54,7 @@ char *str;
size_t sz;
void
-stradd(char *p)
+stradd(const char *p)
{
size_t n;
@@ -73,7 +73,7 @@ stradd(char *p)
}
void
-strset(char *p)
+strset(const char *p)
{
if (str != NULL)
str[0] = '\0';
Re: Unlock umask(2)
On Wed, May 11, 2022 at 11:20:15AM +0300, Vitaliy Makkoveev wrote:
> sys_umask() only modifies `fd_cmask', which modification is already
> protected by `fd_lock' rwlock(9).
I did run full regress on amd64.
OK bluhm@
> Index: sys/kern/syscalls.master
> ===
> RCS file: /cvs/src/sys/kern/syscalls.master,v
> retrieving revision 1.223
> diff -u -p -r1.223 syscalls.master
> --- sys/kern/syscalls.master 24 Feb 2022 07:41:51 - 1.223
> +++ sys/kern/syscalls.master 11 May 2022 08:14:59 -
> @@ -146,7 +146,7 @@
> char *buf, size_t count); }
> 59 STD { int sys_execve(const char *path, \
> char * const *argp, char * const *envp); }
> -60 STD { mode_t sys_umask(mode_t newmask); }
> +60 STD NOLOCK { mode_t sys_umask(mode_t newmask); }
> 61 STD { int sys_chroot(const char *path); }
> 62 STD { int sys_getfsstat(struct statfs *buf, size_t bufsize,
> \
> int flags); }
rpki-client: ASN.1 bit string flag errors
ip_addr_parse() sticks its fingers into undocumented API surface of
libcrypto. What is true is that the unused bit count is in the lower
three bits of p->flags, provided that ASN1_STRING_FLAG_BITS_LEFT is set.
This is "documented" in asn1.h
#define ASN1_STRING_FLAG_BITS_LEFT 0x08 /* Set if 0x07 has bits left value */
What the flags beyond 0x08 mean is mostly unspecified. They a priori
don't have anything to do with the bit string's unused bits, so the
checks and errors make no sense.
Maybe we should add a check of the form
if (p->flags & ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07)) {
warnx("%s: unexpected flags set in address bit string", fn);
return 0;
}
since equivalent checks have been working so far, but I'm not sure.
This complains about an implementation detail, not on anything directly
related to the (mis)parsing of a cert, roa, or ...
Index: ip.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/ip.c,v
retrieving revision 1.22
diff -u -p -r1.22 ip.c
--- ip.c11 May 2022 18:48:35 - 1.22
+++ ip.c12 May 2022 11:20:52 -
@@ -189,17 +189,9 @@ ip_addr_parse(const ASN1_BIT_STRING *p,
/* Weird OpenSSL-ism to get unused bit count. */
if ((p->flags & ASN1_STRING_FLAG_BITS_LEFT))
- unused = p->flags & ~ASN1_STRING_FLAG_BITS_LEFT;
+ unused = p->flags & 0x07;
- if (unused < 0) {
- warnx("%s: RFC 3779 section 2.2.3.8: "
- "unused bit count must be non-negative", fn);
- return 0;
- } else if (unused >= 8) {
- warnx("%s: RFC 3779 section 2.2.3.8: "
- "unused bit count must mask an unsigned char", fn);
- return 0;
- } else if (p->length == 0 && unused != 0) {
+ if (p->length == 0 && unused != 0) {
warnx("%s: RFC 3779 section 2.2.3.8: "
"unused bit count must be zero if length is zero", fn);
return 0;
Re: uvm_pagedequeue()
On 10/05/22(Tue) 20:23, Mark Kettenis wrote:
> > Date: Tue, 10 May 2022 18:45:21 +0200
> > From: Martin Pieuchot
> >
> > On 05/05/22(Thu) 14:54, Martin Pieuchot wrote:
> > > Diff below introduces a new wrapper to manipulate active/inactive page
> > > queues.
> > >
> > > ok?
> >
> > Anyone?
>
> Sorry I started looking at this and got distracted.
>
> I'm not sure about the changes to uvm_pageactivate(). It doesn't
> quite match what NetBSD does, but I guess NetBSD assumes that
> uvm_pageactiave() isn't called for a page that is already active? And
> that's something we can't guarantee?
It does look at what NetBSD did 15 years ago. We're not ready to synchronize
with NetBSD -current yet.
We're getting there!
> The diff is correct though in the sense that it is equivalent to the
> code we already have. So if this definitely is the direction you want
> to go:
>
> ok kettenis@
>
> > > Index: uvm/uvm_page.c
> > > ===
> > > RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> > > retrieving revision 1.165
> > > diff -u -p -r1.165 uvm_page.c
> > > --- uvm/uvm_page.c4 May 2022 14:58:26 - 1.165
> > > +++ uvm/uvm_page.c5 May 2022 12:49:13 -
> > > @@ -987,16 +987,7 @@ uvm_pageclean(struct vm_page *pg)
> > > /*
> > >* now remove the page from the queues
> > >*/
> > > - if (pg->pg_flags & PQ_ACTIVE) {
> > > - TAILQ_REMOVE(_active, pg, pageq);
> > > - flags_to_clear |= PQ_ACTIVE;
> > > - uvmexp.active--;
> > > - }
> > > - if (pg->pg_flags & PQ_INACTIVE) {
> > > - TAILQ_REMOVE(_inactive, pg, pageq);
> > > - flags_to_clear |= PQ_INACTIVE;
> > > - uvmexp.inactive--;
> > > - }
> > > + uvm_pagedequeue(pg);
> > >
> > > /*
> > >* if the page was wired, unwire it now.
> > > @@ -1243,16 +1234,7 @@ uvm_pagewire(struct vm_page *pg)
> > > MUTEX_ASSERT_LOCKED();
> > >
> > > if (pg->wire_count == 0) {
> > > - if (pg->pg_flags & PQ_ACTIVE) {
> > > - TAILQ_REMOVE(_active, pg, pageq);
> > > - atomic_clearbits_int(>pg_flags, PQ_ACTIVE);
> > > - uvmexp.active--;
> > > - }
> > > - if (pg->pg_flags & PQ_INACTIVE) {
> > > - TAILQ_REMOVE(_inactive, pg, pageq);
> > > - atomic_clearbits_int(>pg_flags, PQ_INACTIVE);
> > > - uvmexp.inactive--;
> > > - }
> > > + uvm_pagedequeue(pg);
> > > uvmexp.wired++;
> > > }
> > > pg->wire_count++;
> > > @@ -1324,28 +1306,32 @@ uvm_pageactivate(struct vm_page *pg)
> > > KASSERT(uvm_page_owner_locked_p(pg));
> > > MUTEX_ASSERT_LOCKED();
> > >
> > > + uvm_pagedequeue(pg);
> > > + if (pg->wire_count == 0) {
> > > + TAILQ_INSERT_TAIL(_active, pg, pageq);
> > > + atomic_setbits_int(>pg_flags, PQ_ACTIVE);
> > > + uvmexp.active++;
> > > +
> > > + }
> > > +}
> > > +
> > > +/*
> > > + * uvm_pagedequeue: remove a page from any paging queue
> > > + */
> > > +void
> > > +uvm_pagedequeue(struct vm_page *pg)
> > > +{
> > > + if (pg->pg_flags & PQ_ACTIVE) {
> > > + TAILQ_REMOVE(_active, pg, pageq);
> > > + atomic_clearbits_int(>pg_flags, PQ_ACTIVE);
> > > + uvmexp.active--;
> > > + }
> > > if (pg->pg_flags & PQ_INACTIVE) {
> > > TAILQ_REMOVE(_inactive, pg, pageq);
> > > atomic_clearbits_int(>pg_flags, PQ_INACTIVE);
> > > uvmexp.inactive--;
> > > }
> > > - if (pg->wire_count == 0) {
> > > - /*
> > > - * if page is already active, remove it from list so we
> > > - * can put it at tail. if it wasn't active, then mark
> > > - * it active and bump active count
> > > - */
> > > - if (pg->pg_flags & PQ_ACTIVE)
> > > - TAILQ_REMOVE(_active, pg, pageq);
> > > - else {
> > > - atomic_setbits_int(>pg_flags, PQ_ACTIVE);
> > > - uvmexp.active++;
> > > - }
> > > -
> > > - TAILQ_INSERT_TAIL(_active, pg, pageq);
> > > - }
> > > }
> > > -
> > > /*
> > > * uvm_pagezero: zero fill a page
> > > */
> > > Index: uvm/uvm_page.h
> > > ===
> > > RCS file: /cvs/src/sys/uvm/uvm_page.h,v
> > > retrieving revision 1.67
> > > diff -u -p -r1.67 uvm_page.h
> > > --- uvm/uvm_page.h29 Jan 2022 06:25:33 - 1.67
> > > +++ uvm/uvm_page.h5 May 2022 12:49:13 -
> > > @@ -224,6 +224,7 @@ boolean_t uvm_page_physget(paddr_t *);
> > > #endif
> > >
> > > void uvm_pageactivate(struct vm_page *);
> > > +void uvm_pagedequeue(struct vm_page *);
> > > vaddr_t uvm_pageboot_alloc(vsize_t);
> > > void uvm_pagecopy(struct vm_page *, struct vm_page *);
> > > void uvm_pagedeactivate(struct vm_page *);
> > >
> >
> >
Re: changelist: add /etc/login.conf.d/*
On Thu, May 12, 2022 at 11:58:22AM BST, Stuart Henderson wrote: > changelist already has /etc/login.conf, but I think files in the .d > directory should be checked too, both so we have notification of changes > (as it can set environment variables this is a very powerful file), and > also so we keep old versions in /var/backup. The directory itself should probably also go into /etc/mtree/special. Regards, Raf Index: etc/mtree/special === RCS file: /cvs/src/etc/mtree/special,v retrieving revision 1.127 diff -u -p -r1.127 special --- etc/mtree/special 13 Sep 2020 10:03:46 - 1.127 +++ etc/mtree/special 12 May 2022 11:30:54 - @@ -46,6 +46,8 @@ isakmpd.policytype=file mode=0600 uname ldapd.conf type=file mode=0600 uname=root gname=wheel optional ldpd.conf type=file mode=0600 uname=root gname=wheel optional login.conf type=file mode=0644 uname=root gname=wheel +login.conf.d type=dir mode=0755 uname=root gname=wheel +.. #login.conf.d login_ldap.conftype=file mode=0640 uname=root gname=auth optional mail.rctype=file mode=0644 uname=root gname=wheel mailer.conftype=file mode=0644 uname=root gname=wheel
changelist: add /etc/login.conf.d/*
changelist already has /etc/login.conf, but I think files in the .d directory should be checked too, both so we have notification of changes (as it can set environment variables this is a very powerful file), and also so we keep old versions in /var/backup. ok? Index: changelist === RCS file: /cvs/src/etc/changelist,v retrieving revision 1.130 diff -u -p -r1.130 changelist --- changelist 11 Nov 2021 09:38:14 - 1.130 +++ changelist 12 May 2022 10:55:18 - @@ -62,6 +62,7 @@ /etc/ldpd.conf /etc/locate.rc /etc/login.conf +/etc/login.conf.d/* /etc/login_ldap.conf /etc/mail.rc /etc/mail/aliases
Re: Unlock umask(2)
On Wed, May 11, 2022 at 11:41:18PM +0300, Vitaliy Makkoveev wrote:
> > Both of the resizes should happen first, in case there is fallout in
> > userland which isn't visible yet.
>
> No problem.
OK bluhm@
> Index: sys/sys/filedesc.h
> ===
> RCS file: /cvs/src/sys/sys/filedesc.h,v
> retrieving revision 1.45
> diff -u -p -r1.45 filedesc.h
> --- sys/sys/filedesc.h4 Jul 2020 08:06:08 - 1.45
> +++ sys/sys/filedesc.h11 May 2022 20:14:48 -
> @@ -79,8 +79,8 @@ struct filedesc {
> u_int *fd_lomap; /* [f] bitmap of free fds */
> int fd_lastfile;/* [f] high-water mark of fd_ofiles */
> int fd_freefile;/* [f] approx. next free file */
> - u_short fd_cmask; /* [f/w] mask for file creation */
> - u_short fd_refcnt; /* [K] reference count */
> + mode_t fd_cmask; /* [f/w] mask for file creation */
> + u_int fd_refcnt; /* [K] reference count */
> struct rwlock fd_lock; /* lock for the file descs */
> struct mutex fd_fplock; /* lock for reading fd_ofiles without
>* fd_lock */
Re: rpki-client: fewer reallocarrays() for IPAddrBlocks
On Thu, May 12, 2022 at 11:27:21AM +0200, Theo Buehler wrote:
> This aligns sbgp_ipaddrblk() with sbgp_assysnum(), giving it a similar
> treatment. We trade the reallocarray() per prefix or range with at most
> two recallocarray(). I took the liberty of trimming some RFC section
> numbers in warnings to avoid awkward line wrapping.
Looks good to me. Ok claudio@
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 cert.c
> --- cert.c12 May 2022 08:53:33 - 1.80
> +++ cert.c12 May 2022 09:13:15 -
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: cert.c,v 1.80 2022/05/12 08:53:33 tb Exp $ */
> +/* $OpenBSD: cert.c,v 1.79 2022/05/12 07:45:27 claudio Exp $ */
> /*
> * Copyright (c) 2021 Job Snijders
> * Copyright (c) 2019 Kristaps Dzonsons
> @@ -60,17 +60,9 @@ extern ASN1_OBJECT *notify_oid;/* 1.3.6
> static int
> append_ip(struct parse *p, const struct cert_ip *ip)
> {
> - struct cert *res = p->res;
> -
> if (!ip_addr_check_overlap(ip, p->fn, p->res->ips, p->res->ipsz))
> return 0;
> - if (res->ipsz >= MAX_IP_SIZE)
> - return 0;
> - res->ips = reallocarray(res->ips, res->ipsz + 1,
> - sizeof(struct cert_ip));
> - if (res->ips == NULL)
> - err(1, NULL);
> - res->ips[res->ipsz++] = *ip;
> + p->res->ips[p->res->ipsz++] = *ip;
> return 1;
> }
>
> @@ -330,84 +322,6 @@ sbgp_addr_inherit(struct parse *p, enum
> }
>
> /*
> - * Parse an IP address or range, RFC 3779 2.2.3.7.
> - * We don't constrain this parse (as specified in section 2.2.3.6) to
> - * having any kind of order.
> - * Returns zero on failure, non-zero on success.
> - */
> -static int
> -sbgp_addr_or_range(struct parse *p, enum afi afi, const IPAddressOrRanges
> *aors)
> -{
> - const IPAddressOrRange *aor;
> - int i, rc = 0;
> -
> - for (i = 0; i < sk_IPAddressOrRange_num(aors); i++) {
> - aor = sk_IPAddressOrRange_value(aors, i);
> - switch (aor->type) {
> - case IPAddressOrRange_addressPrefix:
> - if (!sbgp_addr(p, afi, aor->u.addressPrefix))
> - goto out;
> - break;
> - case IPAddressOrRange_addressRange:
> - if (!sbgp_addr_range(p, afi, aor->u.addressRange))
> - goto out;
> - break;
> - default:
> - warnx("%s: RFC 3779 section 2.2.3.7: IPAddressOrRange: "
> - "unknown type %d", p->fn, aor->type);
> - goto out;
> - }
> - }
> -
> - rc = 1;
> - out:
> - return rc;
> -}
> -
> -/*
> - * Parse a sequence of address families as in RFC 3779 sec. 2.2.3.2.
> - * Ignore several stipulations of the RFC (2.2.3.3).
> - * Namely, we don't require entries to be ordered in any way (type, AFI
> - * or SAFI group, etc.).
> - * This is because it doesn't matter for our purposes: we're going to
> - * validate in the same way regardless.
> - * Returns zero on failure, non-zero on success.
> - */
> -static int
> -sbgp_ipaddrfam(struct parse *p, const IPAddressFamily *af)
> -{
> - enum afi afi;
> - const IPAddressChoice *choice;
> - int rc = 0;
> -
> - if (!ip_addr_afi_parse(p->fn, af->addressFamily, )) {
> - warnx("%s: RFC 3779 section 2.2.3.2: addressFamily: "
> - "invalid AFI", p->fn);
> - goto out;
> - }
> -
> - choice = af->ipAddressChoice;
> - switch (choice->type) {
> - case IPAddressChoice_addressesOrRanges:
> - if (!sbgp_addr_or_range(p, afi, choice->u.addressesOrRanges))
> - goto out;
> - break;
> - case IPAddressChoice_inherit:
> - if (!sbgp_addr_inherit(p, afi))
> - goto out;
> - break;
> - default:
> - warnx("%s: RFC 3779 section 2.2.3.2: IPAddressChoice: "
> - "unknown type %d", p->fn, choice->type);
> - goto out;
> - }
> -
> - rc = 1;
> - out:
> - return rc;
> -}
> -
> -/*
> * Parse an sbgp-ipAddrBlock X509 extension, RFC 6487 4.8.10, with
> * syntax documented in RFC 3779 starting in section 2.2.
> * Returns zero on failure, non-zero on success.
> @@ -417,7 +331,11 @@ sbgp_ipaddrblk(struct parse *p, X509_EXT
> {
> STACK_OF(IPAddressFamily) *addrblk = NULL;
> const IPAddressFamily *af;
> - int i, rc = 0;
> + const IPAddressOrRanges *aors;
> + const IPAddressOrRange *aor;
> + enum afi afi;
> + size_t ipsz;
> + int i, j, rc = 0;
>
> if
rpki-client: fewer reallocarrays() for IPAddrBlocks
This aligns sbgp_ipaddrblk() with sbgp_assysnum(), giving it a similar
treatment. We trade the reallocarray() per prefix or range with at most
two recallocarray(). I took the liberty of trimming some RFC section
numbers in warnings to avoid awkward line wrapping.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.80
diff -u -p -r1.80 cert.c
--- cert.c 12 May 2022 08:53:33 - 1.80
+++ cert.c 12 May 2022 09:13:15 -
@@ -1,4 +1,4 @@
-/* $OpenBSD: cert.c,v 1.80 2022/05/12 08:53:33 tb Exp $ */
+/* $OpenBSD: cert.c,v 1.79 2022/05/12 07:45:27 claudio Exp $ */
/*
* Copyright (c) 2021 Job Snijders
* Copyright (c) 2019 Kristaps Dzonsons
@@ -60,17 +60,9 @@ extern ASN1_OBJECT *notify_oid;/* 1.3.6
static int
append_ip(struct parse *p, const struct cert_ip *ip)
{
- struct cert *res = p->res;
-
if (!ip_addr_check_overlap(ip, p->fn, p->res->ips, p->res->ipsz))
return 0;
- if (res->ipsz >= MAX_IP_SIZE)
- return 0;
- res->ips = reallocarray(res->ips, res->ipsz + 1,
- sizeof(struct cert_ip));
- if (res->ips == NULL)
- err(1, NULL);
- res->ips[res->ipsz++] = *ip;
+ p->res->ips[p->res->ipsz++] = *ip;
return 1;
}
@@ -330,84 +322,6 @@ sbgp_addr_inherit(struct parse *p, enum
}
/*
- * Parse an IP address or range, RFC 3779 2.2.3.7.
- * We don't constrain this parse (as specified in section 2.2.3.6) to
- * having any kind of order.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_addr_or_range(struct parse *p, enum afi afi, const IPAddressOrRanges
*aors)
-{
- const IPAddressOrRange *aor;
- int i, rc = 0;
-
- for (i = 0; i < sk_IPAddressOrRange_num(aors); i++) {
- aor = sk_IPAddressOrRange_value(aors, i);
- switch (aor->type) {
- case IPAddressOrRange_addressPrefix:
- if (!sbgp_addr(p, afi, aor->u.addressPrefix))
- goto out;
- break;
- case IPAddressOrRange_addressRange:
- if (!sbgp_addr_range(p, afi, aor->u.addressRange))
- goto out;
- break;
- default:
- warnx("%s: RFC 3779 section 2.2.3.7: IPAddressOrRange: "
- "unknown type %d", p->fn, aor->type);
- goto out;
- }
- }
-
- rc = 1;
- out:
- return rc;
-}
-
-/*
- * Parse a sequence of address families as in RFC 3779 sec. 2.2.3.2.
- * Ignore several stipulations of the RFC (2.2.3.3).
- * Namely, we don't require entries to be ordered in any way (type, AFI
- * or SAFI group, etc.).
- * This is because it doesn't matter for our purposes: we're going to
- * validate in the same way regardless.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_ipaddrfam(struct parse *p, const IPAddressFamily *af)
-{
- enum afi afi;
- const IPAddressChoice *choice;
- int rc = 0;
-
- if (!ip_addr_afi_parse(p->fn, af->addressFamily, )) {
- warnx("%s: RFC 3779 section 2.2.3.2: addressFamily: "
- "invalid AFI", p->fn);
- goto out;
- }
-
- choice = af->ipAddressChoice;
- switch (choice->type) {
- case IPAddressChoice_addressesOrRanges:
- if (!sbgp_addr_or_range(p, afi, choice->u.addressesOrRanges))
- goto out;
- break;
- case IPAddressChoice_inherit:
- if (!sbgp_addr_inherit(p, afi))
- goto out;
- break;
- default:
- warnx("%s: RFC 3779 section 2.2.3.2: IPAddressChoice: "
- "unknown type %d", p->fn, choice->type);
- goto out;
- }
-
- rc = 1;
- out:
- return rc;
-}
-
-/*
* Parse an sbgp-ipAddrBlock X509 extension, RFC 6487 4.8.10, with
* syntax documented in RFC 3779 starting in section 2.2.
* Returns zero on failure, non-zero on success.
@@ -417,7 +331,11 @@ sbgp_ipaddrblk(struct parse *p, X509_EXT
{
STACK_OF(IPAddressFamily) *addrblk = NULL;
const IPAddressFamily *af;
- int i, rc = 0;
+ const IPAddressOrRanges *aors;
+ const IPAddressOrRange *aor;
+ enum afi afi;
+ size_t ipsz;
+ int i, j, rc = 0;
if (!X509_EXTENSION_get_critical(ext)) {
cryptowarnx("%s: RFC 6487 section 4.8.10: sbgp-ipAddrBlock: "
@@ -433,8 +351,58 @@ sbgp_ipaddrblk(struct parse *p, X509_EXT
for (i = 0; i < sk_IPAddressFamily_num(addrblk); i++)
Re: rpki-client: clean up ip handling in cert.c
On Thu, May 12, 2022 at 10:17:30AM +0200, Theo Buehler wrote:
> Before refactoring the IP side, let's streamline the code a little.
> Populate struct ip in the leaf functions instead of handing it through
> several layers and copying it along the way. Pass in the afi instead of
> letting struct ip carry it.
Looks good to me OK claudio@
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 cert.c
> --- cert.c12 May 2022 07:45:27 - 1.79
> +++ cert.c12 May 2022 08:09:07 -
> @@ -257,21 +257,28 @@ sbgp_assysnum(struct parse *p, X509_EXTE
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_addr(struct parse *p, struct cert_ip *ip, const ASN1_BIT_STRING *bs)
> +sbgp_addr(struct parse *p, enum afi afi, const ASN1_BIT_STRING *bs)
> {
> - if (!ip_addr_parse(bs, ip->afi, p->fn, >ip)) {
> + struct cert_ip ip;
> +
> + memset(, 0, sizeof(struct cert_ip));
> +
> + ip.afi = afi;
> + ip.type = CERT_IP_ADDR;
> +
> + if (!ip_addr_parse(bs, afi, p->fn, )) {
> warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
> "invalid IP address", p->fn);
> return 0;
> }
>
> - if (!ip_cert_compose_ranges(ip)) {
> + if (!ip_cert_compose_ranges()) {
> warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
> "IP address range reversed", p->fn);
> return 0;
> }
>
> - return append_ip(p, ip);
> + return append_ip(p, );
> }
>
> /*
> @@ -279,28 +286,47 @@ sbgp_addr(struct parse *p, struct cert_i
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_addr_range(struct parse *p, struct cert_ip *ip,
> -const IPAddressRange *range)
> +sbgp_addr_range(struct parse *p, enum afi afi, const IPAddressRange *range)
> {
> - if (!ip_addr_parse(range->min, ip->afi, p->fn, >range.min)) {
> + struct cert_ip ip;
> +
> + memset(, 0, sizeof(struct cert_ip));
> +
> + ip.afi = afi;
> + ip.type = CERT_IP_RANGE;
> +
> + if (!ip_addr_parse(range->min, afi, p->fn, )) {
> warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> "invalid IP address", p->fn);
> return 0;
> }
>
> - if (!ip_addr_parse(range->max, ip->afi, p->fn, >range.max)) {
> + if (!ip_addr_parse(range->max, afi, p->fn, )) {
> warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> "invalid IP address", p->fn);
> return 0;
> }
>
> - if (!ip_cert_compose_ranges(ip)) {
> + if (!ip_cert_compose_ranges()) {
> warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
> "IP address range reversed", p->fn);
> return 0;
> }
>
> - return append_ip(p, ip);
> + return append_ip(p, );
> +}
> +
> +static int
> +sbgp_addr_inherit(struct parse *p, enum afi afi)
> +{
> + struct cert_ip ip;
> +
> + memset(, 0, sizeof(struct cert_ip));
> +
> + ip.afi = afi;
> + ip.type = CERT_IP_INHERIT;
> +
> + return append_ip(p, );
> }
>
> /*
> @@ -310,25 +336,20 @@ sbgp_addr_range(struct parse *p, struct
> * Returns zero on failure, non-zero on success.
> */
> static int
> -sbgp_addr_or_range(struct parse *p, struct cert_ip *ip,
> -const IPAddressOrRanges *aors)
> +sbgp_addr_or_range(struct parse *p, enum afi afi, const IPAddressOrRanges
> *aors)
> {
> - struct cert_ip nip;
> const IPAddressOrRange *aor;
> int i, rc = 0;
>
> for (i = 0; i < sk_IPAddressOrRange_num(aors); i++) {
> - nip = *ip;
> aor = sk_IPAddressOrRange_value(aors, i);
> switch (aor->type) {
> case IPAddressOrRange_addressPrefix:
> - nip.type = CERT_IP_ADDR;
> - if (!sbgp_addr(p, , aor->u.addressPrefix))
> + if (!sbgp_addr(p, afi, aor->u.addressPrefix))
> goto out;
> break;
> case IPAddressOrRange_addressRange:
> - nip.type = CERT_IP_RANGE;
> - if (!sbgp_addr_range(p, , aor->u.addressRange))
> + if (!sbgp_addr_range(p, afi, aor->u.addressRange))
> goto out;
> break;
> default:
> @@ -355,13 +376,11 @@ sbgp_addr_or_range(struct parse *p, stru
> static int
> sbgp_ipaddrfam(struct parse *p, const IPAddressFamily *af)
> {
> - struct cert_ip ip;
> + enum afi afi;
> const IPAddressChoice *choice;
> int rc = 0;
>
> - memset(, 0, sizeof(struct cert_ip));
> -
> - if (!ip_addr_afi_parse(p->fn, af->addressFamily, )) {
> + if
rpki-client: clean up ip handling in cert.c
Before refactoring the IP side, let's streamline the code a little.
Populate struct ip in the leaf functions instead of handing it through
several layers and copying it along the way. Pass in the afi instead of
letting struct ip carry it.
Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.79
diff -u -p -r1.79 cert.c
--- cert.c 12 May 2022 07:45:27 - 1.79
+++ cert.c 12 May 2022 08:09:07 -
@@ -257,21 +257,28 @@ sbgp_assysnum(struct parse *p, X509_EXTE
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_addr(struct parse *p, struct cert_ip *ip, const ASN1_BIT_STRING *bs)
+sbgp_addr(struct parse *p, enum afi afi, const ASN1_BIT_STRING *bs)
{
- if (!ip_addr_parse(bs, ip->afi, p->fn, >ip)) {
+ struct cert_ip ip;
+
+ memset(, 0, sizeof(struct cert_ip));
+
+ ip.afi = afi;
+ ip.type = CERT_IP_ADDR;
+
+ if (!ip_addr_parse(bs, afi, p->fn, )) {
warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
"invalid IP address", p->fn);
return 0;
}
- if (!ip_cert_compose_ranges(ip)) {
+ if (!ip_cert_compose_ranges()) {
warnx("%s: RFC 3779 section 2.2.3.8: IPAddress: "
"IP address range reversed", p->fn);
return 0;
}
- return append_ip(p, ip);
+ return append_ip(p, );
}
/*
@@ -279,28 +286,47 @@ sbgp_addr(struct parse *p, struct cert_i
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_addr_range(struct parse *p, struct cert_ip *ip,
-const IPAddressRange *range)
+sbgp_addr_range(struct parse *p, enum afi afi, const IPAddressRange *range)
{
- if (!ip_addr_parse(range->min, ip->afi, p->fn, >range.min)) {
+ struct cert_ip ip;
+
+ memset(, 0, sizeof(struct cert_ip));
+
+ ip.afi = afi;
+ ip.type = CERT_IP_RANGE;
+
+ if (!ip_addr_parse(range->min, afi, p->fn, )) {
warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
"invalid IP address", p->fn);
return 0;
}
- if (!ip_addr_parse(range->max, ip->afi, p->fn, >range.max)) {
+ if (!ip_addr_parse(range->max, afi, p->fn, )) {
warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
"invalid IP address", p->fn);
return 0;
}
- if (!ip_cert_compose_ranges(ip)) {
+ if (!ip_cert_compose_ranges()) {
warnx("%s: RFC 3779 section 2.2.3.9: IPAddressRange: "
"IP address range reversed", p->fn);
return 0;
}
- return append_ip(p, ip);
+ return append_ip(p, );
+}
+
+static int
+sbgp_addr_inherit(struct parse *p, enum afi afi)
+{
+ struct cert_ip ip;
+
+ memset(, 0, sizeof(struct cert_ip));
+
+ ip.afi = afi;
+ ip.type = CERT_IP_INHERIT;
+
+ return append_ip(p, );
}
/*
@@ -310,25 +336,20 @@ sbgp_addr_range(struct parse *p, struct
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_addr_or_range(struct parse *p, struct cert_ip *ip,
-const IPAddressOrRanges *aors)
+sbgp_addr_or_range(struct parse *p, enum afi afi, const IPAddressOrRanges
*aors)
{
- struct cert_ip nip;
const IPAddressOrRange *aor;
int i, rc = 0;
for (i = 0; i < sk_IPAddressOrRange_num(aors); i++) {
- nip = *ip;
aor = sk_IPAddressOrRange_value(aors, i);
switch (aor->type) {
case IPAddressOrRange_addressPrefix:
- nip.type = CERT_IP_ADDR;
- if (!sbgp_addr(p, , aor->u.addressPrefix))
+ if (!sbgp_addr(p, afi, aor->u.addressPrefix))
goto out;
break;
case IPAddressOrRange_addressRange:
- nip.type = CERT_IP_RANGE;
- if (!sbgp_addr_range(p, , aor->u.addressRange))
+ if (!sbgp_addr_range(p, afi, aor->u.addressRange))
goto out;
break;
default:
@@ -355,13 +376,11 @@ sbgp_addr_or_range(struct parse *p, stru
static int
sbgp_ipaddrfam(struct parse *p, const IPAddressFamily *af)
{
- struct cert_ip ip;
+ enum afi afi;
const IPAddressChoice *choice;
int rc = 0;
- memset(, 0, sizeof(struct cert_ip));
-
- if (!ip_addr_afi_parse(p->fn, af->addressFamily, )) {
+ if (!ip_addr_afi_parse(p->fn, af->addressFamily, )) {
warnx("%s: RFC 3779 section 2.2.3.2: addressFamily: "
"invalid AFI", p->fn);
goto out;
@@ -370,12 +389,11 @@ sbgp_ipaddrfam(struct parse *p,
