On Mon, Nov 27, 2017 at 11:34:49PM +0100, Alexander Bluhm wrote:
> On Mon, Nov 27, 2017 at 11:26:06PM +0100, Claudio Jeker wrote:
> > Guess we should make the 3 fatalx() in that code different so that it
> > becomes more clear on which call it fails.
>
> As in the diff below?
>
> > Wonder what kind of startup race we lose...
>
> I can reproduce one of the errors on my laptop:
>
> make run-regress-args-https-inspect.pl # create test certificates
> find / >/dev/null # clear the buffer cache for a while
> make run-regress-args-https-inspect.pl # fails at the first try
>
> ======== run-regress-args-https-inspect.pl ========
> time SUDO=sudo KTRACE= RELAYD= perl -I/usr/src/regress/usr.sbin/relayd
> /usr/src/regress/usr.sbin/relayd/relayd.pl copy
> /usr/src/regress/usr.sbin/relayd/args-https-inspect.pl
> Client child status: 65280 exit: 255 at
> /usr/src/regress/usr.sbin/relayd/Proc.pm line 161.
> *** Error 255 in /usr/src/regress/usr.sbin/relayd (Makefile:68
> 'run-regress-args-https-inspect.pl')
>
> Now amd64, T430s, ssd, single machine
>
> execute: sudo relayd -dvv -f relayd.conf
> startup
> relay_load_certfiles: using ca certificate ca.crt
> pfe: filter init done
> socket_rlimit: max open files 1024
> socket_rlimit: max open files 1024
> relay_load_certfiles: using ca key ca.key
> relay_load_certfiles: using certificate /etc/ssl/127.0.0.1.crt
> relay_load_certfiles: using private key /etc/ssl/private/127.0.0.1.key
> parent_tls_ticket_rekey: rekeying tickets
> relay_privinit: adding relay relay-args-https-inspect.pl
> protocol 1: name proto-args-https-inspect.pl
> flags: used, relay flags: tls, tls client, tls inspect
> tcp flags: no splice
> tls flags: tlsv1.2, cipher-server-preference
> tls session tickets: disabled
> type: http
> match request header log "foo" value "*"
> match response header log "bar" value "*"
> ca_engine_init: using RSA privsep engine
> ca_engine_init: using RSA privsep engine
> relay_tls_ctx_create: loading certificate
> failed to load tls CA certificate: Invalid argument
> relay: relay_launch: failed to create TLS context
> pfe exiting, pid 18436
> hce exiting, pid 69282
> lost child: pid 5175 exited abnormally
> ca exiting, pid 40825
> parent terminating, pid 53770
>
> bluhm
>
> Index: ca.c
> ===================================================================
> RCS file: /mount/openbsd/cvs/src/usr.sbin/relayd/ca.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 ca.c
> --- ca.c 27 Nov 2017 21:06:25 -0000 1.29
> +++ ca.c 27 Nov 2017 21:45:59 -0000
> @@ -123,14 +123,14 @@ ca_launch(void)
> if (rlay->rl_tls_cert_fd != -1) {
> if ((buf = relay_load_fd(rlay->rl_tls_cert_fd,
> &len)) == NULL)
> - fatalx("ca_launch: cert");
> + fatalx("ca_launch: cert relay_load_fd");
>
> if ((in = BIO_new_mem_buf(buf, len)) == NULL)
> - fatalx("ca_launch: cert");
> + fatalx("ca_launch: cert BIO_new_mem_buf");
>
> if ((cert = PEM_read_bio_X509(in, NULL,
> NULL, NULL)) == NULL)
> - fatalx("ca_launch: cert");
> + fatalx("ca_launch: cert PEM_read_bio_X509");
>
> hash_x509(cert, hash, sizeof(hash));
>
> @@ -160,14 +160,14 @@ ca_launch(void)
> if (rlay->rl_tls_cacert_fd != -1) {
> if ((buf = relay_load_fd(rlay->rl_tls_cacert_fd,
> &len)) == NULL)
> - fatalx("ca_launch: cacert");
> + fatalx("ca_launch: cacert relay_load_fd");
>
> if ((in = BIO_new_mem_buf(buf, len)) == NULL)
> - fatalx("ca_launch: cacert");
> + fatalx("ca_launch: cacert BIO_new_mem_buf");
>
> if ((cert = PEM_read_bio_X509(in, NULL,
> NULL, NULL)) == NULL)
> - fatalx("ca_launch: cacert");
> + fatalx("ca_launch: cacert PEM_read_bio_X509");
>
> hash_x509(cert, hash, sizeof(hash));
>
Have the same diff in my tree. OK claudio@
The problem is a race in relay_load_fd() according to guenther@ we should
use pread instead of the lseek/ read combo. Lets see if that works.
--
:wq Claudio
