Skip to content

Replumbing: re-implement error reporting for providers#9174

Closed
levitte wants to merge 10 commits intoopenssl:masterfrom
levitte:provider-error-reporting
Closed

Replumbing: re-implement error reporting for providers#9174
levitte wants to merge 10 commits intoopenssl:masterfrom
levitte:provider-error-reporting

Conversation

@levitte
Copy link
Member

@levitte levitte commented Jun 18, 2019

The idea is that providers should only have to report a reason code.
The library code is considered to be libcrypto internal, and are
allocated dynamically and automatically for providers on creation.

We reserve the upper 8 bits of the reason code for internal OpenSSL
use. This allows our own providers to report errors in form of a
packed number that includes library number, function number and
reason number.

With this, a provider can potentially use any reason number it wants
from 1 to 16777216, although the current error semantics really only
allow 1 to 4095 (because only the lower 12 bits are currently
considered an actual reason code by the ERR subsystem).

A provider can provide a reason string table in form of an array of
ERR_STRING_DATA, with each item containing just the reason code and
the associated string, with the dispatch function numbered
OSSL_FUNC_PROVIDER_GET_REASON_STRINGS matching the type
OSSL_provider_get_reason_strings_fn.
If available, libcrypto will call that function on provider
activation.


Also added the FIPS module adaptation.

@levitte levitte added the branch: master Applies to master branch label Jun 18, 2019
@levitte
Copy link
Member Author

levitte commented Jun 18, 2019

Note that this seems to conflict with #8887, but only partially. The change of semantics for ERR_add_error_data there is fine, but the provider adaptation isn't.

@levitte levitte force-pushed the provider-error-reporting branch from 4513aa3 to 6ab4a13 Compare June 18, 2019 12:29
mattcaswell
mattcaswell previously approved these changes Jun 18, 2019
@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Jun 18, 2019
@levitte
Copy link
Member Author

levitte commented Jun 18, 2019

Argh, too early approval... I realised that we need to add a string for the lib code (prov->error_lib). I need to go away for a little bit, so that won't happen for one or two hours yet.

@mattcaswell mattcaswell removed the approval: done This pull request has the required number of approvals label Jun 18, 2019
@mattcaswell mattcaswell dismissed their stale review June 18, 2019 17:49

@levitte not done yet

@levitte levitte force-pushed the provider-error-reporting branch from 30d79e3 to 42e4b54 Compare June 18, 2019 19:31
@levitte
Copy link
Member Author

levitte commented Jun 18, 2019

Now added the code I intended.

@levitte levitte force-pushed the provider-error-reporting branch from 1323102 to 03c4178 Compare June 18, 2019 21:36
@levitte
Copy link
Member Author

levitte commented Jun 18, 2019

... and hopefully cleared the last Travis issue

@levitte levitte force-pushed the provider-error-reporting branch from 03c4178 to e75fbbd Compare June 19, 2019 11:28
levitte added 9 commits June 19, 2019 16:22
The idea is that providers should only have to report a reason code.
The library code is considered to be libcrypto internal, and are
allocated dynamically and automatically for providers on creation.

We reserve the upper 8 bits of the reason code for internal OpenSSL
use.  This allows our own providers to report errors in form of a
packed number that includes library number, function number and
reason number.

With this, a provider can potentially use any reason number it wants
from 1 to 16777216, although the current error semantics really only
allow 1 to 4095 (because only the lower 12 bits are currently
considered an actual reason code by the ERR subsystem).

A provider can provide a reason string table in form of an array of
ERR_STRING_DATA, with each item containing just the reason code and
the associated string, with the dispatch function numbered
OSSL_FUNC_PROVIDER_GET_REASON_STRINGS matching the type
OSSL_provider_get_reason_strings_fn.
If available, libcrypto will call that function on provider
activation.
The FIPS module inner provider doesn't need to deal with error reason
strings or error library number, since it uses the outer provider's
error reporting upcalls.  We therefore disable that code in
crypto/provider_core.c when building the FIPS module.
@levitte levitte force-pushed the provider-error-reporting branch from e75fbbd to 268180f Compare June 19, 2019 14:23
@levitte
Copy link
Member Author

levitte commented Jun 19, 2019

A rebase may make Travis happier...

@levitte
Copy link
Member Author

levitte commented Jun 22, 2019

Ping

1 similar comment
@levitte
Copy link
Member Author

levitte commented Jul 2, 2019

Ping

@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Jul 2, 2019
@levitte
Copy link
Member Author

levitte commented Jul 2, 2019

Merged.

6592ab8 FIPS module: adapt for the changed error reporting methods
6ebc2f5 Replumbing: re-implement error reporting for providers

@levitte levitte closed this Jul 2, 2019
levitte added a commit that referenced this pull request Jul 2, 2019
The idea is that providers should only have to report a reason code.
The library code is considered to be libcrypto internal, and are
allocated dynamically and automatically for providers on creation.

We reserve the upper 8 bits of the reason code for internal OpenSSL
use.  This allows our own providers to report errors in form of a
packed number that includes library number, function number and
reason number.

With this, a provider can potentially use any reason number it wants
from 1 to 16777216, although the current error semantics really only
allow 1 to 4095 (because only the lower 12 bits are currently
considered an actual reason code by the ERR subsystem).

A provider can provide a reason string table in form of an array of
ERR_STRING_DATA, with each item containing just the reason code and
the associated string, with the dispatch function numbered
OSSL_FUNC_PROVIDER_GET_REASON_STRINGS matching the type
OSSL_provider_get_reason_strings_fn.
If available, libcrypto will call that function on provider
activation.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from #9174)
levitte added a commit that referenced this pull request Jul 2, 2019
The FIPS module inner provider doesn't need to deal with error reason
strings or error library number, since it uses the outer provider's
error reporting upcalls.  We therefore disable that code in
crypto/provider_core.c when building the FIPS module.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from #9174)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants