Skip to content

Replumbing: add fallback provider capability#8480

Closed
levitte wants to merge 17 commits intoopenssl:masterfrom
levitte:add-fallback-provider-capability
Closed

Replumbing: add fallback provider capability#8480
levitte wants to merge 17 commits intoopenssl:masterfrom
levitte:add-fallback-provider-capability

Conversation

@levitte
Copy link
Member

@levitte levitte commented Mar 14, 2019 •
edited
Loading

To ensure that old applications aren't left without any provider, and
at the same time not forcing any default provider on applications that
know how to deal with them, we device the concept of fallback
providers, which are automatically activated if no other provider is
already activated.

We fully expect to see something like this in libcrypto initialization:

OSSL_PROVIDER *prov = NULL;

if ((prov = ossl_provider_new(NULL, "default",
                              default_provider_init)) != NULL
    && ossl_provider_set_fallback(prov))
    ...

UPDATE: prepopulating with whatever default fallbacks now happens
whenever a new provider store is created. This automatically ties it to
the library context being used.

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

@levitte levitte added the branch: master Applies to master branch label Mar 14, 2019
mattcaswell
mattcaswell previously approved these changes Mar 14, 2019
View reviewed changes
@levitte levitte dismissed mattcaswell’s stale review March 14, 2019 10:21

The code wasn't quite right yet

@levitte levitte changed the title WIP: Replumbing: add fallback provider capability Replumbing: add fallback provider capability Mar 14, 2019
@levitte
Copy link
Member Author

levitte commented Mar 14, 2019

Test added. Let's see what the CIs think of it

@mattcaswell
Copy link
Member

mattcaswell commented Mar 14, 2019

Travis failure looks relevant:

../../_srcdist/test/recipes/02-test_internal_provider.t ........ 

1..1

    # Subtest: ../../test/provider_internal_test

    1..1

    ok 1 - test_builtin_provider

provider_internal_test(34828,0x7fffab0ce340) malloc: *** error for object 0x7fb6b24101d0: incorrect checksum for freed object - object was probably modified after being freed.

*** set a breakpoint in malloc_error_break to debug

../../util/shlib_wrap.sh ../../test/provider_internal_test => 134

not ok 1 - running provider_internal_test

@mattcaswell
Copy link
Member

mattcaswell commented Mar 14, 2019

Aside from the above issue this looks ok.

@mattcaswell
Copy link
Member

mattcaswell commented Mar 14, 2019

We fully expect to see something like this in libcrypto initialization:

Just wanted to point out that this has to happen on a per OPENSSL_CTX basis

@levitte
Copy link
Member Author

levitte commented Mar 14, 2019

Just wanted to point out that this has to happen on a per OPENSSL_CTX basis

True... and that's going to be interesting, 'cause I assume some of the global init stuff might actually move to library context init. But either way, NULL in the ossl_provider_new call is that argument, where NULL is the default library context, and could as well be an actual application allocated library context.

@levitte
Copy link
Member Author

levitte commented Mar 14, 2019

Re the Travis failure, I did have some vague notion that this might happen. I believe my test program is at fault...

@levitte levitte force-pushed the add-fallback-provider-capability branch from c6d7b43 to f617092 Compare March 15, 2019 08:46
@levitte
Copy link
Member Author

levitte commented Mar 15, 2019

I've remove the changes that I did in test/provider_internal_test.c because the fallback mechanism isn't testable with just the internal API. We cannot really test it before we have some minimal public API that uses providers in place, i.e. when we start having a default provider.

@levitte levitte force-pushed the add-fallback-provider-capability branch 2 times, most recently from 7bff220 to 0f5e168 Compare March 17, 2019 17:18
@levitte
Copy link
Member Author

levitte commented Mar 17, 2019

@mattcaswell, you might be interested in the last development... I just added a mechanism to add predefined providers, which is especially interesting for fallback providers.

(yes, I know that we currently plan on just one fallback provider, but who knows what we decide in the future? We might want to break down the default by algorithm...)

paulidale
paulidale reviewed Mar 18, 2019
View reviewed changes
paulidale
paulidale reviewed Mar 18, 2019
View reviewed changes
@levitte levitte force-pushed the add-fallback-provider-capability branch from fbae435 to a51e2b7 Compare March 18, 2019 09:53
@mattcaswell mattcaswell mentioned this pull request Mar 18, 2019
Closed
mattcaswell
mattcaswell requested changes Mar 18, 2019
View reviewed changes
@levitte levitte force-pushed the add-fallback-provider-capability branch from 1edf5ac to 2399e5a Compare March 18, 2019 13:19
@levitte levitte requested a review from mattcaswell March 18, 2019 15:28
@levitte levitte force-pushed the add-fallback-provider-capability branch 2 times, most recently from 2692f34 to 8eefefc Compare March 18, 2019 15:50
@mattcaswell
Copy link
Member

mattcaswell commented Mar 18, 2019

Looks like there is a "make update" discrepancy (hence the travis red cross)

@levitte
Copy link
Member Author

levitte commented Mar 18, 2019

Looks like there is a "make update" discrepancy (hence the travis red cross)

Ah, right. Fixing it now

@levitte levitte force-pushed the add-fallback-provider-capability branch from 8eefefc to eb8693f Compare March 18, 2019 16:32
mattcaswell
mattcaswell requested changes Mar 18, 2019
View reviewed changes
@levitte levitte requested a review from mattcaswell March 18, 2019 17:41
mattcaswell
mattcaswell requested changes Mar 19, 2019
View reviewed changes
levitte added 8 commits March 19, 2019 11:34
@levitte levitte force-pushed the add-fallback-provider-capability branch from 08ce271 to 04bf075 Compare March 19, 2019 10:34
levitte added 9 commits March 19, 2019 11:56
@levitte
fixup! Replumbing: add fallback provider capability
67c5772
@levitte
Replumbing: Add a mechanism to pre-populate the provider store
03901e4 
OpenSSL will come with a set of well known providers, some of which
need to be accessible from the start.  These are typically built in
providers, or providers that will work as fallbacks.

We do this when creating a new provider store, which means that this
will happen in every library context, regardless of if it's the global
default one, or an explicitely created one.

We keep the data about the known providers we want to make accessible
this way in crypto/provider_predefined.h, which may become generated.
For now, though, we make it simple and edited manually.
@levitte
fixup! Replumbing: Add a mechanism to pre-populate the provider store
64ef38f
@levitte
fixup! Replumbing: Add a mechanism to pre-populate the provider store
191d2d6
@levitte
fixup! Replumbing: Add a mechanism to pre-populate the provider store
0f5dd76
@levitte
fixup! Replumbing: Add a mechanism to pre-populate the provider store
601f95b
@levitte
fixup! Replumbing: Add a mechanism to pre-populate the provider store
c6f4bb6
@levitte
fixup! Replumbing: Add a mechanism to pre-populate the provider store
eaa3935
@levitte
fixup! Replumbing: Add a mechanism to pre-populate the provider store
5cbc484
@levitte levitte force-pushed the add-fallback-provider-capability branch from 04bf075 to 5cbc484 Compare March 19, 2019 10:56
@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Mar 19, 2019
@levitte
Copy link
Member Author

levitte commented Mar 19, 2019

Merged. Thanks for the review!

c41f3ae Replumbing: Add a mechanism to pre-populate the provider store
e55008a Replumbing: add fallback provider capability

@levitte levitte closed this Mar 19, 2019
levitte added a commit that referenced this pull request Mar 19, 2019
@levitte
Replumbing: add fallback provider capability
e55008a 
To ensure that old applications aren't left without any provider, and
at the same time not forcing any default provider on applications that
know how to deal with them, we device the concept of fallback
providers, which are automatically activated if no other provider is
already activated.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from #8480)
levitte added a commit that referenced this pull request Mar 19, 2019
@levitte
Replumbing: Add a mechanism to pre-populate the provider store
c41f3ae 
OpenSSL will come with a set of well known providers, some of which
need to be accessible from the start.  These are typically built in
providers, or providers that will work as fallbacks.

We do this when creating a new provider store, which means that this
will happen in every library context, regardless of if it's the global
default one, or an explicitely created one.

We keep the data about the known providers we want to make accessible
this way in crypto/provider_predefined.h, which may become generated.
For now, though, we make it simple and edited manually.

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

Reviewers

@paulidale paulidale paulidale left review comments

@mattcaswell mattcaswell mattcaswell approved these changes

+1 more reviewer

@richsalz richsalz richsalz left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@levitte @mattcaswell @richsalz @paulidale