Chunk 12 of CMP contribution to OpenSSL: CLI-based test by DDvO · Pull Request #11998 · openssl/openssl

DDvO · 2020-05-30T17:10:38Z

Finally the last chunk of the CMP contribution,
adding numerous tests run via the cmp app.

These tests address all previous contribution chunks:

high-level use cases for the CRMF+CMP sub-libraries contributed in chunks 1-7,
using the CMP server lib contributed in chunk 8: Chunk 8 of CMP contribution to OpenSSL: CMP server #11142 and mock server (in chunk 11),
using the CMP client logic contributed in chunk 9: Chunk 9 of CMP contribution to OpenSSL: CMP client and related tests #11300,
using the HTTP client contributed in chunk 10: Chunk 10 of CMP contribution to OpenSSL: CMP HTTP client and related tests #11404, and
the CLI option handling of the cmp app, contributed in chunk 11: Chunk 11 of CMP contribution to OpenSSL: CMP command-line interface #11470.

For part of the test cases the CMP client side is sufficient,
while many tests naturally also require a CMP server.
Therefore we made sure that our contribution contains
a simple CMP server that is sufficient for testing purposes.

The test bed is sufficiently flexible to support addressing also externals servers.
Throughout our development we have been testing against various EJBCA instances,
the Demo CA of the Insta Certifier, and Siemens-internal prototype RA implementations.

While updating and consolidating the many test cases that we have compiled earlier
I came across various issues mostly with the CMP code contributed so far but also
with the internal app library. So this PR also contains commits implementing fixes for
those as far as needed for successfully running all the CMP tests added in this PR.

The high number of lines added by the last commit is mostly due to large test data files.

Checklist

documentation is added or updated
tests are added or updated

DDvO · 2020-05-30T19:39:37Z

Can we have the Perl module Proc::Background installed on the build systems?
I even tried system ("perl -MCPAN -e \"install 'Proc::Background'\"") from within the tests
but no luck.
This module would offer a very nice system-independent way of launching the CMP test server in the background and shutting it down at the end of 81-test_cmp_cli.t.

So far I'm using the system("cmd &") pattern but of course this does not work on Windows.
Meanwhile I've disabled the tests for Windows and made sure they launch fine also on Mac OS.

DDvO · 2020-05-31T10:33:24Z

The Travis s390x build keeps timing out after 50 minutes; everything else works fine.

DDvO · 2020-06-09T06:01:32Z

Again my question:
what is the preferred way of launching a test server to run in parallel to a client and shutting it down at the end of the test?
There would be a nice likely system-independent Perl module for this purpose: Proc::Background, but at least so far it is not available on the build systems.

So I had to restrict these sort of tests to Unix-like systems, where the shell ampersand operator & for running commands in the background, the lsof command, and the kill command are available.

mattcaswell

what is the preferred way of launching a test server to run in parallel to a client and shutting it down at the end of the test?

We do this in util/perl/TLSProxy.pm where we start instances of s_server and s_client and get them to talk to each other. You might like to take a look how it is done there.

apps/lib/opt.c

crypto/cmp/cmp_server.c

doc/internal/man3/ossl_cmp_msg_check_update.pod

doc/man1/openssl-cmp.pod.in

doc/man3/OSSL_CMP_SRV_CTX_new.pod

test/recipes/81-test_cmp_cli.t

…nder CMP options

…g numbers

…ate_msg()

…k_update() Bugfix: allow using extraCerts contained in msg already while checking signature Improve function name, simplify its return value, and update its documentation

Also adds ossl_cmp_hdr_get_protection_nid() simplifying cmp_vfy.c

…ted sender

as checking expected_sender and adding caPubs is not part of msg validation. Also constify a couple of internal and public functions related to cmp_vfy.c

… request template

Certificate Management Protocol (CMP, RFC 4210) extension to OpenSSL Also includes CRMF (RFC 4211) and HTTP transfer (RFC 6712). Adds the CMP and CRMF API to libcrypto and the "cmp" app to the CLI. Adds extensive documentation and tests.

DDvO · 2020-06-10T20:04:52Z

Thanks @mattcaswell for your yesterday's review comments!
I've handled all of them, rebased the PR, and squashed the commits where it made sense.

DDvO · 2020-06-10T20:23:27Z

what is the preferred way of launching a test server to run in parallel to a client and shutting it down at the end of the test?

We do this in util/perl/TLSProxy.pm where we start instances of s_server and s_client and get them to talk to each other. You might like to take a look how it is done there.

Thanks @mattcaswell for this hint.
I've meanwhile had a look at util/perl/TLSProxy/Proxy.pm
and its use in test/recipes/70-test_*.t.
This appears pretty cumbersome for the basic use needed in 81-test_cmp_cli.t, and it would be hard to extract a simple mechanism for just launching any process in the background, checking if it is running, and shutting it down later.

With the Perl module Proc::Background it would be very nice and straightforward:

use Proc::Background;

# launch the server:
my $proc = Proc::Background->new({'die_upon_destroy' => 1}, "$cmd $args");
die "Cannot start server" unless $proc;

# interact with the server

# shut down the server:
$proc = undef;

As mentioned above, since unfortunately Proc::Background is not available (by default at least),
I've resorted to using instead the shell ampersand operator & for running commands in the background, the lsof command for obtaining its pid, and the kill command.
Due to this implementation I had to restrict the server-based to tests to Unix-like systems.

Would this be acceptable for the time being,
as long as apparently no general and simple-to-use solution is available?

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

…g numbers Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

…ate_msg() Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

…k_update() Bugfix: allow using extraCerts contained in msg already while checking signature Improve function name, simplify its return value, and update its documentation Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

Also adds ossl_cmp_hdr_get_protection_nid() simplifying cmp_vfy.c Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

…ted sender Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

as checking expected_sender and adding caPubs is not part of msg validation. Also constify a couple of internal and public functions related to cmp_vfy.c Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

… request template Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

Certificate Management Protocol (CMP, RFC 4210) extension to OpenSSL Also includes CRMF (RFC 4211) and HTTP transfer (RFC 6712). Adds the CMP and CRMF API to libcrypto and the "cmp" app to the CLI. Adds extensive documentation and tests. Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

DDvO · 2020-06-13T13:17:23Z

Merged - thanks @mattcaswell!

romen · 2020-06-13T23:41:17Z

@DDvO I am seeing this error compiling with ubsan on:

../../home/tuveri/repos/ossl_committer/workbench/crypto/cmp/cmp_msg.c:259:45: runtime error: signed integer overflow: 86400 * 36500 cannot be represented in type 'int'                                                                 
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../home/tuveri/repos/ossl_committer/workbench/crypto/cmp/cmp_msg.c:259:45 in                                                                                                 
../../../../../../../../../tmpram/ossl_build/util/wrap.pl ../../../../../../../../../tmpram/ossl_build/apps/openssl cmp -config ../Mock/test.cnf -section 'Mock enrollment' -proxy '' -cmd ir -newkey new.key -newkeypass 'pass:' -days 3
6500 -certout test.cert.pem -out_trusted root.crt => 1                                                                                                                                                                                  
    not ok 23 - days 36500                                                                                                                                                                                                              
                                                                                                                                                                                                                                        
    #   Failed test 'days 36500'                                                                                                                                                                                                        
    #   at ../../home/tuveri/repos/ossl_committer/workbench/test/recipes/81-test_cmp_cli.t line 171.                                                                                                                                    
cmp: Can't parse "-certout" as a number                                                                                                                                                                                                 
OPENSSL_FUNC:../../home/tuveri/repos/ossl_committer/workbench/apps/cmp.c:2848:CMP error: use -help for summary of 'cmp' options                                                                                                         
# OPENSSL_FUNC:../../home/tuveri/repos/ossl_committer/workbench/apps/cmp.c:2892:CMP info: using OpenSSL configuration file '../Mock/test.cnf'                                                                                           
# OPENSSL_FUNC:../../home/tuveri/repos/ossl_committer/workbench/apps/cmp.c:2498:CMP warning: argument of -proxy option is empty string, resetting option                                                                                
../../../../../../../../../tmpram/ossl_build/util/wrap.pl ../../../../../../../../../tmpram/ossl_build/apps/openssl cmp -config ../Mock/test.cnf -section 'Mock enrollment' -proxy '' -cmd ir -newkey new.key -newkeypass 'pass:' -days -
certout test.cert.pem -out_trusted root.crt => 1                                                                                                                                                                                        
    ok 24 - days missing arg

DDvO · 2020-06-17T06:19:16Z

@romen, I've been able to reproduce this, having configured with enable-uban.
This will be fixed by #12175.

Among others, I've improved the range checking. A too large -days option value now would give

make test VFO=1 TESTS="test_cmp_cli" 
make depend && make _tests
make[1]: Entering directory '/home/david/Siemens/proj/PKI/CMP/CMPforOpenSSL/cmpossl_incremental12'
make[1]: Leaving directory '/home/david/Siemens/proj/PKI/CMP/CMPforOpenSSL/cmpossl_incremental12'
make[1]: Entering directory '/home/david/Siemens/proj/PKI/CMP/CMPforOpenSSL/cmpossl_incremental12'
( SRCTOP=. \
  BLDTOP=. \
  PERL="/usr/bin/perl" \
  EXE_EXT= \
  /usr/bin/perl ./test/run_tests.pl test_cmp_cli )

81-test_cmp_cli.t .. 6/? OPENSSL_FUNC:apps/cmp.c:1886:CMP error: -days argument is too large: resulting epoch value exceeds INT_MAX
OPENSSL_FUNC:apps/cmp.c:3055:CMP error: cannot set up CMP context
# OPENSSL_FUNC:apps/cmp.c:2895:CMP info: using OpenSSL configuration file '../Mock/test.cnf'
# OPENSSL_FUNC:apps/cmp.c:2500:CMP warning: argument of -proxy option is empty string, resetting option
# OPENSSL_FUNC:apps/cmp.c:2112:CMP info: will contact http://127.0.0.1:1700/pkix/
# OSSL_CMP_CTX_set_option:crypto/cmp/cmp_ctx.c:945:CMP error: value too large
../../../../util/wrap.pl ../../../../apps/openssl cmp -config ../Mock/test.cnf -section 'Mock enrollment' -proxy '' -cmd ir -newkey new.key -newkeypass 'pass:' -days 36500 -certout test.cert.pem -out_trusted root.crt => 1
    not ok 23 - days 36500
    # Looks like you failed 1 test of 93.
not ok 7 - CMP app CLI Mock enrollment
81-test_cmp_cli.t .. 7/? # 

#   Failed test 'CMP app CLI Mock enrollment
# '
#   at /home/david/Siemens/proj/PKI/CMP/CMPforOpenSSL/cmpossl_incremental12/util/perl/OpenSSL/Test.pm line 1302.
# Looks like you failed 1 test of 7.
81-test_cmp_cli.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/7 subtests 

Test Summary Report
-------------------
81-test_cmp_cli.t (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1
Files=1, Tests=7, 65 wallclock secs ( 0.17 usr  0.01 sys + 20.67 cusr  5.28 csys = 26.13 CPU)
Result: FAIL

mouse07410 · 2020-06-17T14:46:31Z

@DDvO please see #12145. #12175 (at least, the way it is now) did not seem to help. :-(

DDvO · 2020-06-18T04:43:57Z

@DDvO please see #12145. #12175 (at least, the way it is now) did not seem to help. :-(

I've just fixed this in a new commit in #12175.

DDvO force-pushed the cmpossl_incremental12 branch from fbb7656 to eaf5950 Compare June 5, 2020 06:16

mattcaswell requested changes Jun 9, 2020

View reviewed changes

DDvO added 22 commits June 10, 2020 21:57

Fix too strict checks of ossl_cmp_calc_protection()

65d4a6e

Streamline the approach to set CMP message recipient and expected sender

925ae81

Improve description of -trusted, -srvcert, -recipient, and -expect_se…

7d038e0

…nder CMP options

Check expected sender not only for signature-protected CMP messages

cc50c9c

Correct error output of parse_name() in apps/lib/apps.c and apps/cmp.c

85e7396

Allow subject of CMP -oldcert as sender unless protection cert is given

d1196f7

Add request URL path checking and status responses to HTTP server

345874e

Fix use of -no-proxy option of CMP app

bb48c13

Remove meanwhile redundant error output of apps/opt_next(void) parsin…

9ce6b59

…g numbers

Extend error output of apps/opt_format() to all error cases

d689df0

Correct error reason of verify_signature() in cmp_vfy.c

988b6aa

Remove misleading diagnostics on pinned sender cert in OSSL_CMP_valid…

6526fad

…ate_msg()

Improve cert checking diagnostics of OSSL_CMP_validate_msg()

1047ee1

Improve ossl_cmp_msg_check_received() and rename to ossl_cmp_msg_chec…

50af167

…k_update() Bugfix: allow using extraCerts contained in msg already while checking signature Improve function name, simplify its return value, and update its documentation

Fill in transactionID on any error in OSSL_CMP_SRV_process_request()

aa43c75

Make CMP server use same protection for response as for request

63f4ebb

Also adds ossl_cmp_hdr_get_protection_nid() simplifying cmp_vfy.c

Improve description of CMP untrusted certs and msg 'sender' field

26f4c38

Bug fix in ossl_cmp_certRep_new(): must allocate empty extraCerts stack

4830116

Bug fix in ossl_cmp_hdr_init(): sould not remember recipient as expec…

5590808

…ted sender

Move part of OSSL_CMP_validate_msg() to ossl_cmp_msg_check_update()

aa6792c

as checking expected_sender and adding caPubs is not part of msg validation. Also constify a couple of internal and public functions related to cmp_vfy.c

Remove extra newline from CMP mock server error and add TODO on using…

50b8592

… request template

DDvO force-pushed the cmpossl_incremental12 branch from eaf5950 to d72364e Compare June 10, 2020 20:00

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Check expected sender not only for signature-protected CMP messages

6d934ad

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Correct error output of parse_name() in apps/lib/apps.c and apps/cmp.c

7e998a0

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Allow subject of CMP -oldcert as sender unless protection cert is given

1693135

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Add request URL path checking and status responses to HTTP server

5a2ba20

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Fix use of -no-proxy option of CMP app

50e0978

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Remove meanwhile redundant error output of apps/opt_next(void) parsin…

2620c38

…g numbers Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Extend error output of apps/opt_format() to all error cases

f009e37

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Correct error reason of verify_signature() in cmp_vfy.c

032b13c

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Remove misleading diagnostics on pinned sender cert in OSSL_CMP_valid…

7aa70fd

…ate_msg() Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Improve cert checking diagnostics of OSSL_CMP_validate_msg()

ca6f1ba

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Fill in transactionID on any error in OSSL_CMP_SRV_process_request()

5aed178

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Make CMP server use same protection for response as for request

12bbcee

Also adds ossl_cmp_hdr_get_protection_nid() simplifying cmp_vfy.c Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Improve description of CMP untrusted certs and msg 'sender' field

8b22c28

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Bug fix in ossl_cmp_certRep_new(): must allocate empty extraCerts stack

1a27fe4

Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Bug fix in ossl_cmp_hdr_init(): sould not remember recipient as expec…

7e8dbb7

…ted sender Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

openssl-machine pushed a commit that referenced this pull request Jun 13, 2020

Remove extra newline from CMP mock server error and add TODO on using…

5e7be6e

… request template Reviewed-by: Matt Caswell <[email protected]> (Merged from #11998)

DDvO closed this Jun 13, 2020

romen mentioned this pull request Jun 16, 2020

Extended tests are failing cmp tests #12145

Closed

DDvO mentioned this pull request Jun 18, 2020

Fix CMP CLI tests #12175

Closed

DDvO mentioned this pull request Jun 30, 2020

Test 81-test_cmp_cli.t hangs on some platforms #12324

Closed

DDvO mentioned this pull request Nov 16, 2020

apps.c: re-enable loading single certs and CRLs over HTTP #13404

Closed

DDvO mentioned this pull request May 23, 2021

Test 80-test_cmp_http fails at alpha17 on NonStop x86/ia64 #15386

Closed

Uh oh!

Conversation

DDvO commented May 30, 2020

Checklist

Uh oh!

DDvO commented May 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DDvO commented May 31, 2020

Uh oh!

DDvO commented Jun 9, 2020

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DDvO commented Jun 10, 2020

Uh oh!

DDvO commented Jun 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DDvO commented Jun 13, 2020

Uh oh!

romen commented Jun 13, 2020

Uh oh!

DDvO commented Jun 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mouse07410 commented Jun 17, 2020

Uh oh!

DDvO commented Jun 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

DDvO commented May 30, 2020 •

edited

Loading

DDvO commented Jun 10, 2020 •

edited

Loading

DDvO commented Jun 17, 2020 •

edited

Loading