Skip to content

Conversation

@huwcbjones
Copy link
Contributor

@huwcbjones huwcbjones commented Aug 6, 2025

Work towards #2047 by adding ability to create PKeys from OSSL params.

Also:

  • tidy up creation of OSSL_PARAM_* on the way.
  • write some tests for the internal OsslParam* API.

@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch 6 times, most recently from 33d55eb to 6ec4f5e Compare August 6, 2025 16:58
@huwcbjones huwcbjones marked this pull request as ready for review August 6, 2025 17:33
@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch 5 times, most recently from ddfc4d6 to d5a03e9 Compare August 13, 2025 15:32
@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch from d5a03e9 to a87b819 Compare August 19, 2025 09:12
@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch from a87b819 to 5e26fd8 Compare August 26, 2025 11:01
@huwcbjones
Copy link
Contributor Author

@alex, any chance you could take a look at this PR?

@alex
Copy link
Collaborator

alex commented Aug 27, 2025

So, I'm not interested in including ossl params anywhere in the public API, I think they're a giant mistake and I hope OpenSSL gets rid of them.

I'd be ok:

  • With a PR that just adds the -sys bindings
  • Refactoring the private usage of OSSL_PARAMS to have a param builder or similar

But I'm not interested in any changes to the public APIs for this.

@huwcbjones
Copy link
Contributor Author

After working with params API to get things working in a non-deprecated way, I completely agree with you!

Regarding the comment about changes to the public APIs, there was a reason why I added the params mod as a non-public one, see: https://github.com/sfackler/rust-openssl/blob/5e26fd8465aa245c90cce7859d65817c34cc2ce4/openssl/src/lib.rs#L180-L181.
I left the visibility of everything else in that mod as pub because I was lazy. I can change it all to pub(crate) if you want?
Either way, I don't think I've changed the public API (and if I have, it was completely unintentional as I agree that the params stuff should be kept internal only).

@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch 3 times, most recently from 944b90d to e584932 Compare September 1, 2025 15:47
@huwcbjones
Copy link
Contributor Author

@alex since #2451 went in (which implemented essentially the same functionality I had in this PR a month ago), please could I get some eyes on this?

@alex
Copy link
Collaborator

alex commented Sep 11, 2025

(Merge conflict here, it's on my TODO to review Friday)

@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch 3 times, most recently from d8aec06 to 08d33f2 Compare September 11, 2025 10:01
@huwcbjones
Copy link
Contributor Author

huwcbjones commented Sep 11, 2025

After rebasing my non-deprecated branch, I've realised I can drop the merge implementation (not to mention the lifetimes were completely borked too)!
No I can't, I still need it for upgrading DH Params -> Public -> Private!

@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch 2 times, most recently from 2ce2e42 to f15f435 Compare September 12, 2025 14:54
@huwcbjones huwcbjones force-pushed the huw/evp-from-params branch 2 times, most recently from 38fa6a5 to 274160d Compare September 15, 2025 13:08
@huwcbjones
Copy link
Contributor Author

@alex did you manage to get a chance to look at this one?

@huwcbjones
Copy link
Contributor Author

@alex did you get around to having some time to look at this one please?

@huwcbjones
Copy link
Contributor Author

@alex apologies for the delay, had some higher priority stuff to look at. Please could you cast your eyes over this one?
(See #2464 for how these changes fit in)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants