restrict libctx test to oqsprovider #620

baentsch · 2025-01-09T07:13:39Z

Restrict libctx test to oqsprovider as it is using oqsprovider specific features

Signed-off-by: Michael Baentsch <[email protected]>

t8m

LGTM

t8m · 2025-01-09T09:01:52Z

test/oqs_test_libctx.c

-                     EVP_PKEY_keygen_init(ctx) && EVP_PKEY_generate(ctx, key);
+        testresult =
+            (ctx = EVP_PKEY_CTX_new_from_name(
+                 libctx, kemalg_name, "provider=oqsprovider")) != NULL &&


Due to the CI failure, you will need to use "?provider=oqsprovider" instead.

Nope - the test will fail (later on) if oqsprovider cannot load. It'd be much better to completely skip the test in such case. But then again, what's the point in testing a provider that cannot load?

I do not understand what exactly fails? You mean that you do some negative testing without oqsprovider ~~not~~ loaded? If so, these tests are really useless and should be dropped.

The test for correctness of classical key parts in hybrid keys will fail if oqsprovider cannot load (see here) as it is only oqsprovider that has parameters that permit this check.

Ah, I was confused with the aarch64-cross-compilation test failure.

ok to resolve then such as to merge in case someone from the OQS team gets around to approving this?

Just a minor comment: The property query of restricting to oqsprovider should also be employed within the signature generation procedure (here), right?

Good catch @RodriM11 . Thanks!

Signed-off-by: Michael Baentsch <[email protected]>

RodriM11

Two additional comments regarding property queries on Sig operations

RodriM11 · 2025-01-10T17:16:00Z

test/oqs_test_libctx.c

-        testresult = (ctx = EVP_PKEY_CTX_new_from_name(libctx, sigalg_name,
-                                                       NULL)) != NULL &&
+        testresult = (ctx = EVP_PKEY_CTX_new_from_name(
+                          libctx, sigalg_name, OQSPROV_PROPQ)) != NULL &&
                     EVP_PKEY_keygen_init(ctx) && EVP_PKEY_generate(ctx, key) &&
                     (mdctx = EVP_MD_CTX_new()) != NULL &&
                     EVP_DigestSignInit_ex(mdctx, NULL, "SHA512", libctx, NULL,


I guess the OQSPROV_PROPQ should also be present on the EVP_DigestSignInit_ex/EVP_DigestVerifyInit_ex, right?

No: oqsprovider does not contain SHA implementations.

Yes, that was my doubt as well, because documentation clearly states that "If the provider supports fetching the digest then it may use the props argument for the properties to be used during the fetch". But then I saw that the property query is also being used in here, which is an operation where I would assume the property query provider=oqsprovider would be desired (as it is done in the KEM tests, for example). But, if I understand correctly, oqsprovider will later fetch the digest to use without that property query (here), although is true that openssl will at first try to use the property query to fetch the digest (here) and that could pose problematic, specially in the future. I'm sure I am missing something, so I apologize for the confusion I might be creating.

No confusion nor need to apologize -- I'm in fact very glad that someone else but just me checks these code paths! In this case, though, I do not see a path leading to something other than the loading of a digest. And that must be fulfilled by something other than oqsprovider (default or fips provider, notably -- whatever is loaded into the NULL/default libctx). If you see a path to something different, please let me know (where): Thanks in advance!

My point is that the property query passed to the EVP_DigestSignInit_ex (or analogue for Verify) is not only used for the query properties of the digest (which, of course, should not be fetched from oqsprovider), it is also employed in the underlying EVP_PKEY_CTX *of the EVP_MD_CTX * to use (see here), and if the property query provider=oqsprovider is desired on that EVP_PKEY_CTX * (like, for example, is employed in the "analogue" for Encapsulation/Decapsulation in KEMs (e.g. here), then there would indeed be a "problem". But I'm sure there is a reason why on that particular EVP_PKEY_CTX * the requirement of provider=oqsprovider is not needed.

I'm sure there is a reason why on that particular EVP_PKEY_CTX * the requirement of provider=oqsprovider is not needed.

Well, I'm not so sure now as your argument looks plausible. The only "reason" I see is that tests of this logic within the feature branch with standardized PQC algs passed that didn't pass before. Unfortunately, due to another problem with ML-DSA all tests are breaking now and I think it is best to wait with further work until the feature/ml-dsa branch is functionally far enough to serve as guidance for an update of oqsprovider with regard to the openssl naming.

I agree. I imagine is not a problem now, but it could be once the default provider has an implementation of the same signature algorithm as the one someone would be working with on oqsprovider (like it will soon happen).
Additionally, on a (quick) look, if you interpret the underlying OpenSSL procedure do_sigver_init , it seems like it does not account for the hypothetical of having different providers for the pkey operation and the digest to employ (if different providers have both implementations of one of them), since are both being impacted from the same property query, and this could potentially be used to restrict from which to take each implementation.
And I imagine there is little oqsprovider could do on that regard, as you cannot avoid the prop query received (since it might be any other thing like fips, etc...). The only possibility I see would maybe be to have an OSSL_PARAM for this fact, but even with that, do_sigver_init is using the same propq for two different purposes (which now, since they are maintaining some legacy code, is not a failure (the hash will be fetch with a digestByName approach) but it might be in the future.

"?provider=oqsprovider" query could be used in this case to prefer the algorithm implementation from oqsprovider if available there.

In this scenario I agree that this would solve the problem: since oqsprovider does not have any digest implementation, the digest fetch would be done via the default provider, while the pkey operation would be given priority to oqsprovider.
What I was pointing out is that openssl code would not account for the existance of two providers both of which have an implementation of the same digest and pkey operation to be used since they are both affected from the same property queries, but (in the case that is of interest) it is not a matter of oqsprovider in any case.

RodriM11 · 2025-01-10T17:16:18Z

test/oqs_test_signatures.c

-            (ctx = EVP_PKEY_CTX_new_from_name(
-                 libctx, sigalg_name, "provider=oqsprovider")) != NULL &&
+            (ctx = EVP_PKEY_CTX_new_from_name(libctx, sigalg_name,
+                                              OQSPROV_PROPQ)) != NULL &&
            EVP_PKEY_keygen_init(ctx) && EVP_PKEY_generate(ctx, &key) &&
            (mdctx = EVP_MD_CTX_new()) != NULL &&
            EVP_DigestSignInit_ex(mdctx, NULL, "SHA512", libctx, NULL, key,


Same here :-) No, Thanks.

baentsch added 2 commits January 9, 2025 08:11

limit libctx test to oqsprovider

32de7e2

Signed-off-by: Michael Baentsch <[email protected]>

format fixup

f62d9a5

Signed-off-by: Michael Baentsch <[email protected]>

baentsch mentioned this pull request Jan 9, 2025

Provide alias names for ML-KEM algorithms without dashes openssl/openssl#26328

Closed

baentsch requested a review from a team January 9, 2025 07:15

baentsch changed the title ~~limit libctx test to oqsprovider~~ restrict libctx test to oqsprovider Jan 9, 2025

t8m reviewed Jan 9, 2025

View reviewed changes

SWilson4 approved these changes Jan 10, 2025

View reviewed changes

streamline test-propq

4ca62ac

Signed-off-by: Michael Baentsch <[email protected]>

RodriM11 reviewed Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

restrict libctx test to oqsprovider #620

restrict libctx test to oqsprovider #620

baentsch commented Jan 9, 2025

t8m left a comment

t8m Jan 9, 2025

baentsch Jan 9, 2025

t8m Jan 9, 2025 •

edited

Loading

baentsch Jan 10, 2025

t8m Jan 10, 2025

baentsch Jan 10, 2025

RodriM11 Jan 10, 2025

baentsch Jan 10, 2025

RodriM11 left a comment

RodriM11 Jan 10, 2025 •

edited

Loading

baentsch Jan 11, 2025

RodriM11 Jan 11, 2025

baentsch Jan 11, 2025

RodriM11 Jan 11, 2025 •

edited

Loading

baentsch Jan 11, 2025

RodriM11 Jan 11, 2025 •

edited

Loading

t8m Jan 22, 2025

RodriM11 Jan 22, 2025

RodriM11 Jan 10, 2025

baentsch Jan 11, 2025

restrict libctx test to oqsprovider #620

Are you sure you want to change the base?

restrict libctx test to oqsprovider #620

Conversation

baentsch commented Jan 9, 2025

t8m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t8m Jan 9, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RodriM11 left a comment

Choose a reason for hiding this comment

RodriM11 Jan 10, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RodriM11 Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RodriM11 Jan 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t8m Jan 9, 2025 •

edited

Loading

RodriM11 Jan 10, 2025 •

edited

Loading

RodriM11 Jan 11, 2025 •

edited

Loading

RodriM11 Jan 11, 2025 •

edited

Loading