Hi,

As announced in #5926, here the official pull request for the CMP extension. In the meanwhile we have continued improving the code and documentation, added unit tests, etc.

Among others, we took care of all three points mentioned by @levitte on Apr 11; adding the OSSL_ prefix to all exported symbols, updating the storage location of test input files and how to refer to them, and removing all OPENSSL_VERSION switches.

We have significantly extended the tests and now have hundreds of test cases as well on low-level as also for the CLI application. Some of the test cases require online connectivity and availability of the respective servers - and thus are not enabled by default. For executing the tests (with verbose output) with the EJBCA AWS instance currently provided by PrimeKey, use e.g.

http_proxy=(set if needed) CMP_TESTS="EJBCA" make tests TESTS="test_cmp_cli" VERBOSE=1

Of course, code is is never finished, and we are aware of additional features and fixes, which would be nice to have, and will eventually also come - those are documented on https://github.com/mpeylo/cmpossl/issues

@DDvO and I will be happy to read any of your feedback,
Martin

There are two zip files included in this PR. What are they for?

Thanks @mattcaswell for your quick comments and questions.
I'll answer them step by step. My changes mentioned here will become visible later (when they have been merged and pushed by @mpeylo).

One .zip file contains build info for OpenSSL v1.0.2 and should not have been part of this PR (like the version switches we already eliminated). I've just removed it.

The other .zip file contains conformance test data for EJBCA and is no longer needed in this form. I had forgotten to remove it; done now.

The figure 640,787 is not actually the number of lines of code , but apparently the code size in bytes.
This PR has around 55,000 16,700 lines of code.
UPDATE of 2018-8-10: The figure I gave at first unfortunately was also misleading because what I referred to by mistake was the number of words. The statistics given by git were deformed by the huge number of lines of five test files, which contained 1,000 PEM-encoded certificates each. After removing these from the PR the total number of lines dropped below 28,000. In fact this contribution has less than 17,000 lines of actual code, while the rest of the lines reported by git is mostly due to test data files.

Here is a list of the included .c and .h files (apart from those generated automatically) and their sizes produced with wc include/openssl/crmf.h crypto/crmf/crmf_int.h crypto/crmf/crmf_{asn ,lib,pbm}.c include/openssl/cmp.h crypto/cmp/cmp_int.h crypto/cmp/cmp_{asn,ctx,lib,msg,ses,srv,vfy,h ttp}.c apps/cmp.c test/cmp_{ctx,lib,msg,ses,vfy,internal}_test.c test/cmptestlib.{c,h}:

   130    365   5597 include/openssl/crmf.h
   445   1669  15453 crypto/crmf/crmf_int.h
   266    585  11483 crypto/crmf/crmf_asn.c
   688   1980  21987 crypto/crmf/crmf_lib.c
   226    861   7259 crypto/crmf/crmf_pbm.c
   574   2410  27914 include/openssl/cmp.h
   743   3297  30884 crypto/cmp/cmp_int.h
   395    939  16867 crypto/cmp/cmp_asn.c
  1488   4601  39599 crypto/cmp/cmp_ctx.c
  1743   5802  54509 crypto/cmp/cmp_lib.c
   783   2368  24574 crypto/cmp/cmp_msg.c
   827   2785  28425 crypto/cmp/cmp_ses.c
   650   1917  22078 crypto/cmp/cmp_srv.c
   783   3216  30134 crypto/cmp/cmp_vfy.c
   506   1909  15974 crypto/cmp/cmp_http.c
  4119  14502 140651 apps/cmp.c
    96    227   2708 test/cmp_ctx_test.c
   784   1828  28491 test/cmp_lib_test.c
   451   1006  14482 test/cmp_msg_test.c
   332    767  11184 test/cmp_ses_test.c
   341    910  11701 test/cmp_vfy_test.c
   221    567   7148 test/cmp_internal_test.c
   128    390   3157 test/cmptestlib.c
    34    116   1003 test/cmptestlib.h
 16753  55017 573262 total

A potential split could for instance be done as follows, in the given order of dependencies:

• CRMF lib (crypto/crmf)
• CMP lib (crypto/cmp/)
• CMP CLI (apps/cmp.c)
• Tests (test/)

The fact that there is pretty little interference with other parts of OpenSSL was on purpose.

• In this way we have been able to develop mostly independently of code evolution in the OpenSSL core libs, apps, and tests.
• Moreover, we think/hope that also the reviewing and merging process is smoother if it is clear that existing code will not break by including this code.

Actually, there are a couple of overlaps with existing code:

• the primitive HTTP client for far being used only for OCSP has been extended in crypto/cmp/cmp_http.c in terms of stability, error checking and reporting, and the addtion of timeout.
• the functions for reading and writing various types of files in apps/ have been extended and partly improved in apps/cmp.c.
• the functionality for certificate status checking (including CRL fetching and combining CRL-based checks with classical OCSP and with OCSP stapling, where I still have further improvements in the pipeline) have been significantly extended, also in apps/cmp.c.

I started almost a year ago with carving out these changes because they are of more general interest (this is, for use also independently of CMP). This resulted in a series of rather small PRs (the first one being #4124). Part of them have meanwhile been merged, while others are still open and further ones are yet to be given, which one can see also from the open TODOs I put in apps/cmp.c and crypto/cmp/cmp_http.c (marked as (PR #...)

The dilemma we face here is that the CMP code (mostly in apps/cmp.c) needs these additions and improvements already for productive use, so we cannot remove them from our code until they have been included in OpenSSL and the respective new OpenSSL release has been adopted within our companies and by various external users.
How do you advise us to proceed with this topic?

What is the smallest standalone piece that we can pull out of here? CRMF?

A possible route could be like this:

1. A PR for the CRMF documentation. This enables us to review the API as a whole and make sure it is coherent and well designed.
2. Once (1) is approved then raise a PR for the CRMF implementation, as well as its associated tests

Possibly we may want to wait until both (1) and (2) are approved before pushing either of them.

Once CRMF is in we pick the next smallest standalone thing that makes sense and repeat the above. And so on.

This sounds like a good pragmatic way to go.

Yes, the CRMF sub-lib is the smallest piece that can be seen as an independent contribution. (Yet the CRMF layer does not have tests of its own - we test it indirectly along with CMP.)

As you suggested, I'll produce a PR for its documentation and then one for its implementation.
(To this end, I'll first solve some structural issues with the .pod files indicated by Travis CI running make doc-nits).

As recently agreed by email we have started providing the CMPforOpenSSL contribution in small incremental chunks, in PR #7328.

@mpeylo, I think you can close this PR since it is superseded by PRs #7328, #7646, and further upcoming chunks.

Closing this PR as it as been superseded since long.

Not all CMP contribution chunks have been provided as PR and reviewed/merged yet,
but the plan is to include all parts with OpenSSL version 3.0.
For details on the current state and planning see
https://github.com/mpeylo/cmpossl/wiki#planning-and-current-status-of-the-contribution-chunks