Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add extern C functions for ENGINE to use hardware private keys #1480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TheEnbyperor
Copy link

Needed this functions to use PKCS#11 to access private keys using libp11, so am now suggesting they're merged into upstream.

More work would be needed to expose a safe interface for these, although given that loading an engine results in telling openssl to randomly open .so files I'm sure a safe interface is even possible / desirable.

Hope these are useful contributions.

@sfackler
Copy link
Owner

It looks like you'll need to add another header here to get systest compiling: https://github.com/sfackler/rust-openssl/blob/master/systest/build.rs#L47


#[repr(C)]
pub struct UI_STRING {
string_type: UI_string_types,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to just use a c_int here since it's not publicly exposed, which also avoids potential UB issues if new variants are added to UI_string_types on OpenSSL's end.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is documented on the openssl website (https://www.openssl.org/docs/man1.1.1/man3/UI_string_types.html) so I assumed this is something that should be public.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But none of the fields in the structs you've defined are public.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, my bad, will fix!

@TheEnbyperor TheEnbyperor force-pushed the master branch 2 times, most recently from 6a34b60 to 3ea7e49 Compare May 30, 2021 21:56
@artemist
Copy link

On my system UI_METHOD is an incomplete type. I have a quick fix in my repo at artemist@43af0af but I don't believe this is ready for upstream (I've only tested one OpenSSL version)

@taikulawo
Copy link
Contributor

Same here, any progress? We need openssl engine do some intel QAT offload

@larrydewey
Copy link

Would there be any objections if I were to pick this up and finish the work here?

@IniterWorker
Copy link

It seems like the previous struct comment is missing:

/* .... */

/*
 * Duplicate the ui_data that often comes alongside a ui_method. This
 * allows some backends to save away UI information for later use.
 */
void *(*ui_duplicate_data)(UI *ui, void *ui_data);
void (*ui_destroy_data)(UI *ui, void *ui_data);

/* .... */

/*
 * UI_METHOD specific application data.
 */
CRYPTO_EX_DATA ex_data;

See OpenSSL.

I checked the rest, and I think with the current state of this crate, we have everything to test it. The remaining question is: do we need to provide a safe interface that will take care of allocating and deallocating the resources?

@IniterWorker
Copy link

IniterWorker commented Aug 3, 2024

It looks like you'll need to add another header to get systest compiling: build.rs#L51.

Specifically, it is missing openssl/engine.h.

Details

I am working a bit on this and hit a wall addressing the support of the UI_METHOD type.

EDIT: I think, we should create ui.rs handwritten types to sustain the implementation of engine.

EDIT2: I made some code starter on https://github.com/IniterWorker/rust-openssl/tree/feature/engine. If you have comments, I could maybe open a new PR.

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.

6 participants