feat(dpapi): WS tunneling support #410
Conversation
refactor(dpapi): apply code review suggestions refactor(dpapi): fix a typo in doc comment
refactor(dpapi): apply code review suggestions fix(dpapi-cli): fix the large futures issue
crates/dpapi-cli-client/src/main.rs
Outdated
|
|
||
| fn run(data: Dpapi) -> Result<()> { | ||
| async fn run(data: Dpapi) -> Result<()> { |
There was a problem hiding this comment.
I made all codebase async, because all WS/HTTP clients for WASM are async
|
@CBenoit When will you have a chance to review it? 🙂 |
|
@TheBestTvarynka I’ll review tomorrow! 🙂 |
|
@CBenoit I finished refactoring the code. Feel free to review whenever you want |
|
|
||
| /// Obtains the session token from the [tokengen server](https://github.com/Devolutions/devolutions-gateway/tree/master/tools/tokengen). | ||
| /// | ||
| /// Paramers: |
There was a problem hiding this comment.
typo
| /// Paramers: | |
| /// Parameters: |
tools/dpapi-cli-client/README.md
Outdated
| @@ -0,0 +1,5 @@ | |||
| # DPAPI cli client | |||
|
|
|||
| This is a simple DPAPI client for using the DPAPI. It can encrypt secrets or decrupt them using DPAPI. Run `dpapi-cli-client --help` for more information and usage manual. | |||
There was a problem hiding this comment.
| This is a simple DPAPI client for using the DPAPI. It can encrypt secrets or decrupt them using DPAPI. Run `dpapi-cli-client --help` for more information and usage manual. | |
| This is a simple DPAPI client for using the DPAPI. It can encrypt secrets or decrypt them using DPAPI. Run `dpapi-cli-client --help` for more information and usage manual. |
There was a problem hiding this comment.
I'm wondering why VS code didn't highlight it 🤔
There was a problem hiding this comment.
I'm wondering why VS code didn't highlight it 🤔
the spell checker extension was turned off...I don't know why. Anyway, I turned it on again and fixed many other typos. See this commit: b0856be
src/rustls.rs
Outdated
| pub(crate) fn install_default_crypto_provider_if_necessary() -> Result<(), ()> { | ||
| #[allow(clippy::result_unit_err)] | ||
| pub fn install_default_crypto_provider_if_necessary() -> Result<(), ()> { |
There was a problem hiding this comment.
issue: This is not intended to be part of the public API of sspi.
suggestion: Not ideal, but can you add #[doc(hidden)]? Let’s at least hide it.
src/lib.rs
Outdated
| mod rustls; | ||
| pub mod rustls; |
There was a problem hiding this comment.
issue: Let’s keep this module private. Following previous advice, re-export from lib.rs and ensure the item is properly hidden from the documentation.
ffi/src/winscard/scard_handle.rs
Outdated
| let scard_pci_info_len = unsafe { (*scard_io_request).cb_pci_length }.try_into()?; | ||
| let scard_pci_info_len = usize::try_from(unsafe { (*scard_io_request).cb_pci_length })?; |
There was a problem hiding this comment.
style: I recommend splitting on two lines instead of having the unsafe block inlined.
ffi/src/dpapi/session_token.rs
Outdated
| // * session_id is an object on stack. | ||
| // * destination is created (and validated) using `CString`. | ||
| // * token_buf is a non-empty Vec. | ||
| // * token len is a local variale. |
There was a problem hiding this comment.
| // * token len is a local variale. | |
| // * token len is a local variable. |
ffi/src/dpapi/session_token.rs
Outdated
| let mut token_len = 2048; | ||
| let mut token_buf = vec![0; 2048]; | ||
|
|
||
| // SAFETY: all function input parameters are valid because: |
There was a problem hiding this comment.
| // SAFETY: all function input parameters are valid because: | |
| // SAFETY: | |
| // As per safety preconditions, the C function pointer is safe to be called with valid parameters. | |
| // Parameters are valid: |
ffi/src/dpapi/session_token.rs
Outdated
| /// This function wraps a C-function into a Rust closure which we can pass into the Rust API. | ||
| pub fn session_token_fn(get_session_token: CGetSessiontokenFn) -> Box<GetSessionTokenFn> { |
There was a problem hiding this comment.
| /// This function wraps a C-function into a Rust closure which we can pass into the Rust API. | |
| pub fn session_token_fn(get_session_token: CGetSessiontokenFn) -> Box<GetSessionTokenFn> { | |
| /// This function wraps a C-function into a Rust closure which we can pass into the Rust API. | |
| /// | |
| /// # Safety | |
| /// | |
| /// The C function pointer must be safe to call provided parameters are valid. | |
| pub unsafe fn session_token_fn(get_session_token: CGetSessiontokenFn) -> Box<GetSessionTokenFn> { |
| name = "dpapi-native-transport" | ||
| version = "0.1.0" | ||
| edition = "2024" |
There was a problem hiding this comment.
suggestion: Can you add the required meta data needed for publishing crates?
There was a problem hiding this comment.
done (sorry for the delay, I had a meeting)
| @@ -1,7 +1,13 @@ | |||
| [package] | |||
| name = "dpapi-transport" | |||
| version = "0.1.0" | |||
| edition = "2024" | |||
| edition = "2021" | |||
There was a problem hiding this comment.
suggestion: Feel free to keep edition 2024 for new code. We’ll want to update to 2024 eventually.
There was a problem hiding this comment.
I just wanted to be consistent. I know that crates can have different edition versions, but my inner voice says that I want it to be the same for all crates in the workspace 😄
There was a problem hiding this comment.
That’s fair! As you prefer, anyway I don’t think you used many features from the 2024 edition yet 😉
Hi,
I implemented WS tunneling support in this PR.
I tested the DPAPI on the following scenarios:
Everything works well.