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

Kerberos smart card credentials handling #143

Merged
merged 86 commits into from
Aug 25, 2023

Conversation

TheBestTvarynka
Copy link
Collaborator

Hello,
In this PR I've added Kerberos smart card credentials handling. Note: it doesn't contain actual smart card logon implementation or code for working with smart cards (they will be in other PRs).

Changes overview:

  • Added the "scard" feature to enable the smart card logon. This feature can be enabled only on Windows.
  • Improved the ffi bindings. Now users can pass the smart card credentials using the AcquireCredentialsHandleA/W function. How it works: the smart card credentials are always marshaled into a string that always starts with the "@" character.
  • Added a CredentialsBuffers, Credentials, SmartCardIdentityBuffers, and SmartCardIdentity structure to be able to handle any kind of credentials.
  • I didn't change the credentials_handle type of the CredSSP client because of backward compatibility with the IronRDP that doesn't support smart card logon yet.
  • Improved TsCredentials encoding. For the TsCredentials, TsSmartCardCreds, and TsCspDataDetail structures I use the picky-krb crate. See the corresponding PR: picky-krb: CredSSP credentials structures picky-rs#233.

Docs & references:

…oty context builder that allows us to transform builder into another with another authdata type
… handle builder that allows us to transform builder into another with another authdata type
…text builder that allows us to transform builder into another with another credshandle type
…sform function for the created builder;

feat(sspi): builder: acq_cred_handle: implemnt `.transform` method for the AcquireCredentialsHandleResult;
…nsform function for the created builder. Implement credentials_handle_mut function
…ass regular auth identity and smart card creds
… general CredentialsBuffers structure. replace custom ber encoding with structures from the picky-krb crate;
Cargo.toml Outdated Show resolved Hide resolved
@CBenoit CBenoit self-assigned this Aug 18, 2023
Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Looking good.

I didn't change the credentials_handle type of the CredSSP client because of backward compatibility with the IronRDP that doesn't support smart card logon yet.

You can introduce a breaking change if necessary. We’ll increase the version number as appropriate and fix downstream code (IronRDP). I guess it would not be too hard in this case.

Improved the ffi bindings. Now users can pass the smart card credentials using the AcquireCredentialsHandleA/W function. How it works: the smart card credentials are always marshaled into a string that always starts with the "@" character.

Any opinion? @awakecoding

ffi/Cargo.toml Show resolved Hide resolved
@TheBestTvarynka
Copy link
Collaborator Author

I improved the CredSspClient/Server. Now it uses a Credentials structure and can handle both types of credentials.

@@ -263,7 +265,7 @@ pub unsafe fn auth_data_to_identity_buffers_w(

// only marshaled smart card creds starts with '@' char
#[cfg(feature = "scard")]
if user[0] == b'@' {
if user[0] == b'@' && CredIsMarshaledCredentialW(user.as_ptr() as *const _) != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

calling CredIsMarshaledCredentialW should be enough, it checks for @@ under the hood anyway. maybe the only potential issue is that CredIsMarshaledCredentialW could return true for types other than 1

Copy link
Collaborator Author

@TheBestTvarynka TheBestTvarynka Aug 25, 2023

Choose a reason for hiding this comment

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

fixed

@CBenoit
Copy link
Member

CBenoit commented Aug 25, 2023

This is looking good to me. Do we need to publish a new version of the Rust crate, or should this wait for the follow-up PRs?

@TheBestTvarynka
Copy link
Collaborator Author

Wait for follow up prs. I have 2 more pull requests

@CBenoit CBenoit merged commit 6250e8a into kerberos-smart-card-dev Aug 25, 2023
40 checks passed
@CBenoit CBenoit deleted the kerberos-smart-card-credentials branch August 25, 2023 18:49
CBenoit pushed a commit that referenced this pull request Aug 29, 2023
This adds Kerberos smart card credentials handling.

Note: it doesn't contain actual smart card logon implementation or code
for working with smart cards (follow-up patches incoming).

- Adds the `scard` feature to enable the smart card logon. This feature is only supported on Windows.
- Improves the `ffi` bindings. Now users can pass the smart card credentials using the
  `AcquireCredentialsHandleA/W` function. How it works: the smart card credentials are always marshaled
  into a string that always starts with the "@" character.
- Adds a `CredentialsBuffers`, `Credentials`, `SmartCardIdentityBuffers`, and `SmartCardIdentity` structure
  to be able to handle any kind of credentials.
- Improves `TsCredentials` encoding. For the `TsCredentials`, `TsSmartCardCreds`, and `TsCspDataDetail`
  structures I use the `picky-krb` crate.

References:

* [MS-CSSP: TSCredentials](https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-cssp/94a1ab00-5500-42fd-8d3d-7a84e6c2cf03).
* [CredUnmarshalCredentialW](https://learn.microsoft.com/en-us/windows/win32/api/wincred/nf-wincred-credunmarshalcredentialw).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants