From: Theo Buehler Subject: Re: [PATCH] relayd client certificate validation again To: Sören Tempel Cc: tech@openbsd.org, markus.l2ll@gmail.com, rivo@elnit.ee, brian@planetunix.net Date: Mon, 21 Oct 2024 09:22:58 +0200 On Mon, Oct 07, 2024 at 07:03:47PM +0200, Sören Tempel wrote: > Hello Theo, > > Thanks for your detailed comments and input so far! > Please find the updated patch and additional comments below. > > Theo Buehler wrote: > > We need a diff that is tested, has regress coverage and applies to > > current. Which one it is, is your choice, but it sounds like you want > > [2] + some additions in regress. > > I decided to update the original patch by Ashe Connor, i.e. [1]. > Regarding those updates, I made the following changes: > > 1. Rebased the patch to ensure that it applies cleanly to -current. > 2. Incorporated your feedback regarding code style from [2]. > 3. Fixed the run-args-ssl-client-verify-fail.pl test case which was > failing previously as the default client func (write_char) does > not work correctly if we can't establish a connection to the socket. > Instead, the test now uses the relayd.log to check for failure/success. > > With these changes, the entire relayd regress suite now passes > successfully on my system. Furthermore, I performed some additional > manual tests as well and didn't run into issues with those either. > > Therefore, I hope the patch is now in a shape where it can be merged. > If additional changes are needed, please let me know. > > diff --git usr.sbin/relayd/relayd.conf.5 usr.sbin/relayd/relayd.conf.5 > index 50c73cbec15..771d3632398 100644 > --- usr.sbin/relayd/relayd.conf.5 > +++ usr.sbin/relayd/relayd.conf.5 > @@ -954,6 +954,10 @@ will be used (strong crypto cipher suites without anonymous DH). > See the CIPHERS section of > .Xr openssl 1 > for information about TLS cipher suites and preference lists. > +.It Ic client ca Ar path > +Require TLS client certificates whose authenticity can be verified > +against the CA certificate(s) in the specified file in order to > +proceed beyond the TLS handshake. Maybe this could be simplified to Require TLS client certificates that can be verified against the CA certificates in the specified file. Other than that the diff looks good to me and I think it should go in ok tb Or I'm happy to take an ok to commit.