[Qemu-devel] [PATCH] iotests: Avoid SIGPIPE death to certtool

Eric Blake posted 1 patch 5 years, 2 months ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190219160920.32705-1-eblake@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
tests/qemu-iotests/common.tls | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] iotests: Avoid SIGPIPE death to certtool
Posted by Eric Blake 5 years, 2 months ago
Our use of 'head -1' to log less output of certtool during
iotest 233 could result in certtool dying early due to SIGPIPE
if it generates enough output; if that happens, the certificate
it was supposed to generate may be zero length, which causes
failures such as:

  == check TLS client to plain server fails ==
 -qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': Denied by server for option 5 (starttls)
 -server reported: TLS not configured
 -qemu-nbd: Denied by server for option 5 (starttls)
 -server reported: TLS not configured
 +qemu-nbd: Unable to import client certificate /tmp/qemu-iotests-quick-28354/tls/client1/client-cert.pem: Base64 unexpected header error.

Fix the pipelines to consume all output.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/qemu-iotests/common.tls | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
index eae81789bbc..15c6e8c8425 100644
--- a/tests/qemu-iotests/common.tls
+++ b/tests/qemu-iotests/common.tls
@@ -74,7 +74,7 @@ EOF
     certtool --generate-self-signed \
              --load-privkey "${tls_dir}/key.pem" \
              --template "${tls_dir}/ca.info" \
-             --outfile "${tls_dir}/$name-cert.pem" 2>&1 | head -1
+             --outfile "${tls_dir}/$name-cert.pem" 2>&1 | sed -n 1p

     rm -f "${tls_dir}/ca.info"
 }
@@ -103,7 +103,7 @@ EOF
              --load-ca-certificate "${tls_dir}/$caname-cert.pem" \
              --load-privkey "${tls_dir}/key.pem" \
              --template "${tls_dir}/cert.info" \
-             --outfile "${tls_dir}/$name/server-cert.pem" 2>&1 | head -1
+             --outfile "${tls_dir}/$name/server-cert.pem" 2>&1 | sed -n 1p
     ln -s "${tls_dir}/$caname-cert.pem" "${tls_dir}/$name/ca-cert.pem"
     ln -s "${tls_dir}/key.pem" "${tls_dir}/$name/server-key.pem"

@@ -132,7 +132,7 @@ EOF
              --load-ca-certificate "${tls_dir}/$caname-cert.pem" \
              --load-privkey "${tls_dir}/key.pem" \
              --template "${tls_dir}/cert.info" \
-             --outfile "${tls_dir}/$name/client-cert.pem" 2>&1 | head -1
+             --outfile "${tls_dir}/$name/client-cert.pem" 2>&1 | sed -n 1p
     ln -s "${tls_dir}/$caname-cert.pem" "${tls_dir}/$name/ca-cert.pem"
     ln -s "${tls_dir}/key.pem" "${tls_dir}/$name/client-key.pem"

-- 
2.20.1


Re: [Qemu-devel] [PATCH] iotests: Avoid SIGPIPE death to certtool
Posted by Daniel P. Berrangé 5 years, 2 months ago
On Tue, Feb 19, 2019 at 10:09:20AM -0600, Eric Blake wrote:
> Our use of 'head -1' to log less output of certtool during
> iotest 233 could result in certtool dying early due to SIGPIPE
> if it generates enough output; if that happens, the certificate
> it was supposed to generate may be zero length, which causes
> failures such as:
> 
>   == check TLS client to plain server fails ==
>  -qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': Denied by server for option 5 (starttls)
>  -server reported: TLS not configured
>  -qemu-nbd: Denied by server for option 5 (starttls)
>  -server reported: TLS not configured
>  +qemu-nbd: Unable to import client certificate /tmp/qemu-iotests-quick-28354/tls/client1/client-cert.pem: Base64 unexpected header error.
> 
> Fix the pipelines to consume all output.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qemu-iotests/common.tls | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
> index eae81789bbc..15c6e8c8425 100644
> --- a/tests/qemu-iotests/common.tls
> +++ b/tests/qemu-iotests/common.tls
> @@ -74,7 +74,7 @@ EOF
>      certtool --generate-self-signed \
>               --load-privkey "${tls_dir}/key.pem" \
>               --template "${tls_dir}/ca.info" \
> -             --outfile "${tls_dir}/$name-cert.pem" 2>&1 | head -1
> +             --outfile "${tls_dir}/$name-cert.pem" 2>&1 | sed -n 1p

I've actually prepared a patch which gets rid of the pipe entirely,
because in debugging this problem I found it unhelpful that we culled
stderr when certtool failed.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] iotests: Avoid SIGPIPE death to certtool
Posted by Thomas Huth 5 years, 2 months ago
On 19/02/2019 17.09, Eric Blake wrote:
> Our use of 'head -1' to log less output of certtool during
> iotest 233 could result in certtool dying early due to SIGPIPE
> if it generates enough output; if that happens, the certificate
> it was supposed to generate may be zero length, which causes
> failures such as:
> 
>   == check TLS client to plain server fails ==
>  -qemu-img: Could not open 'driver=nbd,host=127.0.0.1,port=PORT,tls-creds=tls0': Denied by server for option 5 (starttls)
>  -server reported: TLS not configured
>  -qemu-nbd: Denied by server for option 5 (starttls)
>  -server reported: TLS not configured
>  +qemu-nbd: Unable to import client certificate /tmp/qemu-iotests-quick-28354/tls/client1/client-cert.pem: Base64 unexpected header error.
> 
> Fix the pipelines to consume all output.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qemu-iotests/common.tls | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls
> index eae81789bbc..15c6e8c8425 100644
> --- a/tests/qemu-iotests/common.tls
> +++ b/tests/qemu-iotests/common.tls
> @@ -74,7 +74,7 @@ EOF
>      certtool --generate-self-signed \
>               --load-privkey "${tls_dir}/key.pem" \
>               --template "${tls_dir}/ca.info" \
> -             --outfile "${tls_dir}/$name-cert.pem" 2>&1 | head -1
> +             --outfile "${tls_dir}/$name-cert.pem" 2>&1 | sed -n 1p
> 
>      rm -f "${tls_dir}/ca.info"
>  }
> @@ -103,7 +103,7 @@ EOF
>               --load-ca-certificate "${tls_dir}/$caname-cert.pem" \
>               --load-privkey "${tls_dir}/key.pem" \
>               --template "${tls_dir}/cert.info" \
> -             --outfile "${tls_dir}/$name/server-cert.pem" 2>&1 | head -1
> +             --outfile "${tls_dir}/$name/server-cert.pem" 2>&1 | sed -n 1p
>      ln -s "${tls_dir}/$caname-cert.pem" "${tls_dir}/$name/ca-cert.pem"
>      ln -s "${tls_dir}/key.pem" "${tls_dir}/$name/server-key.pem"
> 
> @@ -132,7 +132,7 @@ EOF
>               --load-ca-certificate "${tls_dir}/$caname-cert.pem" \
>               --load-privkey "${tls_dir}/key.pem" \
>               --template "${tls_dir}/cert.info" \
> -             --outfile "${tls_dir}/$name/client-cert.pem" 2>&1 | head -1
> +             --outfile "${tls_dir}/$name/client-cert.pem" 2>&1 | sed -n 1p
>      ln -s "${tls_dir}/$caname-cert.pem" "${tls_dir}/$name/ca-cert.pem"
>      ln -s "${tls_dir}/key.pem" "${tls_dir}/$name/client-key.pem"
> 

Thanks, this fixes the issue for me!

Tested-by: Thomas Huth <thuth@redhat.com>