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

Smart card credentials handling #206

Merged
merged 20 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f7a2844
feat(ffi): sspi: add logs to handle emulated smart card creds
TheBestTvarynka Feb 5, 2024
9bfba4b
feat(winscard): derive Debug and Clone for SmartCard and ScardState
TheBestTvarynka Feb 5, 2024
5a9a7e2
feat(winscard): add SmartCardContext::auth_pk_pe field. Derive Debug …
TheBestTvarynka Feb 5, 2024
3134205
feat(sspi): add private key to smart card creds;
TheBestTvarynka Feb 5, 2024
e696f98
refactor(sspi): format code
TheBestTvarynka Feb 5, 2024
f904eba
feat(sspi): improve smart card creds passing. replaced winapi scard w…
TheBestTvarynka Feb 5, 2024
40a2003
feat(ffi): implement emulated smart card credentials collecting
TheBestTvarynka Feb 5, 2024
1a8370a
feat(winscard): env macro available only if std feature is turned on
TheBestTvarynka Feb 5, 2024
6ac8e4f
feat(scard): add pi verification during the as-req pa-datas generation
TheBestTvarynka Feb 5, 2024
61f9bb7
refactor(sspi-ffi): added verfication of the requested package to Acq…
TheBestTvarynka Dec 15, 2023
0e0ca33
fix(kerberos): fixed sensitive data leakage in logs
TheBestTvarynka Oct 24, 2023
7baf5af
fix(ffi): clippy error. refactor(sspi): remove unneeded logs
TheBestTvarynka Feb 5, 2024
36054d9
feat(ffi): fix clippy warning
TheBestTvarynka Feb 5, 2024
e9ccbcf
feat(winscard): rename PIV_AUTHENTICATION_KEY into PIV_DIGITAL_SIGNAT…
TheBestTvarynka Feb 6, 2024
ba45274
refactor(ffi): improve comments
TheBestTvarynka Feb 6, 2024
22b48a4
refactor(sspi): kerberos: remove unneeded log
TheBestTvarynka Feb 6, 2024
b2854b0
feat(winscard): improve cert file reading
TheBestTvarynka Feb 7, 2024
c1c0c04
feat(sspi): kerberos: add FIXME comment about problem with flags
TheBestTvarynka Feb 7, 2024
f1924fa
feat(ffi): sspi: improve comments when reading credentials
TheBestTvarynka Feb 7, 2024
713e27b
feat(sspi): add comment about unused code
TheBestTvarynka Feb 7, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ dns_resolver = ["dep:trust-dns-resolver", "dep:tokio", "tokio/rt-multi-thread"]
# TSSSP should be used only on Windows as a native CREDSSP replacement
tsssp = ["dep:rustls"]
# Turns on Kerberos smart card login (available only on Windows and users WinSCard API)
scard = ["dep:pcsc", "dep:winscard", "dep:iso7816-tlv"]
scard = ["dep:pcsc", "dep:winscard"]

[dependencies]
byteorder = "1.4"
Expand Down Expand Up @@ -70,7 +70,6 @@ tokio = { version = "1.32", features = ["time", "rt"], optional = true }
pcsc = { version = "2.8", optional = true }
async-recursion = "1.0.5"
winscard = { path = "./crates/winscard", optional = true }
iso7816-tlv = { version = "0.4.3", optional = true }

[target.'cfg(windows)'.dependencies]
winreg = "0.51"
Expand Down
1 change: 1 addition & 0 deletions crates/winscard/src/macros.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[cfg(feature = "std")]
macro_rules! env {
($name:expr) => {{
std::env::var($name).map_err(|_| {
Expand Down
29 changes: 24 additions & 5 deletions crates/winscard/src/scard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ pub const ATR: [u8; 17] = [
/// Emulated smart card.
///
/// Currently, we support one key container per smart card.
#[derive(Debug, Clone)]
pub struct SmartCard<'a> {
reader_name: Cow<'a, str>,
chuid: [u8; CHUID_LENGTH],
Expand Down Expand Up @@ -101,7 +102,7 @@ impl SmartCard<'_> {
})
}

fn validate_and_pad_pin(mut pin: Vec<u8>) -> WinScardResult<Vec<u8>> {
fn validate_and_pad_pin(pin: Vec<u8>) -> WinScardResult<Vec<u8>> {
// All PIN requirements can be found here: NIST.SP.800-73-4 part 2, section 2.4.3
if !(PIN_LENGTH_RANGE_LOW_BOUND..=PIN_LENGTH_RANGE_HIGH_BOUND).contains(&pin.len()) {
return Err(Error::new(
Expand All @@ -116,13 +117,17 @@ impl SmartCard<'_> {
));
};

Ok(Self::pad_pin(pin))
}

fn pad_pin(mut pin: Vec<u8>) -> Vec<u8> {
if pin.len() < PIN_LENGTH_RANGE_HIGH_BOUND {
// NIST.SP.800-73-4 part 2, section 2.4.3
const PIN_PAD_VALUE: u8 = 0xFF;
pin.resize(PIN_LENGTH_RANGE_HIGH_BOUND, PIN_PAD_VALUE);
}

Ok(pin)
pin
}

/// This functions handles one APDU command.
Expand Down Expand Up @@ -342,9 +347,9 @@ impl SmartCard<'_> {
// NIST.SP.800-73-4, Part 1, Table 5
const RSA_ALGORITHM: u8 = 0x07;
// NIST.SP.800-73-4, Part 1, Table 4b
const PIV_AUTHENTICATION_KEY: u8 = 0x9A;
const PIV_DIGITAL_SIGNATURE_KEY: u8 = 0x9C;

if cmd.p1 != RSA_ALGORITHM || cmd.p2 != PIV_AUTHENTICATION_KEY {
if cmd.p1 != RSA_ALGORITHM || cmd.p2 != PIV_DIGITAL_SIGNATURE_KEY {
return Err(Error::new(
ErrorKind::UnsupportedFeature,
format!("Provided algorithm or key reference isn't supported: got algorithm {:x}, expected 0x07; got key reference {:x}, expected 0x9A", cmd.p1, cmd.p2)
Expand Down Expand Up @@ -428,6 +433,20 @@ impl SmartCard<'_> {
Ok(signature)
}

/// Verifies the PIN code. This method alters the scard state.
pub fn verify_pin(&mut self, pin: &[u8]) -> WinScardResult<()> {
if self.pin != Self::pad_pin(pin.into()) {
return Err(Error::new(
ErrorKind::InvalidValue,
"PIN verification error: Invalid PIN",
));
}

self.state = SCardState::PinVerified;

Ok(())
}

fn get_next_response_chunk(&mut self) -> Option<(Vec<u8>, usize)> {
let vec = self.pending_response.as_mut()?;
if vec.is_empty() {
Expand All @@ -439,7 +458,7 @@ impl SmartCard<'_> {
}
}

#[derive(Debug, PartialEq)]
#[derive(Debug, Clone, PartialEq)]
enum SCardState {
Ready,
PivAppSelected,
Expand Down
7 changes: 7 additions & 0 deletions crates/winscard/src/scard_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ pub struct Reader<'a> {
}

/// Describes smart card info used for the smart card creation.
#[derive(Debug, Clone)]
pub struct SmartCardInfo<'a> {
/// Container name which stores the certificate along with its private key.
pub container_name: Cow<'a, str>,
/// Smart card PIN code.
pub pin: Vec<u8>,
/// DER-encoded smart card certificate.
pub auth_cert_der: Vec<u8>,
/// Encoded private key (pem).
pub auth_pk_pem: Cow<'a, str>,
/// Private key.
pub auth_pk: PrivateKey,
/// Information about smart card reader.
Expand Down Expand Up @@ -93,6 +96,7 @@ impl<'a> SmartCardInfo<'a> {
container_name,
pin,
auth_cert_der: raw_certificate,
auth_pk_pem: raw_private_key.into(),
auth_pk: private_key,
reader,
})
Expand All @@ -104,6 +108,7 @@ impl<'a> SmartCardInfo<'a> {
reader_name: Cow<'a, str>,
pin: Vec<u8>,
auth_cert_der: Vec<u8>,
auth_pk_pem: Cow<'a, str>,
auth_pk: PrivateKey,
) -> Self {
// Standard Windows Reader Icon
Expand All @@ -117,6 +122,7 @@ impl<'a> SmartCardInfo<'a> {
container_name,
pin,
auth_cert_der,
auth_pk_pem,
auth_pk,
reader,
}
Expand All @@ -126,6 +132,7 @@ impl<'a> SmartCardInfo<'a> {
/// Represents the resource manager context (the scope).
///
/// Currently, we support only one smart card per smart card context.
#[derive(Debug, Clone)]
pub struct ScardContext<'a> {
smart_card_info: SmartCardInfo<'a>,
cache: BTreeMap<String, Vec<u8>>,
Expand Down
8 changes: 4 additions & 4 deletions examples/kerberos.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::error::Error;

use base64::Engine;
use reqwest::header::{
ACCEPT, ACCEPT_ENCODING, ACCEPT_LANGUAGE, AUTHORIZATION, CONNECTION, CONTENT_LENGTH, HOST, USER_AGENT,
Expand All @@ -7,11 +9,9 @@ use reqwest::StatusCode;
use sspi::builders::EmptyInitializeSecurityContext;
use sspi::{
AcquireCredentialsHandleResult, ClientRequestFlags, CredentialsBuffers, DataRepresentation,
InitializeSecurityContextResult, KerberosConfig, SecurityBuffer, SecurityBufferType, SecurityStatus, Sspi,
Username,
InitializeSecurityContextResult, Kerberos, KerberosConfig, SecurityBuffer, SecurityBufferType, SecurityStatus,
Sspi, SspiImpl, Username,
};
use sspi::{Kerberos, SspiImpl};
use std::error::Error;
use tracing_subscriber::prelude::*;
use tracing_subscriber::{fmt, EnvFilter};

Expand Down
18 changes: 17 additions & 1 deletion ffi/src/sspi/sec_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub(crate) unsafe fn p_ctxt_handle_to_sspi_context(
))?),
_ => {
return Err(Error::new(
ErrorKind::InvalidParameter,
ErrorKind::SecurityPackageNotFound,
format!("security package name `{}` is not supported", name),
));
}
Expand All @@ -188,6 +188,18 @@ pub(crate) unsafe fn p_ctxt_handle_to_sspi_context(
Ok((*(*context)).dw_lower as *mut SspiContext)
}

fn verify_security_package(package_name: &str) -> Result<()> {
match package_name {
negotiate::PKG_NAME | pku2u::PKG_NAME | kerberos::PKG_NAME | ntlm::PKG_NAME => Ok(()),
#[cfg(feature = "tsssp")]
sspi_cred_ssp::PKG_NAME => Ok(()),
_ => Err(Error::new(
ErrorKind::SecurityPackageNotFound,
format!("security package name `{}` is not supported", package_name),
)),
}
}

#[instrument(skip_all)]
#[cfg_attr(windows, rename_symbol(to = "Rust_AcquireCredentialsHandleA"))]
#[no_mangle]
Expand All @@ -209,6 +221,8 @@ pub unsafe extern "system" fn AcquireCredentialsHandleA(

let security_package_name =
try_execute!(CStr::from_ptr(psz_package).to_str(), ErrorKind::InvalidParameter).to_owned();
try_execute!(verify_security_package(&security_package_name));

debug!(?security_package_name);

let mut package_list: Option<String> = None;
Expand Down Expand Up @@ -257,6 +271,8 @@ pub unsafe extern "system" fn AcquireCredentialsHandleW(
check_null!(ph_credential);

let security_package_name = c_w_str_to_string(psz_package);
try_execute!(verify_security_package(&security_package_name));

debug!(?security_package_name);

let mut package_list: Option<String> = None;
Expand Down
37 changes: 35 additions & 2 deletions ffi/src/sspi/sec_winnt_auth_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ pub unsafe fn auth_data_to_identity_buffers(
p_auth_data: *const c_void,
package_list: &mut Option<String>,
) -> Result<CredentialsBuffers> {
let rawcreds = std::slice::from_raw_parts(p_auth_data as *const u8, 128);
debug!(?rawcreds);
let raw_creds = from_raw_parts(p_auth_data as *const u8, 128);
debug!(?raw_creds);

#[cfg(feature = "tsssp")]
if _security_package_name == sspi::credssp::sspi_cred_ssp::PKG_NAME {
Expand Down Expand Up @@ -505,6 +505,7 @@ unsafe fn handle_smart_card_creds(mut username: Vec<u8>, password: Secret<Vec<u8
container_name: string_to_utf16(key_container_name),
csp_name: string_to_utf16(csp_name),
private_key_file_index: Some(private_key_file_index),
private_key_pem: None,
});

Ok(creds)
Expand Down Expand Up @@ -582,6 +583,38 @@ pub unsafe fn unpack_sec_winnt_auth_identity_ex2_w_sized(
));
}

// try to collect credentials for the emulated smart card
#[cfg(feature = "scard")]
if username.contains(&b'@') {
use winscard::SmartCardInfo;

use crate::utils::str_encode_utf16;
use crate::winscard::scard_context::{DEFAULT_CARD_NAME, MICROSOFT_DEFAULT_CSP};

match SmartCardInfo::try_from_env() {
Ok(smart_card_info) => {
// remove null
let new_len = password.as_ref().len() - 2;
password.as_mut().truncate(new_len);
Copy link
Member

@CBenoit CBenoit Feb 6, 2024

Choose a reason for hiding this comment

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

nitpick: I recommend writing "proper sentences" to create a more appropriate frame of mind when writing comments. The expectation is to write down more of the context you had in mind when writing this code.

For instance, in this case what I really want to know as someone not familiar with the smart card protocol is "why we need to remove the null terminator at this place?" or "why is there a null terminator", etc. It’s okay to not always explain when it’s just a C-style UTF-16 string → Rust string conversion, but this is not what it is. (And if it was, using the appropriate function would be explicit enough.)

This comment can be applied at several places, but I leave you decide if and how much you do it.

(All that being said, the code you wrote is generally well documented, thank you again.)

Copy link
Collaborator Author

@TheBestTvarynka TheBestTvarynka Feb 6, 2024

Choose a reason for hiding this comment

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

I agree with you. Can you check it again? I updated some comments

Copy link
Member

Choose a reason for hiding this comment

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

It’s looking much better! For this place specifically, I would like something more like that:

The "pin" buffer of the SmartCardIdentityBuffers structure must not be NULL-terminated because / as specified in […] (protocol spec reference? empirical observation?).
Since the password data returned by CredUnPackAuthenticationBufferW is a NULL-terminated wide C string, we need to remove the last two bytes.

The conversion itself is a detail, and what is important is the surroundings, the things we can’t see just by looking at the code.

(Again, I’m really being pedantic here.)

Copy link
Collaborator Author

@TheBestTvarynka TheBestTvarynka Feb 7, 2024

Choose a reason for hiding this comment

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

Again, I’m really being pedantic here.

I can understand you and I like it 😃.
I added more comprehensive explanations


return Ok(CredentialsBuffers::SmartCard(SmartCardIdentityBuffers {
username,
certificate: smart_card_info.auth_cert_der.clone(),
card_name: Some(str_encode_utf16(DEFAULT_CARD_NAME)),
reader_name: str_encode_utf16(smart_card_info.reader.name.as_ref()),
container_name: str_encode_utf16(smart_card_info.container_name.as_ref()),
csp_name: str_encode_utf16(MICROSOFT_DEFAULT_CSP),
pin: password,
private_key_file_index: None,
private_key_pem: Some(smart_card_info.auth_pk_pem.as_bytes().to_vec()),
}));
}
Err(err) => {
debug!(?err);
}
};
}

// only marshaled smart card creds starts with '@' char
#[cfg(feature = "scard")]
if CredIsMarshaledCredentialW(username.as_ptr() as *const _) != 0 {
Expand Down
5 changes: 5 additions & 0 deletions ffi/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,8 @@ pub unsafe fn raw_str_into_bytes(raw_buffer: *const c_char, len: usize) -> Vec<u
pub fn str_to_w_buff(data: &str) -> Vec<u16> {
data.encode_utf16().chain(std::iter::once(0)).collect()
}

#[cfg(feature = "scard")]
pub fn str_encode_utf16(data: &str) -> Vec<u8> {
data.encode_utf16().flat_map(|c| c.to_le_bytes()).collect()
}
4 changes: 2 additions & 2 deletions ffi/src/winscard/scard_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ const SCARD_PROVIDER_KSP: u32 = 3;
// The function retrieves the name of the card module.
const SCARD_PROVIDER_CARD_MODULE: u32 = 0x80000001;

const MICROSOFT_DEFAULT_CSP: &str = "Microsoft Base Smart Card Crypto Provider";
pub const MICROSOFT_DEFAULT_CSP: &str = "Microsoft Base Smart Card Crypto Provider";
const MICROSOFT_DEFAULT_KSP: &str = "Microsoft Smart Card Key Storage Provider";
const MICROSOFT_SCARD_DRIVER_LOCATION: &str = "C:\\Windows\\System32\\msclmd.dll";

const DEFAULT_CARD_NAME: &str = "Cool card";
pub const DEFAULT_CARD_NAME: &str = "Cool card";

// https://learn.microsoft.com/en-us/windows/win32/api/winscard/nf-winscard-scardgetstatuschangew
// To be notified of the arrival of a new smart card reader,
Expand Down
Loading
Loading