Skip to content

WIP: Preview of CMP implementation, incremental PR chunk 4: CMP context/parameters#178

Closed
DDvO wants to merge 51 commits intocmpossl_incremental3from
cmpossl_incremental4
Closed

WIP: Preview of CMP implementation, incremental PR chunk 4: CMP context/parameters#178
DDvO wants to merge 51 commits intocmpossl_incremental3from
cmpossl_incremental4

Conversation

@DDvO
Copy link
Collaborator

@DDvO DDvO commented Apr 24, 2019

NOTE: this is not an actual pull request but is meant as a preview of a later OpenSSL PR.
While chunk 3 openssl#8669 is not yet fully approved, this preview can be used for adding initial review comments.

Certificate Management Protocol (CMP, RFC 4210) extension to OpenSSL.
Also includes CRMF (RFC 4211) and HTTP transfer (RFC 6712).

CMP and CRMF API is added to libcrypto, and the "cmp" app to the openssl CLI.
Adds extensive man pages and tests. Integration into build scripts.

4th chunk: CMP context/parameters in cmp_ctx.c and cmp_ctx_test.c and related files.

Checklist
  • documentation is added or updated
  • tests are added or updated

@DDvO
Copy link
Collaborator Author

DDvO commented May 6, 2019

@mattcaswell, did you notice that, as discussed in with our PR for chunk3: openssl#8669, the preview of our chunk 4 is here?

@Akretsch Akretsch force-pushed the cmpossl_incremental3 branch from 67032c0 to ff1d80b Compare May 7, 2019 07:08
@Akretsch Akretsch force-pushed the cmpossl_incremental4 branch from 283178d to b328f9f Compare May 7, 2019 07:32
@mattcaswell
Copy link

Yes, I noticed...I just haven't got as far as taking a look yet. Will try to do that soon.

@DDvO DDvO force-pushed the cmpossl_incremental4 branch 2 times, most recently from 8f1ef7e to 70f9953 Compare May 13, 2019 17:04
@DDvO
Copy link
Collaborator Author

DDvO commented May 15, 2019

Thanks @mattcaswell for all your comments!
Meanwhile we have addressed all of them.

@DDvO DDvO force-pushed the cmpossl_incremental4 branch from 39c2fcb to 168a960 Compare May 17, 2019 13:37
@Akretsch Akretsch force-pushed the cmpossl_incremental3 branch 2 times, most recently from 9a3eafb to 6ac128f Compare May 27, 2019 14:30
@Akretsch Akretsch force-pushed the cmpossl_incremental4 branch from cec9c6b to 317ad15 Compare May 27, 2019 14:54
t8m and others added 9 commits May 28, 2019 17:14
The openssl#7408 implemented mandatory digest checking in TLS.
However this broke compatibility of DSS support with GnuTLS
which supports only SHA1 with DSS.

There is no reason why SHA256 would be a mandatory digest
for DSA as other digests in SHA family can be used as well.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from openssl#9015)
Use -bnoentry, not -bexpall

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from openssl#9012)
Not all Unixen know the -v option

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from openssl#9012)
enabling the 'enable-crypto-mdebug' option and running parameter generation
causes timeouts.
Loading pregenerated params is more suited for these tests.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#9022)
Rework the test so that it fails far less often.

A number of independent tests are executed and 5% are expected to fail.
The number of such failures follows a binomial distribution which permits
a statistical test a 0.01% expected failure rate.

There is a command line option to enable the stochastic range checking.
It is off by default.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#8830)
CLA: trivial

Reviewed-by: Paul Yang <yang.yang@baishancloud.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from openssl#9021)
openssl_config_int() returns the uninitialized variable `ret`
when compiled with OPENSSL_SYS_UEFI.

Fixes openssl#9026

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from openssl#9029)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from openssl#8117)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from openssl#8117)
@DDvO DDvO force-pushed the cmpossl_incremental3 branch from 1586c96 to e2715d6 Compare May 29, 2019 07:30
This has been long overdue.

Note that this does not join the X509 and X509V3 error modules, that
will be too many macro changes at this stage.

Fixes openssl#8919

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from openssl#8925)
@DDvO DDvO force-pushed the cmpossl_incremental3 branch 2 times, most recently from 2abe29d to 483174d Compare May 29, 2019 09:42
@mattcaswell
Copy link

Can this be raised as an official PR now?

@DDvO
Copy link
Collaborator Author

DDvO commented Jun 5, 2019

Can this be raised as an official PR now?

Did you see the two comments I made above: #178 (comment) and #178 (comment) yesterday evening?
Among others, I wrote

In preparation of a separate PR for the trace API improvements and CMP chunk 4 (which I plan for later this week) I've just moved the enhancements of the trace API (namely, a severity/verbosity level and automatic output of function name, file name, line number, and severity level) to trace.h and trace.c and did some small further improvements.
What do you think about the new trace/logging code?

This causes travis build failures on master

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9087)
@DDvO DDvO force-pushed the cmpossl_incremental4 branch from 2ff89f1 to 9c6952d Compare June 5, 2019 19:26
@DDvO
Copy link
Collaborator Author

DDvO commented Jun 5, 2019

@mattcaswell, I've just dome some further small improvements to the preliminary trace enhancements.
Would be nice if you can give some quick first feedback on them before I raise this as an official PR (after carving out the changes to the trace API as an extra RP) because it includes some assumptions about the features of the trace API (mostly in cmp_util.h and cmp_util.c).
I can do the PRs for CMP chunk 4 and for the trace API improvements either tomorrow or on Friday.

@mattcaswell
Copy link

Will try and take a look tomorrow.

slontis and others added 8 commits June 6, 2019 09:34
covID 1445689 Resource leak (in error path)
covID 1445318 Resource leak (in test - minor)
covID 1443705 Unchecked return value (Needed if CRYPTO_atomic_add() was used)
covID 1443691 Resource leak (in app - minor)

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#9071)
… folders

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9083)
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from openssl#9090)
Fixes openssl#9092

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from openssl#9093)
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
GH: openssl#7651
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
GH: openssl#7651
[skip ci]

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9102)
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from openssl#9101)
@Akretsch Akretsch force-pushed the cmpossl_incremental4 branch from 9c6952d to 35d89b4 Compare June 7, 2019 09:05
In preparation for moving the RAND code into the FIPS module we make
drbg_lib.c OPENSSL_CTX aware.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9039)
This is in preparation for moving this code inside the FIPS module.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9039)
It was previously rand_lib but it makes more sense in drbg_lib.c since
all the functions that use this lock are only ever called from drbg_lib.c

We add some FIPS_MODE defines in preparation for later moving this code
into the FIPS module.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9039)
Various functions have been added that take an OPENSSL_CTX parameter as
a result of moving the RAND code into the FIPS module. We document all of
those functions.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9039)
@Akretsch Akretsch force-pushed the cmpossl_incremental4 branch 2 times, most recently from e7e96b7 to 394e744 Compare June 7, 2019 11:25
Akretsch added 3 commits June 7, 2019 14:21
Also includes CRMF (RFC 4211) and HTTP transfer (RFC 6712)

CMP and CRMF API is added to libcrypto, and the "cmp" app to the openssl CLI.
    Adds extensive man pages and tests.  Integration into build scripts.

Incremental pull request based on OpenSSL commit 1362190 of 2018-09-26

3rd chunk: CMP ASN.1 structures (in crypto/cmp/cmp_asn.c) and related files
Also includes CRMF (RFC 4211) and HTTP transfer (RFC 6712)

CMP and CRMF API is added to libcrypto, and the "cmp" app to the openssl CLI.
    Adds extensive man pages and tests.  Integration into build scripts.

Incremental pull request based on OpenSSL commit 1362190 of 2018-09-26

3rd chunk: CMP ASN.1 structures (in crypto/cmp/cmp_asn.c) and related files
    Also includes CRMF (RFC 4211) and HTTP transfer (RFC 6712)

    CMP and CRMF API is added to libcrypto, and the "cmp" app to the openssl CLI.
        Adds extensive man pages and tests.  Integration into build scripts.

    Incremental pull request based on OpenSSL commit 8869ad4 of 2019-04-02

    4th chunk: CMP context/parameters and utilities
    in crypto/cmp/cmp_ctx.c, crypto/cmp/cmp_util.c, and related files
@DDvO
Copy link
Collaborator Author

DDvO commented Jun 7, 2019

@mattcaswell
Copy link

Yes I saw - apologies for not getting back to you before you got that far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.