Chunk 8 of CMP contribution to OpenSSL: CMP server by DDvO · Pull Request #11142 · openssl/openssl

DDvO · 2020-02-21T07:01:34Z

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.

8th chunk:

generic CMP server code in crypto/cmp/cmp_server.c
mock server for testing the CMP client in apps/cmp_mock_srv.c

and related files.

This PR is the continuation of mpeylo#201.

Due to moving the mock server code out of libcrypto as discussed mpeylo#201 (comment)
a couple of further CMP functions had to be exported.

This PR also includes various fixups to earlier CMP contribution chunks that came up during re-structuring the (test) server code and doing tests on the client code in upcoming chunk 9.
Among others, it slightly generalizes
static int ts_check_status_info(TS_RESP *response) in crypto/ts/ts_rsp_verify.c to
sk_ASN1_UTF8STRING2text(STACK_OF(ASN1_UTF8STRING) *text, char sep) and
makes it accessible within libcrypto by adding its declaration to include/internal/cryptlib.h.

documentation is added or updated
tests are added or updated

DDvO · 2020-02-21T08:26:47Z

CI builds went fine (except for the one Travis build that notoriously times out these days).

DDvO · 2020-02-24T09:42:34Z

Rebased this PR after conflicts with latest master.
@mattcaswell, when do you think you can continue your review of chunk 8?

mattcaswell · 2020-02-25T04:45:58Z

@mattcaswell, when do you think you can continue your review of chunk 8?

Probably not until next week. I am away at the RSA conference this week.

DDvO · 2020-03-03T14:32:25Z

Rebased this PR again after the usual conflicts with util/libcrypto.num of the latest master.

DDvO · 2020-03-04T06:38:42Z

Travis fails just due to unrelated doc nit:

doc/man1/openssl-rand.pod:1: found 'non-zero' should use 'nonzero'

apps/cmp_mock_srv.c

crypto/cmp/cmp_server.c

doc/man3/OSSL_CMP_STATUSINFO_new.pod

mattcaswell

For the record I have already provided significant review comments on a preview of this PR. This is just my comments on the latest iteration.

apps/cmp_mock_srv.c

crypto/ts/ts_rsp_verify.c

doc/man3/OSSL_CMP_CTX_new.pod

doc/man3/OSSL_CMP_X509_digest.pod

test/cmp_ctx_test.c

doc/man3/OSSL_CMP_SRV_CTX_new.pod

DDvO · 2020-03-05T17:22:48Z

Thanks a lot @mattcaswell for your second review round with pretty good comments mostly on the structure of this CMP chunk!
I've meanwhile handled all of them.

richsalz · 2020-03-05T19:09:15Z

You can use TEST_note to print out useful debug notes.

DDvO · 2020-03-05T20:02:52Z

You can use TEST_note to print out useful debug notes.

Thanks @richsalz for the hint - done.

DDvO · 2020-03-09T11:15:51Z

@mattcaswell, is there anything more to do on this PR?

crypto/asn1/asn1_lib.c

test/cmp_server_test.c

DDvO · 2020-03-09T12:59:28Z

@mattcaswell, all (new) change requests fulfilled and rebased (to handle libcrypto.num conflicts).

mattcaswell

LGTM

…csp.h

…_verify.c to asn1_lib.c

… for testing 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.

…fy_popo()

…tions

DDvO · 2020-03-10T09:45:28Z

I've re-ordered and partly squashed the commits such that merging (which I plan for this afternoon, after the grace period has passed) should be smooth and clean.

openssl-machine · 2020-03-10T14:00:19Z

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

…csp.h Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

…_verify.c to asn1_lib.c Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

… for testing 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]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

…fy_popo() Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

…tions Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

DDvO · 2020-03-10T15:17:54Z

Pushed. Thanks @mattcaswell for your swift review!

crypto/cmp/cmp_msg.c

DDvO mentioned this pull request Feb 21, 2020

WIP: CMP implementation, incremental PR chunk 8: CMP generic and mock server mpeylo/cmpossl#201

Closed

1 task

DDvO force-pushed the cmpossl_incremental8 branch from 664609d to 2021599 Compare February 21, 2020 07:38

DDvO requested a review from mattcaswell February 21, 2020 10:43

DDvO force-pushed the cmpossl_incremental8 branch from 2021599 to 81919c9 Compare February 24, 2020 09:40

DDvO force-pushed the cmpossl_incremental8 branch from 04ab728 to d020066 Compare March 3, 2020 14:30

DDvO commented Mar 4, 2020

View reviewed changes

DDvO self-assigned this Mar 4, 2020

Akretsch force-pushed the cmpossl_incremental8 branch from 3d702ab to c412f86 Compare March 4, 2020 14:29

mattcaswell requested changes Mar 4, 2020

View reviewed changes

richsalz mentioned this pull request Mar 4, 2020

Don't compile commands if disabled #11250

Closed

DDvO force-pushed the cmpossl_incremental8 branch from 84ad597 to 84cb068 Compare March 5, 2020 17:01

mattcaswell requested changes Mar 9, 2020

View reviewed changes

crypto/asn1/asn1_lib.c Outdated Show resolved Hide resolved

crypto/asn1/asn1_lib.c Outdated Show resolved Hide resolved

crypto/asn1/asn1_lib.c Outdated Show resolved Hide resolved

test/cmp_server_test.c Outdated Show resolved Hide resolved

DDvO force-pushed the cmpossl_incremental8 branch from 8bfb2b2 to 6a8645d Compare March 9, 2020 12:58

mattcaswell approved these changes Mar 9, 2020

View reviewed changes

mattcaswell added the approval: done This pull request has the required number of approvals label Mar 9, 2020

DDvO added 5 commits March 10, 2020 10:41

fix two mistakes w.r.t. ERR_LIB_* parameters in ERR_add_error_txt()

6dfd20c

fix coding style nits w.r.t. 'defined' in http_local.h, http.h, and o…

d4078b0

…csp.h

Extract sk_ASN1_UTF8STRING2text() from ts_get_status_text() in ts_rsp…

8b4b687

…_verify.c to asn1_lib.c

Fix error in case CMP msg POPO is not provided in OSSL_CRMF_MSGS_veri…

ee93953

…fy_popo()

DDvO added 2 commits March 10, 2020 10:41

Fix handling of CMP msg senderKID and improve doc of related CTX func…

b99a58a

…tions

Move OSSL_CMP_X509_digest() to x_all.c, renaming it to X509_digest_sig()

089188d

DDvO force-pushed the cmpossl_incremental8 branch from 6a8645d to 089188d Compare March 10, 2020 09:42

DDvO added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Mar 10, 2020

openssl-machine pushed a commit that referenced this pull request Mar 10, 2020

fix two mistakes w.r.t. ERR_LIB_* parameters in ERR_add_error_txt()

4c7f898

Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

openssl-machine pushed a commit that referenced this pull request Mar 10, 2020

fix coding style nits w.r.t. 'defined' in http_local.h, http.h, and o…

b5adba2

…csp.h Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

openssl-machine pushed a commit that referenced this pull request Mar 10, 2020

Fix error in case CMP msg POPO is not provided in OSSL_CRMF_MSGS_veri…

7269071

…fy_popo() Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

openssl-machine pushed a commit that referenced this pull request Mar 10, 2020

Fix handling of CMP msg senderKID and improve doc of related CTX func…

cfca56d

…tions Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

openssl-machine pushed a commit that referenced this pull request Mar 10, 2020

Move OSSL_CMP_X509_digest() to x_all.c, renaming it to X509_digest_sig()

44387c9

Reviewed-by: Matt Caswell <[email protected]> Reviewed-by: David von Oheimb <[email protected]> (Merged from #11142)

DDvO closed this Mar 10, 2020

levitte reviewed Mar 10, 2020

View reviewed changes

crypto/cmp/cmp_msg.c Show resolved Hide resolved

DDvO mentioned this pull request Mar 11, 2020

Rename OSSL_{d2i,i2d}_CMP_MSG_bio to {d2i,id2}_OSSL_CMP_MSG_bio #11307

Closed

DDvO mentioned this pull request May 30, 2020

Chunk 12 of CMP contribution to OpenSSL: CLI-based test #11998

Closed

2 tasks

Uh oh!

Conversation

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

Uh oh!

Uh oh!

DDvO commented Feb 21, 2020

Uh oh!

DDvO commented Feb 24, 2020

Uh oh!

mattcaswell commented Feb 25, 2020

Uh oh!

DDvO commented Mar 3, 2020

Uh oh!

DDvO commented Mar 4, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

DDvO commented Mar 5, 2020

Uh oh!

richsalz commented Mar 5, 2020

Uh oh!

DDvO commented Mar 5, 2020

Uh oh!

DDvO commented Mar 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

DDvO commented Mar 10, 2020

Uh oh!

openssl-machine commented Mar 10, 2020

Uh oh!

DDvO commented Mar 10, 2020

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DDvO commented Feb 21, 2020 •

edited

Loading

DDvO commented Mar 9, 2020 •

edited

Loading