OSSL_PROVIDER_load_ex #21604

beldmit · 2023-07-31T13:38:29Z

to configure provider in load-time.
The tests are to be written, the documentation is also to be written.

Checklist

documentation is added or updated
tests are added or updated

beldmit · 2023-07-31T13:42:12Z

It's an attempt to implement #21350 function using available API.

@openssl/committers the comments wanted. I tried to implement as an extension of OPENSSL_provider_load but didn't find an easy option to provide configuration options.

crypto/provider_conf.c

beldmit · 2023-08-01T11:18:15Z

I've pushed a draft of API design for better clarity

t8m

My feeling is this really needs some active discussion within the OTC to find out what we want to do here. We should avoid requiring changes to existing providers if they would like to use this functionality if at all possible. I assume some changes will be inevitable if we want to support setting the parameters to providers after they are loaded & initialized but at least for the initial load it should be possible to use the current callback.

doc/prov_loadex.md

simo5

So having thought about this some more I think I agree with a OSSL_LIB_CTX_load_provider() call where the provider config is entirely provided via the params option.

However I am entirely against OSSL_PROVIDER_set_params_by_name(), I think mixing static and dynamic config or attempting to change config will be error prone, difficult to debug for all involved (openssl, provider and application developers), and ultimately not a good pattern.

simo5 · 2023-08-01T14:57:53Z

So having thought about this some more I think I agree with a OSSL_LIB_CTX_load_provider() call where the provider config is entirely provided via the params option.

If a provider is already loaded this call should fail, btw, up to the app to decide what to do with it.

However I am entirely against OSSL_PROVIDER_set_params_by_name(), I think mixing static and dynamic config or attempting to change config will be error prone, difficult to debug for all involved (openssl, provider and application developers), and ultimately not a good pattern.

In general I think if the config is in a config file users should tweak the config file.
And if apps load the provider dynamically they should just set the params right at load time.

simo5 · 2023-08-01T15:13:09Z

Note that if you decide something like OSSL_PROVIDER_set_params() is the way you want to go you really need to also add a OSSL_PROVIDER_settable_params() so the provider can tell you what params can be set, and be prepared for that list to change after initialization...
This way a provider can tell what is changeable after init...

beldmit · 2023-08-01T15:25:22Z

Thanks, added OSSL_PROVIDER_settable_params to the scenario

beldmit · 2023-08-02T15:56:45Z

OK, I realized how to pass the data to the provider in more or less standard way.
Hopefully will push the tests tomorrow.

beldmit · 2023-08-02T16:14:04Z

This test failure seems to be unrelated to the patch but may be of interest for @hlandau https://github.com/openssl/openssl/actions/runs/5741052478/job/15560296780?pr=21604

beldmit · 2023-08-18T13:04:15Z

@openssl/committers any review/feedback, please?

beldmit · 2023-08-22T14:55:08Z

Rebased to resolve the trivial conflict in util/libcrypto.num

openssl-machine · 2023-08-30T19:00:20Z

This pull request is ready to merge

beldmit · 2023-08-30T19:57:44Z

Merged. Thanks for the review!

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #21604)

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from openssl#21604)

mspncp

Hi @beldmit, sorry once again for my late review, I didn't manage to get it done earlier. Hope you find something useful in it nevertheless.

doc/designs/prov_loadex.md

mspncp · 2023-09-05T07:57:08Z

doc/designs/prov_loadex.md

+Several instances of the same provider in the same context using different
+section names, module names (e.g. via symlinks) and provider names. But unless
+the provider does not support some configuration options, the algorithms in
+this case will have the same `provider` property and the result of fetching is
+not determined. We strongly discourage against this trick.


The intention of this paragraph is not obvious to me. Is this a suggested or discouraged scenario?

Suggested change

Several instances of the same provider in the same context using different

section names, module names (e.g. via symlinks) and provider names. But unless

the provider does not support some configuration options, the algorithms in

this case will have the same `provider` property and the result of fetching is

not determined. We strongly discourage against this trick.

Several instances of the same provider can be loaded in the same context using different

section names, module names (e.g. via symlinks) and provider names. But unless

the provider supports some configuration options, the algorithms in

this case will have the same `provider` property and the result of fetching is

not determined. We strongly discourage against this trick.

Added, but is this indeed what you meant to say? (You may answer this question in PR #22029)

doc/designs/prov_loadex.md

mspncp · 2023-09-05T08:23:05Z

test/provider_test.c

+    OSSL_PARAM_BLD_push_utf8_string(bld, "greeting", custom_buf, strlen(custom_buf));
+    params = OSSL_PARAM_BLD_to_param(bld);
+
+    OSSL_PARAM_BLD_free(bld);


This test just passes an unknown parameter to the provider which gets ignored. So it does not actually prove the usefulnes of the api, it's just a "hello world" example.

Would it be possible to add a proof-of-concept test that loads the "default" or "fips" provider using extra parameters that actually changes the behaviour of the provider in a measurable (testable) effect?

The parameter is not unknown, but I understand your reasons and yes, a FIPS provider test would be better.

mspncp · 2023-09-05T08:47:50Z

doc/designs/prov_loadex.md

+We need a possibility to initialize providers on per-application level
+according to per-application parameters. It's necessary for example for PKCS#11


Looking at provider-base, it seems possible to dispense of the configuration file entirely using the new extended api.

Additionally, provider specific configuration parameters from the config file are available, in dotted name form. The dotted name form is a concatenation of section names and final config command name separated by periods.

If that's the case, it would be great to have a test case and/or example as a proof-of concept to show that this can be achieved using your new extended API.

Probably some example of its usage could also be added to the ossl-guide-libcrypto-introduction manpage?

(If you agree, I will create a separate issue for this)

I agree though I totally missed this point

doc/designs/prov_loadex.md

beldmit · 2023-09-07T12:23:29Z

@mspncp could you please raise your documentation updates as a PR?

Late review comments for pull request openssl#21604, sort of.

mspncp · 2023-09-08T10:46:18Z

See PR #22029. Please also reconsider the conversations which I did not mark as resolved yet.

Late review comments for pull request openssl#21604, sort of.

Late review comments for pull request #21604, sort of. Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #22029)

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 31, 2023

beldmit added branch: master Merge to master branch hold: needs tests The PR needs tests to be added to it hold: needs documentation The PR needs documentation to be added to it and removed severity: fips change The pull request changes FIPS provider sources labels Jul 31, 2023

mspncp reviewed Jul 31, 2023

View reviewed changes

crypto/provider_conf.c Outdated Show resolved Hide resolved

mattcaswell requested changes Jul 31, 2023

View reviewed changes

crypto/provider_conf.c Outdated Show resolved Hide resolved

crypto/provider_conf.c Outdated Show resolved Hide resolved

crypto/provider_conf.c Outdated Show resolved Hide resolved

github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Aug 1, 2023

t8m reviewed Aug 1, 2023

View reviewed changes

doc/prov_loadex.md Outdated Show resolved Hide resolved

doc/prov_loadex.md Outdated Show resolved Hide resolved

beldmit force-pushed the OSSL_PROVIDER_activate branch from 756dc6f to 25e0bfa Compare August 1, 2023 14:13

Jakuje reviewed Aug 1, 2023

View reviewed changes

doc/prov_loadex.md Outdated Show resolved Hide resolved

simo5 suggested changes Aug 1, 2023

View reviewed changes

beldmit force-pushed the OSSL_PROVIDER_activate branch 2 times, most recently from ece77c3 to a108c78 Compare August 1, 2023 15:23

beldmit force-pushed the OSSL_PROVIDER_activate branch 2 times, most recently from a0fcdec to 66c79cd Compare August 2, 2023 15:55

beldmit force-pushed the OSSL_PROVIDER_activate branch from e9e8369 to d83a0bd Compare August 3, 2023 11:22

beldmit changed the title ~~[DRAFT] OSSL_PROVIDER_activate - a strawman attempt~~ OSSL_PROVIDER_load_ex Aug 3, 2023

beldmit added 4 commits August 22, 2023 16:54

OSSL_PROVIDER_load_ex

5df70aa

OSSL_PROVIDER_load_ex tests

2fb73aa

Design document of the run-time parameters activation

31ad804

fixup! Design document of the run-time parameters activation

f5578c4

beldmit force-pushed the OSSL_PROVIDER_activate branch from 1553eac to f5578c4 Compare August 22, 2023 14:54

t-j-h approved these changes Aug 29, 2023

View reviewed changes

t-j-h added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Aug 29, 2023

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Aug 30, 2023

beldmit closed this Aug 30, 2023

beldmit deleted the OSSL_PROVIDER_activate branch August 30, 2023 19:57

openssl-machine pushed a commit that referenced this pull request Aug 30, 2023

OSSL_PROVIDER_load_ex

9d2f7e1

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #21604)

openssl-machine pushed a commit that referenced this pull request Aug 30, 2023

OSSL_PROVIDER_load_ex tests

4f3e3d9

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #21604)

openssl-machine pushed a commit that referenced this pull request Aug 30, 2023

Design document of the run-time parameters activation

9f5102b

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from #21604)

heygauri pushed a commit to heygauri/openssl that referenced this pull request Sep 1, 2023

OSSL_PROVIDER_load_ex

28f8799

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from openssl#21604)

heygauri pushed a commit to heygauri/openssl that referenced this pull request Sep 1, 2023

OSSL_PROVIDER_load_ex tests

2172f63

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from openssl#21604)

heygauri pushed a commit to heygauri/openssl that referenced this pull request Sep 1, 2023

Design document of the run-time parameters activation

feb543b

Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from openssl#21604)

mspncp reviewed Sep 5, 2023

View reviewed changes

mspncp added a commit to mspncp/openssl that referenced this pull request Sep 8, 2023

doc: Suggestions for OSSL_PROVIDER_load_ex design document

263cfbc

Late review comments for pull request openssl#21604, sort of.

mspncp added a commit to mspncp/openssl that referenced this pull request Sep 8, 2023

doc: suggestions for OSSL_PROVIDER_load_ex design document

3aa7ff7

Late review comments for pull request openssl#21604, sort of.

mspncp added a commit to mspncp/openssl that referenced this pull request Sep 8, 2023

doc: suggestions for OSSL_PROVIDER_load_ex design document

127f95b

Late review comments for pull request openssl#21604, sort of.

simo5 mentioned this pull request Oct 12, 2023

repart: support OpenSSL engines for signing systemd/systemd#29539

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSSL_PROVIDER_load_ex #21604

OSSL_PROVIDER_load_ex #21604

beldmit commented Jul 31, 2023 •

edited

beldmit commented Jul 31, 2023

beldmit commented Aug 1, 2023

t8m left a comment

simo5 left a comment •

edited

simo5 commented Aug 1, 2023

simo5 commented Aug 1, 2023

beldmit commented Aug 1, 2023

beldmit commented Aug 2, 2023

beldmit commented Aug 2, 2023

beldmit commented Aug 18, 2023

beldmit commented Aug 22, 2023

openssl-machine commented Aug 30, 2023

beldmit commented Aug 30, 2023

mspncp left a comment

mspncp Sep 5, 2023

mspncp Sep 8, 2023 •

edited

mspncp Sep 5, 2023

beldmit Sep 8, 2023

mspncp Sep 5, 2023

beldmit Sep 8, 2023

beldmit commented Sep 7, 2023

mspncp commented Sep 8, 2023

		We need a possibility to initialize providers on per-application level
		according to per-application parameters. It's necessary for example for PKCS#11

OSSL_PROVIDER_load_ex #21604

OSSL_PROVIDER_load_ex #21604

Conversation

beldmit commented Jul 31, 2023 • edited

Checklist

beldmit commented Jul 31, 2023

beldmit commented Aug 1, 2023

t8m left a comment

Choose a reason for hiding this comment

simo5 left a comment • edited

Choose a reason for hiding this comment

simo5 commented Aug 1, 2023

simo5 commented Aug 1, 2023

beldmit commented Aug 1, 2023

beldmit commented Aug 2, 2023

beldmit commented Aug 2, 2023

beldmit commented Aug 18, 2023

beldmit commented Aug 22, 2023

openssl-machine commented Aug 30, 2023

beldmit commented Aug 30, 2023

mspncp left a comment

Choose a reason for hiding this comment

mspncp Sep 5, 2023

Choose a reason for hiding this comment

mspncp Sep 8, 2023 • edited

Choose a reason for hiding this comment

mspncp Sep 5, 2023

Choose a reason for hiding this comment

beldmit Sep 8, 2023

Choose a reason for hiding this comment

mspncp Sep 5, 2023

Choose a reason for hiding this comment

beldmit Sep 8, 2023

Choose a reason for hiding this comment

beldmit commented Sep 7, 2023

mspncp commented Sep 8, 2023

beldmit commented Jul 31, 2023 •

edited

simo5 left a comment •

edited

mspncp Sep 8, 2023 •

edited