Index | Thread | Search

From:
Bob Beck <beck@obtuse.com>
Subject:
Re: isakmpd: don't reach into ASN1_STRING [step 2/2]
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org, beck@openbsd.org
Date:
Fri, 5 Dec 2025 14:05:13 -0700

Download raw body.

Thread
On Fri, Dec 05, 2025 at 08:37:51PM +0100, Theo Buehler wrote:
> On Fri, Dec 05, 2025 at 12:14:55PM -0700, Bob Beck wrote:
> > On Fri, Dec 05, 2025 at 11:07:49AM -0700, Bob Beck wrote:
> > > 
> > > 
> > > > On Nov 24, 2025, at 23:07, Theo Buehler <tb@theobuehler.org> wrote:
> > > > 
> > > > On Mon, Nov 24, 2025 at 03:31:58PM +0100, Theo Buehler wrote:
> > > >> beck and davidben have plans to make ASN1_STRING opaque in OpenSSL 4 [1].
> > > >> This breaks somewhere in the vicinity of 200 ports, so I don't think we'll
> > > >> flip the switch anytime soon in libressl, but I want base to be ready for
> > > >> that since that makes my life easier.
> > > > 
> > > > Here's the second step. This allows things that aren't really kosher in
> > > > modern X.509 like omitting seconds or allowing local times.
> > > > 
> > > > Since this transforms notBefore and notAfter into a string to be used in
> > > > a keynote assertion, existing libcrypto interfaces aren't suitable, so I
> > > > kept this mechanical by assigning the data and length fields to local
> > > > variables accessors and replacing tm->data by data and tm->length by
> > > > len, only occasionally fixing up whitespace and omitting parentheses.
> > > > 
> > > > The whole dance should be deduplicated, but I'll leave that for someone
> > > > else.
> > > 
> > > I *think* this is ok? But that code is uglier than my butthole after a week of hiking. 
> > > 
> > > I suspect a lot of the nonconformant crap mattered 20 years ago and is not used at all today so could be removed. 
> > > 
> > > But don???t do that here. 
> > 
> > I.E. I suspect this would work fine (passes regress) but I'd just land yours for now, if
> > we want to do this we put it in and ask isakmp people to test later.
> 
> Yeah, I had something similar in mind. What made me pause is that the
> x509 part of the regress tests is completely busted. As I wrote in my
> first mail in this thread:
> 
>  (In case you wonder: x509_cert_subjectaltname() isn't static because it
>  is "used" from regress/isakmpd/x509. That thing is completely broken and
>  hasn't been successfully compiled since reyk removed math_mp.h back in
>  2010, possibly much longer.)

Indeed, IMO, land your change, and if we want to contemplate somthing like
mine we should maybe consider some unit tests.