BTW, I'm on vacation for the next two weeks (until June 23rd),
but Andreas likely will be able to react to review comments and perform requested changes.

rebased to openssl:master; fix conflicts in util/libcrypto.num and util/private.num

@Akretsch has rebased this PR again such that currently there are no conflicts any more.
The Travis CI issue currently shown is unrelated to our PR.

@FdaSilvaYY, thanks for your comments about 16 days ago.
Did you notice that @Akretsch reacted to them two weeks back?
Anything still to be done from your perspective?

@FdaSilvaYY, thanks for your comments about 16 days ago.
Did you notice that @Akretsch reacted to them two weeks back?
Anything still to be done from your perspective?

I'm Ok with answers, nothing more to add.
My remarks are not blocking ones, as I'm not part of project.

@mattcaswell, on May 8th you suggested for the preview of this chunk (mpeylo#178 (comment)) to use the trace API for CMP logging (info/warning/error etc. output).
It turns out that this is not suitable because the trace API is statically disabled by default, so would not be usable for normal/production builds.

So I'm not sure whether it's worth continuing with the extracted enhancements of the trace API I proposed in PR #9110.

We're currently re-working this aspect of our CMP contribution such that warnings etc. are buffered in the CMP context and can later be output, using any means, at application level.

you suggested for the preview of this chunk (mpeylo#178 (comment)) to use the trace API for CMP logging (info/warning/error etc. output).
It turns out that this is not suitable because the trace API is statically disabled by default, so would not be usable for normal/production builds

Ok, if the trace API is not viable then we need a rethink.

So I'm not sure whether it's worth continuing with the extracted enhancements of the trace API I proposed in PR #9110.

Perhaps not.

We're currently re-working this aspect of our CMP contribution such that warnings etc. are buffered in the CMP context and can later be output, using any means, at application level.

Ok - that sounds interesting. I'd like to understand that proposal a little better.

Thanks @mattcaswell for your swift response.

So I'm not sure whether it's worth continuing with the extracted enhancements of the trace API I proposed in PR #9110.

Perhaps not.

So I tend to close that PR unless anyone objects there.
In case it turns out to be of interest later, I suppose it could be re-opened.
BTW, does the OpenSSL project have a backlog?

We're currently re-working this aspect of our CMP contribution such that warnings etc. are buffered in the CMP context and can later be output, using any means, at application level.

Ok - that sounds interesting. I'd like to understand that proposal a little better.

We plan for two things depending on the source/type of warning/info.

• The PKIStatus info included in the server's reply is already stored in the CMP_CTX: ctx->lastPKIStatus.
  In case this is not 'accepted' and there is no actual error then a respective warning message can be generated and output at application level, e.g., in apps/cmp.c.
• We also have other sources of warnings and infos. For these we plan to simply provide string fields such as ctx->warnings and ctx->infos, which can be incrementally appended to line-by-line by the lib and then output at application level.

Of course it would be simpler to output warnings and infos right away (as we did so far, using a callback stored in the CMP_CTX) such that they do not need to be buffered, but the crypto lib seems not to support this, maybe because the idea is that the lib itself should be silent or because warnings are not considered of interest.

Another option could be to extend the existing error queue mechanism to support warnings (and possibly further levels of severity/verbosity).

What do you think?

Hmm, I just realized that buffering status info messages for output after return is not always adequate.
For instance, intermediate reports of the CMP session handler such as "sending ir" and "received response" should be output right away.
Otherwise, when a CMP transaction, which may involve multiple request/response pairs, gets stuck the user or calling application does not obtain any indication how far the transaction has been able to proceed.

Ok, for timely output (such as INFO: got response) that is more for experimenting and debugging we can make use of the existing trace API (though it unfortunately needs to be enabled statically (e.g., using ./config enable-trace).

Therefore I suppose the solutions sketched above should be sufficient and not hard to implement.
@mattcaswell, do you agree or do you have further comments/advice?
We'd like to continue&finish implementing this tomorrow.

@mattcaswell, do you agree or do you have further comments/advice?

It sounds reasonable to me.

Another option could be to extend the existing error queue mechanism to support warnings (and possibly further levels of severity/verbosity).

I'm unsure about that option. Probably it strays a little far from the original scope of what we are attempting to do here.

A technique used in several other parts of OpenSSL is to allow the application to register a callback that gets progress information. For example while generating prime numbers...

A technique used in several other parts of OpenSSL is to allow the application to register a callback that gets progress information. For example while generating prime numbers...

Yeah, this is what we have been doing so far, but then Matt suggested using the trace API.
Which I tried, but then it turned out that it is disabled by default due to data sensitivity concerns.
So we could either go back to the original (callback-based) solution or do the mix of solutions sketched above.

For what my opinion is worth, I think using common practice rather than a mix of solutions will be easier for consumers to understand.

@mattcaswell, would it be ok for you if we revert to the original version,
which uses a pointer of the following type in CMP_CTX?

typedef int (*OSSL_cmp_log_cb_t) (const char *component,
                                  const char *file, int lineno,
                                  OSSL_CMP_severity level, const char *msg);

@mattcaswell, can we go back to using the CMP-specific logging callback?
Would be nice if you could provide your view within an hour or so since we need to know which way to proceed to today, and progress with this chunk 4 and the upcoming preview of chunk 5 is currently blocked due to the open choice/decision.
Like Rich, we'd prefer the callback variant for its flexibility and convenience of use.

Pushed to master! Thanks all!

65-test_cmp_ctx is causing some test failures on some local test machines when enable-trace option is specified.

1..1
# Subtest: /home/nse/workspace/openssl-linux_x64_GCC/build-linux_x64_gcc/test/cmp_ctx_test
1..46
ok 1 - test_CTX_reinit
ok 2 - test_CTX_set_get_option_16
ok 3 - test_CTX_set_get_log_cb
# ERROR: (int) 'test_log_cb_res == 1' failed @ ../test/cmp_ctx_test.c:276
# [0] compared to [1]
not ok 4 - test_cmp_ctx_log_cb
E0:E6:24:BE:7C:7F:00:00:error:CMP routines:(unknown function):multiple san sources:../test/cmp_ctx_test.c:139:
ok 5 - test_CTX_print_errors

===============

/usr/local/bin/perl ../Configure --prefix=/home/nse/workspace/openssl-linux_x64_GCC/install shared enable-trace linux-x86_64

Perl information:

/usr/local/bin/perl
5.24.0 for x86_64-linux

Enabled features:

aria
asm
async
autoalginit
autoerrinit
autoload-config
bf
blake2
camellia
capieng
cast
chacha
cmac
cmp
cms
comp
ct
deprecated
des
dgram
dh
dsa
dso
dtls
dynamic-engine
ec
ec2m
ecdh
ecdsa
engine
err
filenames
fips
gost
idea
legacy
makedepend
md4
mdc2
module
multiblock
nextprotoneg
pinshared
ocb
ocsp
padlockeng
pic
poly1305
posix-io
psk
rc2
rc4
rdrand
rfc3779
rmd160
scrypt
seed
shared
siphash
siv
sm2
sm3
sm4
sock
srp
srtp
sse2
ssl
static-engine
stdio
tests
threads
tls
trace
ts
ui-console
whirlpool
tls1
tls1-method
tls1_1
tls1_1-method
tls1_2
tls1_2-method
tls1_3
dtls1
dtls1-method
dtls1_2
dtls1_2-method

Disabled features:

ktls                    [default]        OPENSSL_NO_KTLS
afalgeng                [too-old-kernel] 
asan                    [default]        OPENSSL_NO_ASAN
buildtest-c++           [default]        
crypto-mdebug           [default]        OPENSSL_NO_CRYPTO_MDEBUG
crypto-mdebug-backtrace [default]        OPENSSL_NO_CRYPTO_MDEBUG_BACKTRACE
devcryptoeng            [default]        OPENSSL_NO_DEVCRYPTOENG
ec_nistp_64_gcc_128     [default]        OPENSSL_NO_EC_NISTP_64_GCC_128
egd                     [default]        OPENSSL_NO_EGD
external-tests          [default]        OPENSSL_NO_EXTERNAL_TESTS
fuzz-libfuzzer          [default]        OPENSSL_NO_FUZZ_LIBFUZZER
fuzz-afl                [default]        OPENSSL_NO_FUZZ_AFL
md2                     [default]        OPENSSL_NO_MD2 (skip crypto/md2)
msan                    [default]        OPENSSL_NO_MSAN
rc5                     [default]        OPENSSL_NO_RC5 (skip crypto/rc5)
sctp                    [default]        OPENSSL_NO_SCTP
ssl-trace               [default]        OPENSSL_NO_SSL_TRACE
ubsan                   [default]        OPENSSL_NO_UBSAN
unit-test               [default]        OPENSSL_NO_UNIT_TEST
uplink                  [no uplink_arch] OPENSSL_NO_UPLINK
weak-ssl-ciphers        [default]        OPENSSL_NO_WEAK_SSL_CIPHERS
zlib                    [default]        
zlib-dynamic            [default]        
ssl3                    [default]        OPENSSL_NO_SSL3
ssl3-method             [default]        OPENSSL_NO_SSL3_METHOD

Config target attributes:

AR => "ar",
ARFLAGS => "r",
CC => "gcc",
CFLAGS => "-Wall -O3",
CXX => "g++",
CXXFLAGS => "-Wall -O3",
HASHBANGPERL => "/usr/bin/env perl",
RANLIB => "ranlib",
RC => "windres",
asm_arch => "x86_64",
bn_ops => "SIXTY_FOUR_BIT_LONG",
build_file => "Makefile",
build_scheme => [ "unified", "unix" ],
cflags => "-pthread -m64",
cppflags => "",
cxxflags => "-std=c++11 -pthread -m64",
defines => [  ],
disable => [  ],
dso_ldflags => "-z defs",
dso_scheme => "dlfcn",
enable => [ "afalgeng" ],
ex_libs => "-ldl -pthread",
includes => [  ],
lflags => "",
lib_cflags => "",
lib_cppflags => "-DOPENSSL_USE_NODELETE -DL_ENDIAN",
lib_defines => [  ],
module_cflags => "-fPIC",
module_cxxflags => "",
module_ldflags => "-Wl,-znodelete -shared -Wl,-Bsymbolic",
multilib => "64",
perl_platform => "Unix",
perlasm_scheme => "elf",
shared_cflag => "-fPIC",
shared_defflag => "-Wl,--version-script=",
shared_defines => [  ],
shared_ldflag => "-Wl,-znodelete -shared -Wl,-Bsymbolic",
shared_rcflag => "",
shared_sonameflag => "-Wl,-soname=",
shared_target => "linux-shared",
thread_defines => [  ],
thread_scheme => "pthreads",
unistd => "<unistd.h>",

Recorded environment:

AR = 
ARFLAGS = 
AS = 
ASFLAGS = 
BUILDFILE = 
CC = 
CFLAGS = 
CPP = 
CPPDEFINES = 
CPPFLAGS = 
CPPINCLUDES = 
CROSS_COMPILE = 
CXX = 
CXXFLAGS = 
HASHBANGPERL = 
LD = 
LDFLAGS = 
LDLIBS = 
MT = 
MTFLAGS = 
OPENSSL_LOCAL_CONFIG_DIR = 
PERL = 
RANLIB = 
RC = 
RCFLAGS = 
RM = 
WINDRES = 
__CNF_CFLAGS = 
__CNF_CPPDEFINES = 
__CNF_CPPFLAGS = 
__CNF_CPPINCLUDES = 
__CNF_CXXFLAGS = 
__CNF_LDFLAGS = 
__CNF_LDLIBS = 

Makevars:

AR              = ar
ARFLAGS         = r
CC              = gcc
CFLAGS          = -Wall -O3
CPPDEFINES      = 
CPPFLAGS        = 
CPPINCLUDES     = 
CXX             = g++
CXXFLAGS        = -Wall -O3
HASHBANGPERL    = /usr/bin/env perl
LDFLAGS         = 
LDLIBS          = 
PERL            = /usr/local/bin/perl
RANLIB          = ranlib
RC              = windres
RCFLAGS         = 

@slontis, I wonder why your test failure issue pops up here, after this PR has already been merged.
I cannot reproduce it with the current OpenSSL master and (essentially) the configuration you mention:

./Configure shared enable-trace linux-x86_64 

No problems.. I will debug it on one of the machines it fails on (hopefully without using printf :) )
Just asking in case something stood out as being a obvious issue.
I also could not get it to happen on my local machine.