Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
Re: bgpd: rework pfkey (ipsec/tcpmd5) code
To:
tech@openbsd.org
Date:
Tue, 1 Oct 2024 13:40:11 +0200

Download raw body.

Thread
On Tue, Oct 01, 2024 at 01:21:06PM +0200, Claudio Jeker wrote:
> On Tue, Oct 01, 2024 at 11:23:30AM +0200, Theo Buehler wrote:
> > On Fri, Sep 27, 2024 at 05:14:04PM +0200, Claudio Jeker wrote:
> > > This diff reworks the pfkey code to handle ipsec and tcpmd5 to be
> > > independent of struct peer. Instead use struct auth_config and struct
> > > auth_state. Make sure the auth_config (which includes the secrets, remains
> > > in the parent (apart from -portable where Linux needs the md5 keys in the
> > > SE to work)). Only the auth_config method is shared with the SE since it
> > > is needed to turn on the TCP_MD5SIG socketopts.
> > > By spliting the auth_config out of peer_config the sharing can be better
> > > controlled. To be extra sure adjust the control_imsg_relay() function to
> > > double clear the buffer so it is not sent to any bgpctl tool.
> > 
> > I assume that you use memset() instead of explicit_bzero() because of
> > portable?
> 
> No, this code should probably use explicit_bzero(). I changed the memset()
> in control.c. That's the only one right?

I think that's correct. I'd use explicit_bzero() for all the memsets()
in this diff to be on the safe side, though.

> AFAIK -portable already includes explicit_bzero().

Ah yes.

> 
> > pfkey_remove() has an if (as->established) check at the start but all the
> > callers in pfkey.c check that beforehand. So these callers could be simplified.
> 
> I think you're right but I prefer to do this in a 2nd step.

Sure.