[PATCH] Tests: ssl_engine_keys.t improved

Sergey Kandaurov pluknet at nginx.com
Sun May 26 21:13:58 UTC 2024


[ extraneous "Cc: PATCH, ssl_engine_keys.t" removed ]

On Tue, May 07, 2024 at 11:32:45PM +0300, o.deeva at wbsrv.ru wrote:
> # HG changeset patch
> # User Oksana Deeva <o.deeva at wbsrv.ru>
> # Date 1715111756 -10800
> #      Tue May 07 22:55:56 2024 +0300
> # Node ID e5014b423e1391dd1078d064361a0b28d1a488d0
> # Parent  2a607a31f583add7adfa1ac434a3f793d327ca6b

Hello, thanks for the patch.

> Tests: ssl_engine_keys.t improved

style: missing period

For generic commit log style, see example in
http://nginx.org/en/docs/contributing_changes.html

> 
> diff -r 2a607a31f583 -r e5014b423e13 ssl_engine_keys.t
> --- a/ssl_engine_keys.t	Tue Apr 23 17:59:53 2024 +0400
> +++ b/ssl_engine_keys.t	Tue May 07 22:55:56 2024 +0300
> @@ -28,7 +28,7 @@
>  	unless $ENV{TEST_NGINX_UNSAFE};
>  
>  my $t = Test::Nginx->new()->has(qw/http proxy http_ssl/)->has_daemon('openssl')
> -	->has_daemon('softhsm2-util')->has_daemon('pkcs11-tool')->plan(2);
> +	->has_daemon('softhsm2-util')->has_daemon('pkcs11-tool');
>  
>  $t->write_file_expand('nginx.conf', <<'EOF');
>  
> @@ -86,9 +86,29 @@
>  #
>  # http://mailman.nginx.org/pipermail/nginx-devel/2014-October/006151.html
>  #
> -# Note that library paths may differ on different systems,
> +# Note that library paths vary on different systems,
>  # and may need to be adjusted.
>  
> +my $libsofthsm2_path;
> +my @so_paths = (
> +	'/usr/lib/softhsm/',    # alpine, astrase, debian, ubuntu
> +	'/usr/lib64/softhsm/',  # rosachrome, rosafresh

I couldn't test on rosa* variants and cannot claim the path is valid.
The most closest I was able to find is a spec file on repology.  From
there, the library seems to be installed at various compat locations,
including compatibility with Fedora.  So, likely it should be fine to
run the test with rhel-based path.

On the other hand, the list is open-ended, and it makes sense to allow
appending it in some handy way.

> +	'/usr/local/lib/softhsm/', # freebsd
> +	'/lib64/',              # redos, almalinux, centos, oracle, rocky
> +);
> +for my $so_path (@so_paths) {
> +	my $path = $so_path . 'libsofthsm2.so';
> +	if (-e $path) {
> +		$libsofthsm2_path = $path;
> +		last;
> +	}
> +};
> +
> +die 'Can\'t determine libsofthsm2.so path'
> +	unless $libsofthsm2_path;

I don't like terminating the test in unclean fasion just because we
are not aware of a path the library was installed on.  Note that
the "softhsm2-util" availability was checked prior to this.

> +
> +note("libsofthsm2_path: $libsofthsm2_path");
> +
>  $t->write_file('openssl.conf', <<EOF);
>  openssl_conf = openssl_def
>  
> @@ -100,8 +120,8 @@
>  
>  [pkcs11_section]
>  engine_id = pkcs11
> -dynamic_path = /usr/local/lib/engines/pkcs11.so
> -MODULE_PATH = /usr/local/lib/softhsm/libsofthsm2.so
> +#dynamic_path = /usr/local/lib/engines/pkcs11.so

Commenting out "dynamic_path" will break running on FreeBSD.

Note that both base and ports' openssl seeks for pkcs11.so
on paths different from the installed one by package libp11.

> +MODULE_PATH = $libsofthsm2_path
>  init = 1
>  PIN = 1234
>  
> @@ -125,21 +145,37 @@
>  $ENV{OPENSSL_CONF} = "$d/openssl.conf";
>  
>  foreach my $name ('localhost') {
> -	system('softhsm2-util --init-token --slot 0 --label NginxZero '
> +	my $cmd = 'softhsm2-util --init-token --slot 0 --label NginxZero '
>  		. '--pin 1234 --so-pin 1234 '
> -		. ">>$d/openssl.out 2>&1");
> +		. ">>$d/openssl.out 2>&1";
> +
> +	note("SOFTHSM2_CONF=$d/softhsm2.conf OPENSSL_CONF=$d/openssl.conf $cmd");

note() is a direct call to Test::More and may not be suppressed by
out test suite, please avoid using it.  Instead, log_core() and
variants should be used, if at all.  Here it makes little use.

> +
> +	system($cmd);
>  
> -	system('pkcs11-tool --module=/usr/local/lib/softhsm/libsofthsm2.so '
> +	$cmd = "pkcs11-tool --module=$libsofthsm2_path "
>  		. '-p 1234 -l -k -d 0 -a nx_key_0 --key-type rsa:2048 '
> -		. ">>$d/openssl.out 2>&1");
> +		. ">>$d/openssl.out 2>&1";
> +
> +	note("SOFTHSM2_CONF=$d/softhsm2.conf OPENSSL_CONF=$d/openssl.conf $cmd");
>  
> -	system('openssl req -x509 -new '
> +	system($cmd);
> +
> +	$cmd = 'openssl req -x509 -new '
>  		. "-subj /CN=$name/ -out $d/$name.crt -text "
>  		. "-engine pkcs11 -keyform engine -key id_00 "
> -		. ">>$d/openssl.out 2>&1") == 0
> -		or die "Can't create certificate for $name: $!\n";
> +		. ">>$d/openssl.out 2>&1";
> +
> +	note("SOFTHSM2_CONF=$d/softhsm2.conf OPENSSL_CONF=$d/openssl.conf $cmd");
> +
> +	my $openssl_call_result = system($cmd);
> +
> +	plan(skip_all => "Can't create certificate for $name: $!\n")
> +		unless $openssl_call_result == 0;

Such wording is badly suitable for skip_all reason.
You may want to examine existing cases and mimic it.

>  }
>  
> +$t->plan(2);
> +
>  $t->run();
>  
>  $t->write_file('index.html', '');

Below is the patch I tested on FreeBSD, RHEL- and Debian-based.
It should also work on exhotic platforms without modifications.
Please test if it works for you.

# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1716757790 -14400
#      Mon May 27 01:09:50 2024 +0400
# Node ID 6b0ad2d8bda5eaf9b69ad211f9bf28a85fe42cee
# Parent  5068d45ea46515e3437ba978ad012f05ce137374
Tests: adjusted ssl_engine_keys.t to run on Linux.

Previously, library paths were hardcoded for FreeBSD.

The TEST_NGINX_SOFTHSM environment variable is used to append
the list of tested directories to look for the softhsm library.
Further, testing demostrated that the "dynamic_path" command
is needed for FreeBSD only, where openssl is badly packaged.

Based on a patch by Oksana Deeva.

diff --git a/ssl_engine_keys.t b/ssl_engine_keys.t
--- a/ssl_engine_keys.t
+++ b/ssl_engine_keys.t
@@ -28,7 +28,7 @@ plan(skip_all => 'may not work, leaves c
 	unless $ENV{TEST_NGINX_UNSAFE};
 
 my $t = Test::Nginx->new()->has(qw/http proxy http_ssl/)->has_daemon('openssl')
-	->has_daemon('softhsm2-util')->has_daemon('pkcs11-tool')->plan(2);
+	->has_daemon('softhsm2-util')->has_daemon('pkcs11-tool');
 
 $t->write_file_expand('nginx.conf', <<'EOF');
 
@@ -86,10 +86,28 @@ EOF
 #
 # http://mailman.nginx.org/pipermail/nginx-devel/2014-October/006151.html
 #
-# Note that library paths may differ on different systems,
+# Note that library paths vary on different systems,
 # and may need to be adjusted.
 
-$t->write_file('openssl.conf', <<EOF);
+my $libsofthsm2_path;
+my @so_paths = (
+	'/usr/lib/softhsm',		# debian-based
+	'/usr/local/lib/softhsm',	# freebsd
+	'/lib64',			# rhel-based
+	split /:/, $ENV{TEST_NGINX_SOFTHSM} || ''
+);
+
+for my $so_path (@so_paths) {
+	$so_path .= '/libsofthsm2.so';
+	if (-e $so_path) {
+		$libsofthsm2_path = $so_path;
+		last;
+	}
+};
+
+plan(skip_all => "libsofthsm2.so not found") unless $libsofthsm2_path;
+
+my $openssl_conf = <<EOF;
 openssl_conf = openssl_def
 
 [openssl_def]
@@ -101,7 +119,7 @@ pkcs11 = pkcs11_section
 [pkcs11_section]
 engine_id = pkcs11
 dynamic_path = /usr/local/lib/engines/pkcs11.so
-MODULE_PATH = /usr/local/lib/softhsm/libsofthsm2.so
+MODULE_PATH = $libsofthsm2_path
 init = 1
 PIN = 1234
 
@@ -112,6 +130,9 @@ distinguished_name = req_distinguished_n
 [ req_distinguished_name ]
 EOF
 
+$openssl_conf =~ s|^(?=dynamic_path)|# |m if $^O ne 'freebsd';
+$t->write_file('openssl.conf', $openssl_conf);
+
 my $d = $t->testdir();
 
 $t->write_file('softhsm2.conf', <<EOF);
@@ -129,7 +150,7 @@ foreach my $name ('localhost') {
 		. '--pin 1234 --so-pin 1234 '
 		. ">>$d/openssl.out 2>&1");
 
-	system('pkcs11-tool --module=/usr/local/lib/softhsm/libsofthsm2.so '
+	system("pkcs11-tool --module=$libsofthsm2_path "
 		. '-p 1234 -l -k -d 0 -a nx_key_0 --key-type rsa:2048 '
 		. ">>$d/openssl.out 2>&1");
 
@@ -137,10 +158,10 @@ foreach my $name ('localhost') {
 		. "-subj /CN=$name/ -out $d/$name.crt -text "
 		. "-engine pkcs11 -keyform engine -key id_00 "
 		. ">>$d/openssl.out 2>&1") == 0
-		or die "Can't create certificate for $name: $!\n";
+		or plan(skip_all => "missing engine");
 }
 
-$t->run();
+$t->run()->plan(2);
 
 $t->write_file('index.html', '');
 


More information about the nginx-devel mailing list