subject:"rpki\-client refactor tal handling"

Re: rpki-client refactor tal handling

2021-11-04 Thread Theo Buehler

On Wed, Nov 03, 2021 at 08:01:38PM +0100, Claudio Jeker wrote:
> On Wed, Nov 03, 2021 at 12:41:52PM -0600, Theo de Raadt wrote:
> > +   size_t   talid; /* covered by which TAL */
> > 
> > You shouldn't use size_t
> > 
> > It is 32bit on ILP32 systems, and 64bit on I32LP64 machines, because the
> > underlying definition is:
> > 
> > _types.h:typedefunsigned long   __size_t;
> > 
> > So suspect you want to use int or u_int.
> 
> Other code uses size_t because the ids are used as index in arrays.
> It is overkill here the maximum number of TAL is 8 right now. That fits in
> any kind of int. 
> 
> Here an adjusted diff

ok tb

Re: rpki-client refactor tal handling

2021-11-03 Thread Claudio Jeker

On Wed, Nov 03, 2021 at 12:41:52PM -0600, Theo de Raadt wrote:
> +   size_t   talid; /* covered by which TAL */
> 
> You shouldn't use size_t
> 
> It is 32bit on ILP32 systems, and 64bit on I32LP64 machines, because the
> underlying definition is:
> 
> _types.h:typedefunsigned long   __size_t;
> 
> So suspect you want to use int or u_int.

Other code uses size_t because the ids are used as index in arrays.
It is overkill here the maximum number of TAL is 8 right now. That fits in
any kind of int. 

Here an adjusted diff
-- 
:wq Claudio

Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.45
diff -u -p -r1.45 cert.c
--- cert.c  2 Nov 2021 19:30:30 -   1.45
+++ cert.c  3 Nov 2021 18:52:55 -
@@ -1220,7 +1220,6 @@ cert_free(struct cert *p)
free(p->aia);
free(p->aki);
free(p->ski);
-   free(p->tal);
free(p->pubkey);
X509_free(p->x509);
free(p);
@@ -1263,13 +1262,14 @@ cert_buffer(struct ibuf *b, const struct
 {
size_t   i;
 
-   io_simple_buffer(b, >expires, sizeof(time_t));
-   io_simple_buffer(b, >purpose, sizeof(enum cert_purpose));
-   io_simple_buffer(b, >ipsz, sizeof(size_t));
+   io_simple_buffer(b, >expires, sizeof(p->expires));
+   io_simple_buffer(b, >purpose, sizeof(p->purpose));
+   io_simple_buffer(b, >talid, sizeof(p->talid));
+   io_simple_buffer(b, >ipsz, sizeof(p->ipsz));
for (i = 0; i < p->ipsz; i++)
cert_ip_buffer(b, >ips[i]);
 
-   io_simple_buffer(b, >asz, sizeof(size_t));
+   io_simple_buffer(b, >asz, sizeof(p->asz));
for (i = 0; i < p->asz; i++)
cert_as_buffer(b, >as[i]);
io_str_buffer(b, p->mft);
@@ -1279,7 +1279,6 @@ cert_buffer(struct ibuf *b, const struct
io_str_buffer(b, p->aia);
io_str_buffer(b, p->aki);
io_str_buffer(b, p->ski);
-   io_str_buffer(b, p->tal);
io_str_buffer(b, p->pubkey);
 }
 
@@ -1325,9 +1324,10 @@ cert_read(struct ibuf *b)
if ((p = calloc(1, sizeof(struct cert))) == NULL)
err(1, NULL);
 
-   io_read_buf(b, >expires, sizeof(time_t));
-   io_read_buf(b, >purpose, sizeof(enum cert_purpose));
-   io_read_buf(b, >ipsz, sizeof(size_t));
+   io_read_buf(b, >expires, sizeof(p->expires));
+   io_read_buf(b, >purpose, sizeof(p->purpose));
+   io_read_buf(b, >talid, sizeof(p->talid));
+   io_read_buf(b, >ipsz, sizeof(p->ipsz));
 
p->ips = calloc(p->ipsz, sizeof(struct cert_ip));
if (p->ips == NULL)
@@ -1335,7 +1335,7 @@ cert_read(struct ibuf *b)
for (i = 0; i < p->ipsz; i++)
cert_ip_read(b, >ips[i]);
 
-   io_read_buf(b, >asz, sizeof(size_t));
+   io_read_buf(b, >asz, sizeof(p->asz));
p->as = calloc(p->asz, sizeof(struct cert_as));
if (p->as == NULL)
err(1, NULL);
@@ -1349,7 +1349,6 @@ cert_read(struct ibuf *b)
io_read_str(b, >aia);
io_read_str(b, >aki);
io_read_str(b, >ski);
-   io_read_str(b, >tal);
io_read_str(b, >pubkey);
 
assert(p->mft != NULL || p->purpose == CERT_PURPOSE_BGPSEC_ROUTER);
@@ -1406,8 +1405,7 @@ insert_brk(struct brk_tree *tree, struct
 
b->asid = asid;
b->expires = cert->expires;
-   if ((b->tal = strdup(cert->tal)) == NULL)
-   err(1, NULL);
+   b->talid = cert->talid;
if ((b->ski = strdup(cert->ski)) == NULL)
err(1, NULL);
if ((b->pubkey = strdup(cert->pubkey)) == NULL)
@@ -1420,13 +1418,10 @@ insert_brk(struct brk_tree *tree, struct
if ((found = RB_INSERT(brk_tree, tree, b)) != NULL) {
if (found->expires < b->expires) {
found->expires = b->expires;
-   free(found->tal);
-   found->tal = b->tal;
-   b->tal = NULL;
+   found->talid = b->talid;
}
free(b->ski);
free(b->pubkey);
-   free(b->tal);
free(b);
}
 }
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.89
diff -u -p -r1.89 extern.h
--- extern.h3 Nov 2021 10:50:18 -   1.89
+++ extern.h3 Nov 2021 18:51:02 -
@@ -118,6 +118,7 @@ struct cert {
size_t   ipsz; /* length of "ips" */
struct cert_as  *as; /* list of AS numbers and ranges */
size_t   asz; /* length of "asz" */
+   int  talid; /* cert is covered by which TAL */
char*repo; /* CA repository (rsync:// uri) */
char*mft; /* manifest (rsync:// uri) */
char*notify; /* RRDP notify (https:// uri) */
@@ -125,8 +126,7 @@ struct cert {

Re: rpki-client refactor tal handling

2021-11-03 Thread Theo de Raadt

+   size_t   talid; /* covered by which TAL */

You shouldn't use size_t

It is 32bit on ILP32 systems, and 64bit on I32LP64 machines, because the
underlying definition is:

_types.h:typedefunsigned long   __size_t;

So suspect you want to use int or u_int.

rpki-client refactor tal handling

2021-11-03 Thread Claudio Jeker

This diff changes how the certs and roa track the tal that covers them.
Instead of passing strings around use ids and a simple lookup table
for the description. This will make it possible to add tal ids to more
things.

Usual test run works and the output for openbgpd and json look sane.
-- 
:wq Claudio

Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.45
diff -u -p -r1.45 cert.c
--- cert.c  2 Nov 2021 19:30:30 -   1.45
+++ cert.c  3 Nov 2021 17:45:36 -
@@ -1220,7 +1220,6 @@ cert_free(struct cert *p)
free(p->aia);
free(p->aki);
free(p->ski);
-   free(p->tal);
free(p->pubkey);
X509_free(p->x509);
free(p);
@@ -1265,6 +1264,7 @@ cert_buffer(struct ibuf *b, const struct
 
io_simple_buffer(b, >expires, sizeof(time_t));
io_simple_buffer(b, >purpose, sizeof(enum cert_purpose));
+   io_simple_buffer(b, >talid, sizeof(size_t));
io_simple_buffer(b, >ipsz, sizeof(size_t));
for (i = 0; i < p->ipsz; i++)
cert_ip_buffer(b, >ips[i]);
@@ -1279,7 +1279,6 @@ cert_buffer(struct ibuf *b, const struct
io_str_buffer(b, p->aia);
io_str_buffer(b, p->aki);
io_str_buffer(b, p->ski);
-   io_str_buffer(b, p->tal);
io_str_buffer(b, p->pubkey);
 }
 
@@ -1327,6 +1326,7 @@ cert_read(struct ibuf *b)
 
io_read_buf(b, >expires, sizeof(time_t));
io_read_buf(b, >purpose, sizeof(enum cert_purpose));
+   io_read_buf(b, >talid, sizeof(size_t));
io_read_buf(b, >ipsz, sizeof(size_t));
 
p->ips = calloc(p->ipsz, sizeof(struct cert_ip));
@@ -1349,7 +1349,6 @@ cert_read(struct ibuf *b)
io_read_str(b, >aia);
io_read_str(b, >aki);
io_read_str(b, >ski);
-   io_read_str(b, >tal);
io_read_str(b, >pubkey);
 
assert(p->mft != NULL || p->purpose == CERT_PURPOSE_BGPSEC_ROUTER);
@@ -1406,8 +1405,7 @@ insert_brk(struct brk_tree *tree, struct
 
b->asid = asid;
b->expires = cert->expires;
-   if ((b->tal = strdup(cert->tal)) == NULL)
-   err(1, NULL);
+   b->talid = cert->talid;
if ((b->ski = strdup(cert->ski)) == NULL)
err(1, NULL);
if ((b->pubkey = strdup(cert->pubkey)) == NULL)
@@ -1420,13 +1418,10 @@ insert_brk(struct brk_tree *tree, struct
if ((found = RB_INSERT(brk_tree, tree, b)) != NULL) {
if (found->expires < b->expires) {
found->expires = b->expires;
-   free(found->tal);
-   found->tal = b->tal;
-   b->tal = NULL;
+   found->talid = b->talid;
}
free(b->ski);
free(b->pubkey);
-   free(b->tal);
free(b);
}
 }
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.89
diff -u -p -r1.89 extern.h
--- extern.h3 Nov 2021 10:50:18 -   1.89
+++ extern.h3 Nov 2021 17:58:11 -
@@ -118,6 +118,7 @@ struct cert {
size_t   ipsz; /* length of "ips" */
struct cert_as  *as; /* list of AS numbers and ranges */
size_t   asz; /* length of "asz" */
+   size_t   talid; /* cert is covered by which TAL */
char*repo; /* CA repository (rsync:// uri) */
char*mft; /* manifest (rsync:// uri) */
char*notify; /* RRDP notify (https:// uri) */
@@ -125,8 +126,7 @@ struct cert {
char*aia; /* AIA (or NULL, for trust anchor) */
char*aki; /* AKI (or NULL, for trust anchor) */
char*ski; /* SKI */
-   char*tal; /* basename of TAL for this cert */
-   enum cert_purposepurpose; /* Certificate Purpose (BGPSec or CA) 
*/
+   enum cert_purposepurpose; /* BGPSec or CA */
char*pubkey; /* Subject Public Key Info */
X509*x509; /* the cert */
time_t   expires; /* do not use after */
@@ -145,6 +145,7 @@ struct tal {
unsigned char   *pkey; /* DER-encoded public key */
size_t   pkeysz; /* length of pkey */
char*descr; /* basename of tal file */
+   size_t   id;
 };
 
 /*
@@ -192,11 +193,11 @@ struct roa {
uint32_t asid; /* asID of ROA (if 0, RFC 6483 sec 4) */
struct roa_ip   *ips; /* IP prefixes */
size_t   ipsz; /* number of IP prefixes */
+   size_t  talid; /* ROAs are covered by which TAL */
int  valid; /* validated resources */
char*aia; /* AIA */
char*aki; /* AKI */
char*ski; /* SKI */
-   char*tal; /*

Re: rpki-client refactor tal handling

Re: rpki-client refactor tal handling

Re: rpki-client refactor tal handling

rpki-client refactor tal handling

4 matches

Site Navigation

Mail list logo

Footer information