From d0c43734e0bd74aff3ef2c614ffc7d8670e49d06 Mon Sep 17 00:00:00 2001 From: Nicholas Russell Date: Mon, 21 Aug 2023 13:02:09 -0500 Subject: [PATCH 1/5] add: User identity negotiation --- storescu/README.md | 5 + storescu/src/main.rs | 40 ++++++++ ul/src/association/client.rs | 177 +++++++++++++++++++++++++++++++++-- ul/src/association/server.rs | 22 ++++- ul/src/pdu/mod.rs | 73 +++++++++++++++ ul/src/pdu/reader.rs | 46 +++++++++ ul/src/pdu/writer.rs | 64 +++++++++++++ ul/tests/pdu.rs | 101 +++++++++++++++++++- 8 files changed, 515 insertions(+), 13 deletions(-) diff --git a/storescu/README.md b/storescu/README.md index 93eb57803..925ed0a55 100644 --- a/storescu/README.md +++ b/storescu/README.md @@ -32,6 +32,11 @@ OPTIONS: --calling-ae-title the calling Application Entity title [default: STORE-SCU] --max-pdu-length the maximum PDU length accepted by the SCU [default: 16384] -m, --message-id the C-STORE message ID [default: 1] + --username user identity username + --password user identity password + --kerberos-service-ticket user identity Kerberos service ticket + --saml-assertion user identity SAML assertion + --jwt user identity JWT ARGS: socket address to Store SCP, optionally with AE title (example: "STORE-SCP@127.0.0.1:104") diff --git a/storescu/src/main.rs b/storescu/src/main.rs index b80759043..804aaaeb0 100644 --- a/storescu/src/main.rs +++ b/storescu/src/main.rs @@ -56,6 +56,21 @@ struct App { // hide option if transcoding is disabled #[cfg_attr(not(feature = "transcode"), arg(hide(true)))] never_transcode: bool, + /// User Identity username + #[arg(long = "username")] + username: Option, + /// User Identity password + #[arg(long = "password")] + password: Option, + /// User Identity Kerberos service ticket + #[arg(long = "kerberos-service-ticket")] + kerberos_service_ticket: Option, + /// User Identity SAML assertion + #[arg(long = "saml-assertion")] + saml_assertion: Option, + /// User Identity JWT + #[arg(long = "jwt")] + jwt: Option, } struct DicomFile { @@ -112,6 +127,11 @@ fn run() -> Result<(), Error> { max_pdu_length, fail_first, mut never_transcode, + username, + password, + kerberos_service_ticket, + saml_assertion, + jwt, } = App::parse(); // never transcode if the feature is disabled @@ -202,6 +222,26 @@ fn run() -> Result<(), Error> { scu_init = scu_init.called_ae_title(called_ae_title); } + if let Some(username) = username { + scu_init = scu_init.username(username); + } + + if let Some(password) = password { + scu_init = scu_init.password(password); + } + + if let Some(kerberos_service_ticket) = kerberos_service_ticket { + scu_init = scu_init.kerberos_service_ticket(kerberos_service_ticket); + } + + if let Some(saml_assertion) = saml_assertion { + scu_init = scu_init.saml_assertion(saml_assertion); + } + + if let Some(jwt) = jwt { + scu_init = scu_init.jwt(jwt); + } + let mut scu = scu_init.establish_with(&addr).context(InitScuSnafu)?; if verbose { diff --git a/ul/src/association/client.rs b/ul/src/association/client.rs index 21dbde4d3..f41950e99 100644 --- a/ul/src/association/client.rs +++ b/ul/src/association/client.rs @@ -17,7 +17,7 @@ use crate::{ writer::write_pdu, AbortRQSource, AssociationAC, AssociationRJ, AssociationRQ, Pdu, PresentationContextProposed, PresentationContextResult, PresentationContextResultReason, - UserVariableItem, + UserIdentity, UserIdentityType, UserVariableItem, }, AeAddr, IMPLEMENTATION_CLASS_UID, IMPLEMENTATION_VERSION_NAME, }; @@ -167,6 +167,16 @@ pub struct ClientAssociationOptions<'a> { max_pdu_length: u32, /// whether to receive PDUs in strict mode strict: bool, + /// User identity username + username: Option>, + /// User identity password + password: Option>, + /// User identity Kerberos service ticket + kerberos_service_ticket: Option>, + /// User identity SAML assertion + saml_assertion: Option>, + /// User identity JWT + jwt: Option>, } impl<'a> Default for ClientAssociationOptions<'a> { @@ -183,6 +193,11 @@ impl<'a> Default for ClientAssociationOptions<'a> { protocol_version: 1, max_pdu_length: crate::pdu::reader::DEFAULT_MAX_PDU, strict: true, + username: None, + password: None, + kerberos_service_ticket: None, + saml_assertion: None, + jwt: None, } } } @@ -270,6 +285,76 @@ impl<'a> ClientAssociationOptions<'a> { self } + /// Sets the user identity username + pub fn username(mut self, username: T) -> Self + where + T: Into>, + { + let username = username.into(); + if username.is_empty() { + self.username = None; + } else { + self.username = Some(username); + } + self + } + + /// Sets the user identity password + pub fn password(mut self, password: T) -> Self + where + T: Into>, + { + let password = password.into(); + if password.is_empty() { + self.password = None; + } else { + self.password = Some(password); + } + self + } + + /// Sets the user identity Kerberos service ticket + pub fn kerberos_service_ticket(mut self, kerberos_service_ticket: T) -> Self + where + T: Into>, + { + let kerberos_service_ticket = kerberos_service_ticket.into(); + if kerberos_service_ticket.is_empty() { + self.kerberos_service_ticket = None; + } else { + self.kerberos_service_ticket = Some(kerberos_service_ticket); + } + self + } + + /// Sets the user identity SAML assertion + pub fn saml_assertion(mut self, saml_assertion: T) -> Self + where + T: Into>, + { + let saml_assertion = saml_assertion.into(); + if saml_assertion.is_empty() { + self.saml_assertion = None; + } else { + self.saml_assertion = Some(saml_assertion); + } + self + } + + /// Sets the user identity JWT + pub fn jwt(mut self, jwt: T) -> Self + where + T: Into>, + { + let jwt = jwt.into(); + if jwt.is_empty() { + self.jwt = None; + } else { + self.jwt = Some(jwt); + } + self + } + /// Initiate the TCP connection to the given address /// and request a new DICOM association, /// negotiating the presentation contexts in the process. @@ -319,6 +404,11 @@ impl<'a> ClientAssociationOptions<'a> { protocol_version, max_pdu_length, strict, + username, + password, + kerberos_service_ticket, + saml_assertion, + jwt, } = self; // fail if no presentation contexts were provided: they represent intent, @@ -355,19 +445,30 @@ impl<'a> ClientAssociationOptions<'a> { .collect(), }) .collect(); + + let mut user_variables = vec![ + UserVariableItem::MaxLength(max_pdu_length), + UserVariableItem::ImplementationClassUID(IMPLEMENTATION_CLASS_UID.to_string()), + UserVariableItem::ImplementationVersionName(IMPLEMENTATION_VERSION_NAME.to_string()), + ]; + + if let Some(user_identity) = Self::determine_user_identity( + username, + password, + kerberos_service_ticket, + saml_assertion, + jwt, + ) { + user_variables.push(UserVariableItem::UserIdentityItem(user_identity)); + } + let msg = Pdu::AssociationRQ(AssociationRQ { protocol_version, calling_ae_title: calling_ae_title.to_string(), called_ae_title: called_ae_title.to_string(), application_context_name: application_context_name.to_string(), presentation_contexts, - user_variables: vec![ - UserVariableItem::MaxLength(max_pdu_length), - UserVariableItem::ImplementationClassUID(IMPLEMENTATION_CLASS_UID.to_string()), - UserVariableItem::ImplementationVersionName( - IMPLEMENTATION_VERSION_NAME.to_string(), - ), - ], + user_variables, }); let mut socket = std::net::TcpStream::connect(ae_address).context(ConnectSnafu)?; @@ -467,6 +568,66 @@ impl<'a> ClientAssociationOptions<'a> { } } } + + fn determine_user_identity( + username: Option, + password: Option, + kerberos_service_ticket: Option, + saml_assertion: Option, + jwt: Option, + ) -> Option + where + T: Into>, + { + let mut result: Option = None; + + if let Some(username) = username { + if let Some(password) = password { + result = Some(UserIdentity::new( + false, + UserIdentityType::UsernamePassword, + username.into().as_bytes().to_vec(), + password.into().as_bytes().to_vec(), + )); + } else { + result = Some(UserIdentity::new( + false, + UserIdentityType::Username, + username.into().as_bytes().to_vec(), + vec![], + )); + } + } + + if let Some(kerberos_service_ticket) = kerberos_service_ticket { + result = Some(UserIdentity::new( + false, + UserIdentityType::KerberosServiceTicket, + kerberos_service_ticket.into().as_bytes().to_vec(), + vec![], + )); + } + + if let Some(saml_assertion) = saml_assertion { + result = Some(UserIdentity::new( + false, + UserIdentityType::SAMLAssertion, + saml_assertion.into().as_bytes().to_vec(), + vec![], + )); + } + + if let Some(jwt) = jwt { + result = Some(UserIdentity::new( + false, + UserIdentityType::JWT, + jwt.into().as_bytes().to_vec(), + vec![], + )); + } + + result + } } /// A DICOM upper level association from the perspective diff --git a/ul/src/association/server.rs b/ul/src/association/server.rs index 465a62a0b..5819d5842 100644 --- a/ul/src/association/server.rs +++ b/ul/src/association/server.rs @@ -16,7 +16,8 @@ use crate::{ writer::write_pdu, AbortRQServiceProviderReason, AbortRQSource, AssociationAC, AssociationRJ, AssociationRJResult, AssociationRJServiceUserReason, AssociationRJSource, AssociationRQ, - Pdu, PresentationContextResult, PresentationContextResultReason, UserVariableItem, + Pdu, PresentationContextResult, PresentationContextResultReason, UserIdentity, + UserVariableItem, }, IMPLEMENTATION_CLASS_UID, IMPLEMENTATION_VERSION_NAME, }; @@ -98,7 +99,7 @@ pub type Result = std::result::Result; /// but users are free to implement their own. pub trait AccessControl { /// Obtain the decision of whether to accept an incoming association request - /// based on the recorded application entity titles. + /// based on the recorded application entity titles and/or user identity. /// /// Returns Ok(()) if the requester node should be given clearance. /// Otherwise, a concrete association RJ service user reason is given. @@ -107,6 +108,7 @@ pub trait AccessControl { this_ae_title: &str, calling_ae_title: &str, called_ae_title: &str, + user_identity: Option<&UserIdentity>, ) -> Result<(), AssociationRJServiceUserReason>; } @@ -120,6 +122,7 @@ impl AccessControl for AcceptAny { _this_ae_title: &str, _calling_ae_title: &str, _called_ae_title: &str, + _user_identity: Option<&UserIdentity>, ) -> Result<(), AssociationRJServiceUserReason> { Ok(()) } @@ -136,6 +139,7 @@ impl AccessControl for AcceptCalledAeTitle { this_ae_title: &str, _calling_ae_title: &str, called_ae_title: &str, + _user_identity: Option<&UserIdentity>, ) -> Result<(), AssociationRJServiceUserReason> { if this_ae_title == called_ae_title { Ok(()) @@ -389,7 +393,19 @@ where } self.ae_access_control - .check_access(&self.ae_title, &calling_ae_title, &called_ae_title) + .check_access( + &self.ae_title, + &calling_ae_title, + &called_ae_title, + user_variables + .iter() + .find_map(|user_variable| match user_variable { + UserVariableItem::UserIdentityItem(user_identity) => { + Some(user_identity) + } + _ => None, + }), + ) .map(Ok) .unwrap_or_else(|reason| { write_pdu( diff --git a/ul/src/pdu/mod.rs b/ul/src/pdu/mod.rs index eeeb985b6..ebf8c5f54 100644 --- a/ul/src/pdu/mod.rs +++ b/ul/src/pdu/mod.rs @@ -330,6 +330,79 @@ pub enum UserVariableItem { ImplementationClassUID(String), ImplementationVersionName(String), SopClassExtendedNegotiationSubItem(String, Vec), + UserIdentityItem(UserIdentity), +} + +#[derive(Clone, Eq, PartialEq, PartialOrd, Hash, Debug)] +pub struct UserIdentity { + positive_response_requested: bool, + identity_type: UserIdentityType, + primary_field: Vec, + secondary_field: Vec, +} +impl UserIdentity { + pub fn new( + positive_response_requested: bool, + identity_type: UserIdentityType, + primary_field: Vec, + secondary_field: Vec, + ) -> Self { + UserIdentity { + positive_response_requested, + identity_type, + primary_field, + secondary_field, + } + } + + pub fn positive_response_requested(&self) -> bool { + self.positive_response_requested + } + + pub fn identity_type(&self) -> UserIdentityType { + self.identity_type.clone() + } + + pub fn primary_field(&self) -> Vec { + self.primary_field.clone() + } + + pub fn secondary_field(&self) -> Vec { + self.secondary_field.clone() + } +} + +#[derive(Clone, Eq, PartialEq, PartialOrd, Hash, Debug)] +pub enum UserIdentityType { + Username, + UsernamePassword, + KerberosServiceTicket, + SAMLAssertion, + JWT, + Unsupported, +} +impl UserIdentityType { + fn from(user_identity_type: u8) -> Self { + match user_identity_type { + 1 => Self::Username, + 2 => Self::UsernamePassword, + 3 => Self::KerberosServiceTicket, + 4 => Self::SAMLAssertion, + 5 => Self::JWT, + _ => Self::Unsupported, + } + } + + fn to_u8(&self) -> u8 { + match self { + Self::Username => 1, + Self::UsernamePassword => 2, + Self::KerberosServiceTicket => 3, + Self::SAMLAssertion => 4, + Self::JWT => 5, + Self::Unsupported => 0, + } + } } /// An in-memory representation of a full Protocol Data Unit (PDU) diff --git a/ul/src/pdu/reader.rs b/ul/src/pdu/reader.rs index 08b24b89b..dbbd24c89 100644 --- a/ul/src/pdu/reader.rs +++ b/ul/src/pdu/reader.rs @@ -945,6 +945,52 @@ where data, )); } + 0x58 => { + // User Identity Negotiation + + // 5 - User Identity Type + let user_identity_type = cursor.read_u8().context(ReadPduFieldSnafu { + field: "User-Identity-type", + })?; + + // 6 - Positive-response-requested + let positive_response_requested = + cursor.read_u8().context(ReadPduFieldSnafu { + field: "User-Identity-positive-response-requested", + })?; + + // 7-8 - Primary Field Length + let primary_field_length = + cursor.read_u16::().context(ReadPduFieldSnafu { + field: "User-Identity-primary-field-length", + })?; + + // 9-n - Primary Field + let primary_field = read_n(&mut cursor, primary_field_length as usize) + .context(ReadPduFieldSnafu { + field: "User-Identity-primary-field", + })?; + + // n+1-n+2 - Secondary Field Length + // Only non-zero if user identity type is 2 (username and password) + let secondary_field_length = + cursor.read_u16::().context(ReadPduFieldSnafu { + field: "User-Identity-secondary-field-length", + })?; + + // n+3-m - Secondary Field + let secondary_field = read_n(&mut cursor, secondary_field_length as usize) + .context(ReadPduFieldSnafu { + field: "User-Identity-secondary-field", + })?; + + user_variables.push(UserVariableItem::UserIdentityItem(UserIdentity::new( + positive_response_requested == 1, + UserIdentityType::from(user_identity_type), + primary_field, + secondary_field, + ))); + } _ => { user_variables.push(UserVariableItem::Unknown( item_type, diff --git a/ul/src/pdu/writer.rs b/ul/src/pdu/writer.rs index 6efcc5457..5d388609a 100644 --- a/ul/src/pdu/writer.rs +++ b/ul/src/pdu/writer.rs @@ -1025,6 +1025,70 @@ fn write_pdu_variable_user_variables( }) .context(WriteChunkSnafu { name: "Sub-item" })?; } + UserVariableItem::UserIdentityItem(user_identity) => { + // 1 - Item-type - 58H + writer + .write_u8(0x58) + .context(WriteFieldSnafu { field: "Item-type" })?; + + // 2 - Reserved - This reserved field shall be sent with a value 00H but not + // tested to this value when received. + writer + .write_u8(0x00) + .context(WriteReservedSnafu { bytes: 1_u32 })?; + + // 3-4 - Item-length + write_chunk_u16(writer, |writer| { + // 5 - User-Identity-Type + writer + .write_u8(user_identity.identity_type().to_u8()) + .context(WriteFieldSnafu { + field: "User-Identity-Type", + })?; + + // 6 - Positive-response-requested + let positive_response_requested_out: u8 = + if user_identity.positive_response_requested() { + 1 + } else { + 0 + }; + writer.write_u8(positive_response_requested_out).context( + WriteFieldSnafu { + field: "Positive-response-requested", + }, + )?; + + // 7-8 - Primary-field-length + write_chunk_u16(writer, |writer| { + // 9-n - Primary-field + writer + .write_all(user_identity.primary_field().as_slice()) + .context(WriteFieldSnafu { + field: "Primary-field", + }) + }) + .context(WriteChunkSnafu { + name: "Primary-field", + })?; + + // n+1-n+2 - Secondary-field-length + write_chunk_u16(writer, |writer| { + // n+3-m - Secondary-field + writer + .write_all(user_identity.secondary_field().as_slice()) + .context(WriteFieldSnafu { + field: "Secondary-field", + }) + }) + .context(WriteChunkSnafu { + name: "Secondary-field", + }) + }) + .context(WriteChunkSnafu { + name: "Item-length", + })?; + } UserVariableItem::Unknown(item_type, data) => { writer .write_u8(*item_type) diff --git a/ul/tests/pdu.rs b/ul/tests/pdu.rs index 57df27350..c5f7d733a 100644 --- a/ul/tests/pdu.rs +++ b/ul/tests/pdu.rs @@ -1,7 +1,8 @@ use dicom_ul::pdu::reader::{read_pdu, DEFAULT_MAX_PDU}; use dicom_ul::pdu::writer::write_pdu; use dicom_ul::pdu::{ - AssociationRQ, PDataValue, PDataValueType, Pdu, PresentationContextProposed, UserVariableItem, + AssociationRQ, PDataValue, PDataValueType, Pdu, PresentationContextProposed, UserIdentity, + UserIdentityType, UserVariableItem, }; use matches::matches; use std::io::Cursor; @@ -33,6 +34,12 @@ fn can_read_write_associate_rq() -> Result<(), Box> { "abstract 1".to_string(), vec![1, 1, 0, 1, 1, 0, 1], ), + UserVariableItem::UserIdentityItem(UserIdentity::new( + false, + UserIdentityType::UsernamePassword, + "MyUsername".as_bytes().to_vec(), + "MyPassword".as_bytes().to_vec(), + )), ], }; @@ -66,7 +73,7 @@ fn can_read_write_associate_rq() -> Result<(), Box> { assert_eq!(presentation_contexts[1].transfer_syntaxes.len(), 2); assert_eq!(presentation_contexts[1].transfer_syntaxes[0], "transfer 3"); assert_eq!(presentation_contexts[1].transfer_syntaxes[1], "transfer 4"); - assert_eq!(user_variables.len(), 4); + assert_eq!(user_variables.len(), 5); assert!(matches!( &user_variables[0], UserVariableItem::ImplementationClassUID(u) if u == "class uid" @@ -81,6 +88,96 @@ fn can_read_write_associate_rq() -> Result<(), Box> { if sop_class_uid == "abstract 1" && data.as_slice() == [1,1,0,1,1,0,1] )); + assert!(matches!(&user_variables[4], + UserVariableItem::UserIdentityItem(user_identity) + if !user_identity.positive_response_requested() && + user_identity.identity_type() == UserIdentityType::UsernamePassword && + user_identity.primary_field() == [77,121,85,115,101,114,110,97,109,101] && + user_identity.secondary_field() == [77,121,80,97,115,115,119,111,114,100] + )); + } else { + panic!("invalid pdu type"); + } + + Ok(()) +} + +#[test] +fn can_read_write_primary_field_only_user_identity() -> Result<(), Box> { + let association_rq = AssociationRQ { + protocol_version: 2, + calling_ae_title: "calling ae".to_string(), + called_ae_title: "called ae".to_string(), + application_context_name: "application context name".to_string(), + presentation_contexts: vec![PresentationContextProposed { + id: 1, + abstract_syntax: "abstract 1".to_string(), + transfer_syntaxes: vec!["transfer 1".to_string()], + }], + user_variables: vec![ + UserVariableItem::ImplementationClassUID("class uid".to_string()), + UserVariableItem::ImplementationVersionName("version name".to_string()), + UserVariableItem::MaxLength(23), + UserVariableItem::SopClassExtendedNegotiationSubItem( + "abstract 1".to_string(), + vec![1, 1, 0, 1, 1, 0, 1], + ), + UserVariableItem::UserIdentityItem(UserIdentity::new( + false, + UserIdentityType::Username, + "MyUsername".as_bytes().to_vec(), + vec![], + )), + ], + }; + + let mut bytes = vec![0u8; 0]; + write_pdu(&mut bytes, &association_rq.into())?; + + let result = read_pdu(&mut Cursor::new(&bytes), DEFAULT_MAX_PDU, true)?; + + if let Pdu::AssociationRQ(AssociationRQ { + protocol_version, + calling_ae_title, + called_ae_title, + application_context_name, + presentation_contexts, + user_variables, + }) = result + { + assert_eq!(protocol_version, 2); + assert_eq!(calling_ae_title, "calling ae"); + assert_eq!(called_ae_title, "called ae"); + assert_eq!( + application_context_name, + "application context name".to_string() + ); + assert_eq!(presentation_contexts.len(), 1); + assert_eq!(presentation_contexts[0].abstract_syntax, "abstract 1"); + assert_eq!(presentation_contexts[0].transfer_syntaxes.len(), 1); + assert_eq!(presentation_contexts[0].transfer_syntaxes[0], "transfer 1"); + assert_eq!(user_variables.len(), 5); + assert!(matches!( + &user_variables[0], + UserVariableItem::ImplementationClassUID(u) if u == "class uid" + )); + assert!(matches!( + &user_variables[1], + UserVariableItem::ImplementationVersionName(v) if v == "version name" + )); + assert!(matches!(user_variables[2], UserVariableItem::MaxLength(l) if l == 23)); + assert!(matches!(&user_variables[3], + UserVariableItem::SopClassExtendedNegotiationSubItem(sop_class_uid, data) + if sop_class_uid == "abstract 1" && + data.as_slice() == [1,1,0,1,1,0,1] + )); + assert!(matches!(&user_variables[4], + UserVariableItem::UserIdentityItem(user_identity) + if !user_identity.positive_response_requested() && + user_identity.identity_type() == UserIdentityType::Username && + user_identity.primary_field() == [77,121,85,115,101,114,110,97,109,101] && + user_identity.secondary_field() == [] + )); } else { panic!("invalid pdu type"); } From 805725903659a8f3e431d0083e513193ad95670e Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sun, 24 Mar 2024 09:56:46 +0000 Subject: [PATCH 2/5] [ul] Refactor UserIdentityType - Remove `Unsupported` - rename SAMLAssertion to `SamlAssertion` - rename JWT to `Jwt` - make UserIdentityType non-exhaustive --- ul/src/association/client.rs | 4 ++-- ul/src/pdu/mod.rs | 25 ++++++++++++------------- ul/src/pdu/reader.rs | 22 ++++++++++++++++------ 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/ul/src/association/client.rs b/ul/src/association/client.rs index f41950e99..6e78d140a 100644 --- a/ul/src/association/client.rs +++ b/ul/src/association/client.rs @@ -611,7 +611,7 @@ impl<'a> ClientAssociationOptions<'a> { if let Some(saml_assertion) = saml_assertion { result = Some(UserIdentity::new( false, - UserIdentityType::SAMLAssertion, + UserIdentityType::SamlAssertion, saml_assertion.into().as_bytes().to_vec(), vec![], )); @@ -620,7 +620,7 @@ impl<'a> ClientAssociationOptions<'a> { if let Some(jwt) = jwt { result = Some(UserIdentity::new( false, - UserIdentityType::JWT, + UserIdentityType::Jwt, jwt.into().as_bytes().to_vec(), vec![], )); diff --git a/ul/src/pdu/mod.rs b/ul/src/pdu/mod.rs index ebf8c5f54..a7093b39c 100644 --- a/ul/src/pdu/mod.rs +++ b/ul/src/pdu/mod.rs @@ -373,23 +373,23 @@ impl UserIdentity { } #[derive(Clone, Eq, PartialEq, PartialOrd, Hash, Debug)] +#[non_exhaustive] pub enum UserIdentityType { Username, UsernamePassword, KerberosServiceTicket, - SAMLAssertion, - JWT, - Unsupported, + SamlAssertion, + Jwt, } impl UserIdentityType { - fn from(user_identity_type: u8) -> Self { + fn from(user_identity_type: u8) -> Option { match user_identity_type { - 1 => Self::Username, - 2 => Self::UsernamePassword, - 3 => Self::KerberosServiceTicket, - 4 => Self::SAMLAssertion, - 5 => Self::JWT, - _ => Self::Unsupported, + 1 => Some(Self::Username), + 2 => Some(Self::UsernamePassword), + 3 => Some(Self::KerberosServiceTicket), + 4 => Some(Self::SamlAssertion), + 5 => Some(Self::Jwt), + _ => None, } } @@ -398,9 +398,8 @@ impl UserIdentityType { Self::Username => 1, Self::UsernamePassword => 2, Self::KerberosServiceTicket => 3, - Self::SAMLAssertion => 4, - Self::JWT => 5, - Self::Unsupported => 0, + Self::SamlAssertion => 4, + Self::Jwt => 5, } } } diff --git a/ul/src/pdu/reader.rs b/ul/src/pdu/reader.rs index dbbd24c89..3423f2710 100644 --- a/ul/src/pdu/reader.rs +++ b/ul/src/pdu/reader.rs @@ -4,6 +4,7 @@ use byteordered::byteorder::{BigEndian, ReadBytesExt}; use dicom_encoding::text::{DefaultCharacterSetCodec, TextCodec}; use snafu::{ensure, Backtrace, OptionExt, ResultExt, Snafu}; use std::io::{Cursor, ErrorKind, Read, Seek, SeekFrom}; +use tracing::warn; /// The default maximum PDU size pub const DEFAULT_MAX_PDU: u32 = 16_384; @@ -984,12 +985,21 @@ where field: "User-Identity-secondary-field", })?; - user_variables.push(UserVariableItem::UserIdentityItem(UserIdentity::new( - positive_response_requested == 1, - UserIdentityType::from(user_identity_type), - primary_field, - secondary_field, - ))); + match UserIdentityType::from(user_identity_type) { + Some(user_identity_type) => { + user_variables.push(UserVariableItem::UserIdentityItem( + UserIdentity::new( + positive_response_requested == 1, + user_identity_type, + primary_field, + secondary_field, + ), + )); + } + None => { + warn!("Unknown User Identity Type code {}", user_identity_type); + } + } } _ => { user_variables.push(UserVariableItem::Unknown( From 0ebf07ba026f1ea344995b0d2cc01eb10afa4d4e Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sun, 24 Mar 2024 10:01:17 +0000 Subject: [PATCH 3/5] [ul] Use byte string literals in associate rq tests --- ul/tests/pdu.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ul/tests/pdu.rs b/ul/tests/pdu.rs index c5f7d733a..26c7fa157 100644 --- a/ul/tests/pdu.rs +++ b/ul/tests/pdu.rs @@ -37,8 +37,8 @@ fn can_read_write_associate_rq() -> Result<(), Box> { UserVariableItem::UserIdentityItem(UserIdentity::new( false, UserIdentityType::UsernamePassword, - "MyUsername".as_bytes().to_vec(), - "MyPassword".as_bytes().to_vec(), + b"MyUsername".to_vec(), + b"MyPassword".to_vec(), )), ], }; @@ -125,7 +125,7 @@ fn can_read_write_primary_field_only_user_identity() -> Result<(), Box Date: Thu, 11 Apr 2024 08:38:01 +0100 Subject: [PATCH 4/5] [ul] Tweak client user identity options - constrain to at most one user identity negotiation variable set --- ul/src/association/client.rs | 55 ++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/ul/src/association/client.rs b/ul/src/association/client.rs index 6e78d140a..875f2342b 100644 --- a/ul/src/association/client.rs +++ b/ul/src/association/client.rs @@ -295,6 +295,9 @@ impl<'a> ClientAssociationOptions<'a> { self.username = None; } else { self.username = Some(username); + self.saml_assertion = None; + self.jwt = None; + self.kerberos_service_ticket = None; } self } @@ -309,6 +312,30 @@ impl<'a> ClientAssociationOptions<'a> { self.password = None; } else { self.password = Some(password); + self.saml_assertion = None; + self.jwt = None; + self.kerberos_service_ticket = None; + } + self + } + + /// Sets the user identity username and password + pub fn username_password(mut self, username: T, password: U) -> Self + where + T: Into>, + U: Into>, + { + let username = username.into(); + let password = password.into(); + if username.is_empty() { + self.username = None; + self.password = None; + } else { + self.username = Some(username); + self.password = Some(password); + self.saml_assertion = None; + self.jwt = None; + self.kerberos_service_ticket = None; } self } @@ -323,6 +350,10 @@ impl<'a> ClientAssociationOptions<'a> { self.kerberos_service_ticket = None; } else { self.kerberos_service_ticket = Some(kerberos_service_ticket); + self.username = None; + self.password = None; + self.saml_assertion = None; + self.jwt = None; } self } @@ -337,6 +368,10 @@ impl<'a> ClientAssociationOptions<'a> { self.saml_assertion = None; } else { self.saml_assertion = Some(saml_assertion); + self.username = None; + self.password = None; + self.jwt = None; + self.kerberos_service_ticket = None; } self } @@ -351,6 +386,10 @@ impl<'a> ClientAssociationOptions<'a> { self.jwt = None; } else { self.jwt = Some(jwt); + self.username = None; + self.password = None; + self.saml_assertion = None; + self.kerberos_service_ticket = None; } self } @@ -579,18 +618,16 @@ impl<'a> ClientAssociationOptions<'a> { where T: Into>, { - let mut result: Option = None; - if let Some(username) = username { if let Some(password) = password { - result = Some(UserIdentity::new( + return Some(UserIdentity::new( false, UserIdentityType::UsernamePassword, username.into().as_bytes().to_vec(), password.into().as_bytes().to_vec(), )); } else { - result = Some(UserIdentity::new( + return Some(UserIdentity::new( false, UserIdentityType::Username, username.into().as_bytes().to_vec(), @@ -600,7 +637,7 @@ impl<'a> ClientAssociationOptions<'a> { } if let Some(kerberos_service_ticket) = kerberos_service_ticket { - result = Some(UserIdentity::new( + return Some(UserIdentity::new( false, UserIdentityType::KerberosServiceTicket, kerberos_service_ticket.into().as_bytes().to_vec(), @@ -609,7 +646,7 @@ impl<'a> ClientAssociationOptions<'a> { } if let Some(saml_assertion) = saml_assertion { - result = Some(UserIdentity::new( + return Some(UserIdentity::new( false, UserIdentityType::SamlAssertion, saml_assertion.into().as_bytes().to_vec(), @@ -618,7 +655,7 @@ impl<'a> ClientAssociationOptions<'a> { } if let Some(jwt) = jwt { - result = Some(UserIdentity::new( + return Some(UserIdentity::new( false, UserIdentityType::Jwt, jwt.into().as_bytes().to_vec(), @@ -626,7 +663,7 @@ impl<'a> ClientAssociationOptions<'a> { )); } - result + None } } @@ -707,7 +744,7 @@ impl ClientAssociation { let _ = self.socket.shutdown(std::net::Shutdown::Both); out } - + /// Send an abort message and shut down the TCP connection, /// terminating the association. pub fn abort(mut self) -> Result<()> { From d412885c84f005e2758a367224e0624b48248a21 Mon Sep 17 00:00:00 2001 From: Eduardo Pinho Date: Sat, 13 Apr 2024 10:48:17 +0100 Subject: [PATCH 5/5] [storescu] Add constraints to user identity arguments - declare conflict with other user identity forms - + format code --- storescu/src/main.rs | 47 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/storescu/src/main.rs b/storescu/src/main.rs index 804aaaeb0..8ac4e05f0 100644 --- a/storescu/src/main.rs +++ b/storescu/src/main.rs @@ -57,19 +57,39 @@ struct App { #[cfg_attr(not(feature = "transcode"), arg(hide(true)))] never_transcode: bool, /// User Identity username - #[arg(long = "username")] + #[arg( + long = "username", + conflicts_with("kerberos_service_ticket"), + conflicts_with("saml_assertion"), + conflicts_with("jwt") + )] username: Option, /// User Identity password - #[arg(long = "password")] + #[arg(long = "password", requires("username"))] password: Option, /// User Identity Kerberos service ticket - #[arg(long = "kerberos-service-ticket")] + #[arg( + long = "kerberos-service-ticket", + conflicts_with("username"), + conflicts_with("saml_assertion"), + conflicts_with("jwt") + )] kerberos_service_ticket: Option, /// User Identity SAML assertion - #[arg(long = "saml-assertion")] + #[arg( + long = "saml-assertion", + conflicts_with("username"), + conflicts_with("kerberos_service_ticket"), + conflicts_with("jwt") + )] saml_assertion: Option, /// User Identity JWT - #[arg(long = "jwt")] + #[arg( + long = "jwt", + conflicts_with("username"), + conflicts_with("kerberos_service_ticket"), + conflicts_with("saml_assertion") + )] jwt: Option, } @@ -96,7 +116,9 @@ enum Error { }, /// Could not construct DICOM command - CreateCommand { source: Box }, + CreateCommand { + source: Box, + }, /// Unsupported file transfer syntax {uid} UnsupportedFileTransferSyntax { uid: std::borrow::Cow<'static, str> }, @@ -310,7 +332,9 @@ fn run() -> Result<(), Error> { open_file(&file.file).whatever_context("Could not open listed DICOM file")?; let ts_selected = TransferSyntaxRegistry .get(&ts_uid_selected) - .with_context(|| UnsupportedFileTransferSyntaxSnafu { uid: ts_uid_selected.to_string() })?; + .with_context(|| UnsupportedFileTransferSyntaxSnafu { + uid: ts_uid_selected.to_string(), + })?; // transcode file if necessary let dicom_file = into_ts(dicom_file, ts_selected, verbose)?; @@ -523,7 +547,9 @@ fn check_file(file: &Path) -> Result { let transfer_syntax_uid = &meta.transfer_syntax.trim_end_matches('\0'); let ts = TransferSyntaxRegistry .get(transfer_syntax_uid) - .with_context(|| UnsupportedFileTransferSyntaxSnafu { uid: transfer_syntax_uid.to_string() })?; + .with_context(|| UnsupportedFileTransferSyntaxSnafu { + uid: transfer_syntax_uid.to_string(), + })?; Ok(DicomFile { file: file.to_path_buf(), sop_class_uid: storage_sop_class_uid.to_string(), @@ -541,7 +567,9 @@ fn check_presentation_contexts( ) -> Result<(dicom_ul::pdu::PresentationContextResult, String), Error> { let file_ts = TransferSyntaxRegistry .get(&file.file_transfer_syntax) - .with_context(|| UnsupportedFileTransferSyntaxSnafu { uid: file.file_transfer_syntax.to_string() })?; + .with_context(|| UnsupportedFileTransferSyntaxSnafu { + uid: file.file_transfer_syntax.to_string(), + })?; // if destination does not support original file TS, // check whether we can transcode to explicit VR LE @@ -585,7 +613,6 @@ fn check_presentation_contexts( Ok((pc.clone(), String::from(ts.uid()))) } - // transcoding functions #[cfg(feature = "transcode")]