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.

Sign up for GitHub

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

Clients should accept messages secured by an expired SecurityToken #404

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Clients should accept messages secured by an expired SecurityToken #404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2fbf0cb
Clients should accept messages secured by an expired SecurityToken
svanharmelen Nov 9, 2024
b4c4af3
Rename expired_at to expired_in where it concerns a duration
svanharmelen Nov 22, 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
98 changes: 71 additions & 27 deletions lib/src/core/comms/secure_channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
// Copyright (C) 2017-2024 Adam Lock

use std::{
collections::HashMap,
io::{Cursor, Write},
ops::Range,
sync::Arc,
};

use chrono::{Duration, TimeDelta};
use chrono::Duration;

use crate::crypto::{
aeskey::AesKey,
Expand All @@ -35,6 +36,12 @@ pub enum Role {
Server,
}

#[derive(Debug)]
struct RemoteKeys {
keys: (Vec<u8>, AesKey, Vec<u8>),
expires_at: DateTime,
svanharmelen marked this conversation as resolved.
Show resolved Hide resolved
}

/// Holds all of the security information related to this session
#[derive(Debug)]
pub struct SecureChannel {
Expand Down Expand Up @@ -63,7 +70,14 @@ pub struct SecureChannel {
/// Our nonce generated while handling open secure channel
local_nonce: Vec<u8>,
/// Client (i.e. other end's set of keys) Symmetric Signing Key, Encrypt Key, IV
remote_keys: Option<(Vec<u8>, AesKey, Vec<u8>)>,
///
/// This is a map of channel token ids and their respective keys. We need to keep
/// the old keys around as the client should accept messages secured by an expired
/// SecurityToken for up to 25 % of the token lifetime.
///
/// See the "OpenSecureChannel" section in the spec for more info:
/// https://reference.opcfoundation.org/Core/Part4/v105/docs/5.5.2
remote_keys: HashMap<u32, RemoteKeys>,
/// Server (i.e. our end's set of keys) Symmetric Signing Key, Decrypt Key, IV
local_keys: Option<(Vec<u8>, AesKey, Vec<u8>)>,
/// Decoding options
Expand All @@ -88,7 +102,7 @@ impl SecureChannel {
private_key: None,
remote_cert: None,
local_keys: None,
remote_keys: None,
remote_keys: HashMap::new(),
decoding_options: DecodingOptions::default(),
}
}
Expand Down Expand Up @@ -121,7 +135,7 @@ impl SecureChannel {
private_key,
remote_cert: None,
local_keys: None,
remote_keys: None,
remote_keys: HashMap::new(),
decoding_options,
}
}
Expand Down Expand Up @@ -226,10 +240,10 @@ impl SecureChannel {
false
} else {
// Check if secure channel 75% close to expiration in which case send a renew
let renew_lifetime = (self.token_lifetime() * 3) / 4;
let renew_lifetime = TimeDelta::try_milliseconds(renew_lifetime as i64).unwrap();
let renew_lifetime = (self.token_lifetime * 3) / 4;
let renew_lifetime = Duration::milliseconds(renew_lifetime as i64);
// Renew the token?
DateTime::now() - self.token_created_at() > renew_lifetime
DateTime::now() - self.token_created_at > renew_lifetime
}
}

Expand Down Expand Up @@ -356,7 +370,7 @@ impl SecureChannel {
/// are used to secure Messages sent by the Server.
///
pub fn derive_keys(&mut self) {
self.remote_keys = Some(
self.insert_remote_keys(
self.security_policy
.make_secure_channel_keys(&self.local_nonce, &self.remote_nonce),
);
Expand All @@ -366,18 +380,13 @@ impl SecureChannel {
);
trace!("Remote nonce = {:?}", self.remote_nonce);
trace!("Local nonce = {:?}", self.local_nonce);
trace!("Derived remote keys = {:?}", self.remote_keys);
trace!(
"Derived remote keys = {:?}",
self.get_remote_keys(self.token_id)
);
trace!("Derived local keys = {:?}", self.local_keys);
}

/// Test if the token has expired yet
pub fn token_has_expired(&self) -> bool {
let token_created_at = self.token_created_at;
let token_expires =
token_created_at + TimeDelta::try_seconds(self.token_lifetime as i64).unwrap();
DateTime::now().ge(&token_expires)
}

/// Calculates the signature size for a message depending on the supplied security header
pub fn signature_size(&self, security_header: &SecurityHeader) -> usize {
// Signature size in bytes
Expand Down Expand Up @@ -739,11 +748,20 @@ impl SecureChannel {
encrypted_range
);

let SecurityHeader::Symmetric(security_header) = security_header else {
error!(
"Expected symmetric security header, got {:?}",
security_header
);
return Err(StatusCode::BadUnexpectedError);
};

let mut decrypted_data = vec![0u8; message_size];
let decrypted_size = self.symmetric_decrypt_and_verify(
src,
signed_range,
encrypted_range,
security_header.token_id,
&mut decrypted_data,
)?;

Expand Down Expand Up @@ -1048,8 +1066,33 @@ impl SecureChannel {
self.local_keys.as_ref().unwrap()
}

fn remote_keys(&self) -> &(Vec<u8>, AesKey, Vec<u8>) {
self.remote_keys.as_ref().unwrap()
fn insert_remote_keys(&mut self, keys: (Vec<u8>, AesKey, Vec<u8>)) {
// First remove any expired keys.
self.remote_keys
.retain(|_, v| DateTime::now() < v.expires_at);

let expires_in = (self.token_lifetime as f32 * 1.25).ceil();
let expires_in = Duration::milliseconds(expires_in as i64);

// Then insert the new keys to ensure there is
// always at least one set of keys available.
self.remote_keys.insert(
self.token_id,
RemoteKeys {
keys,
expires_at: self.token_created_at + expires_in,
},
);
}

fn get_remote_keys(&self, token_id: u32) -> Result<&(Vec<u8>, AesKey, Vec<u8>), StatusCode> {
match self.remote_keys.get(&token_id) {
Some(remote_keys) => Ok(&remote_keys.keys),
None => {
error!("No remote keys found for token: {}", token_id);
Err(StatusCode::BadUnexpectedError)
}
}
}

fn encryption_keys(&self) -> (&AesKey, &[u8]) {
Expand All @@ -1061,13 +1104,13 @@ impl SecureChannel {
&(self.local_keys()).0
}

fn decryption_keys(&self) -> (&AesKey, &[u8]) {
let keys = self.remote_keys();
(&keys.1, &keys.2)
fn decryption_keys(&self, token_id: u32) -> Result<(&AesKey, &[u8]), StatusCode> {
let keys = self.get_remote_keys(token_id)?;
Ok((&keys.1, &keys.2))
}

fn verification_key(&self) -> &[u8] {
&(self.remote_keys()).0
fn verification_key(&self, token_id: u32) -> Result<&[u8], StatusCode> {
Ok(&(self.get_remote_keys(token_id))?.0)
}

/// Encode data using security. Destination buffer is expected to be same size as src and expected
Expand Down Expand Up @@ -1178,6 +1221,7 @@ impl SecureChannel {
src: &[u8],
signed_range: Range<usize>,
encrypted_range: Range<usize>,
token_id: u32,
dst: &mut [u8],
) -> Result<usize, StatusCode> {
match self.security_mode {
Expand All @@ -1198,7 +1242,7 @@ impl SecureChannel {
signed_range,
signed_range.end
);
let verification_key = self.verification_key();
let verification_key = self.verification_key(token_id)?;
self.security_policy.symmetric_verify_signature(
verification_key,
&dst[signed_range.clone()],
Expand All @@ -1222,7 +1266,7 @@ impl SecureChannel {

// Decrypt encrypted portion
let mut decrypted_tmp = vec![0u8; ciphertext_size + 16]; // tmp includes +16 for blocksize
let (key, iv) = self.decryption_keys();
let (key, iv) = self.decryption_keys(token_id)?;

trace!(
"Secure decrypt called with encrypted range {:?}",
Expand Down Expand Up @@ -1250,7 +1294,7 @@ impl SecureChannel {
signed_range,
signature_range
);
let verification_key = self.verification_key();
let verification_key = self.verification_key(token_id)?;
self.security_policy.symmetric_verify_signature(
verification_key,
&dst[signed_range],
Expand Down
10 changes: 5 additions & 5 deletions lib/src/core/tests/chunk.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn set_chunk_sequence_number(
sequence_number: u32,
) -> u32 {
// Read the sequence header
let mut chunk_info = chunk.chunk_info(&secure_channel).unwrap();
let mut chunk_info = chunk.chunk_info(secure_channel).unwrap();
let old_sequence_number = chunk_info.sequence_header.sequence_number;
chunk_info.sequence_header.sequence_number = sequence_number;
// Write the sequence header out again with new value
Expand All @@ -69,7 +69,7 @@ fn set_chunk_request_id(
request_id: u32,
) -> u32 {
// Read the sequence header
let mut chunk_info = chunk.chunk_info(&secure_channel).unwrap();
let mut chunk_info = chunk.chunk_info(secure_channel).unwrap();
let old_request_id = chunk_info.sequence_header.request_id;
chunk_info.sequence_header.request_id = request_id;
// Write the sequence header out again with new value
Expand Down Expand Up @@ -341,7 +341,7 @@ fn chunk_open_secure_channel() {
assert_eq!(request_header.timestamp.ticks(), 131284521470690000);
assert_eq!(request_header.request_handle, 1);
assert!(request_header.return_diagnostics.is_empty());
assert_eq!(request_header.audit_entry_id.is_null(), true);
assert!(request_header.audit_entry_id.is_null());
assert_eq!(request_header.timeout_hint, 0);
}

Expand Down Expand Up @@ -408,7 +408,7 @@ fn open_secure_channel_response() {
};
assert_eq!(response.response_header.request_handle, 0);
assert_eq!(response.response_header.service_result, StatusCode::Good);
assert_eq!(response.response_header.string_table.is_none(), true);
assert!(response.response_header.string_table.is_none());
assert_eq!(response.server_nonce, ByteString::null());
}

Expand Down Expand Up @@ -464,7 +464,7 @@ fn security_policy_symmetric_encrypt_decrypt() {

let mut src2 = vec![0u8; 200];
let decrypted_len = secure_channel2
.symmetric_decrypt_and_verify(&dst, 0..80, 20..100, &mut src2)
.symmetric_decrypt_and_verify(&dst, 0..80, 20..100, 0, &mut src2)
.unwrap();
assert_eq!(decrypted_len, 100);

Expand Down
4 changes: 2 additions & 2 deletions lib/src/core/tests/secure_channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fn test_symmetric_encrypt_decrypt(

let mut encrypted_data = vec![0u8; chunk.data.len() + 4096];
let encrypted_size = secure_channel1
.apply_security(&chunk, &mut encrypted_data[..])
.apply_security(chunk, &mut encrypted_data[..])
.unwrap();
trace!("Result of applying security = {}", encrypted_size);

Expand Down Expand Up @@ -81,7 +81,7 @@ fn test_asymmetric_encrypt_decrypt(

let mut encrypted_data = vec![0u8; chunk.data.len() + 4096];
let encrypted_size = secure_channel
.apply_security(&chunk, &mut encrypted_data[..])
.apply_security(chunk, &mut encrypted_data[..])
.unwrap();
trace!("Result of applying security = {}", encrypted_size);

Expand Down
Loading