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

Fixes to symmetric decryption #377

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 12 additions & 12 deletions lib/src/client/session/services/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,6 @@ impl Session {
if let SupportedMessage::CreateSessionResponse(response) = response {
process_service_result(&response.response_header)?;

let session_id = {
self.session_id.store(Arc::new(response.session_id.clone()));
response.session_id.clone()
};
self.auth_token
.store(Arc::new(response.authentication_token));

self.channel.update_from_created_session(
&response.server_nonce,
&response.server_certificate,
)?;

let security_policy = self.channel.security_policy();

if security_policy != SecurityPolicy::None {
Expand Down Expand Up @@ -107,6 +95,18 @@ impl Session {
}
}

let session_id = {
self.session_id.store(Arc::new(response.session_id.clone()));
response.session_id.clone()
};
self.auth_token
.store(Arc::new(response.authentication_token));

self.channel.update_from_created_session(
&response.server_nonce,
&response.server_certificate,
)?;

Ok(session_id)
} else {
Err(process_unexpected_response(response))
Expand Down
13 changes: 10 additions & 3 deletions lib/src/client/transport/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,11 @@ impl TransportState {

match chunk_info.message_header.is_final {
MessageIsFinalType::Intermediate => {
debug!(
"receive chunk intermediate {}:{}",
trace!(
"receive chunk intermediate {}:{}. Length {}",
chunk_info.sequence_header.request_id,
chunk_info.sequence_header.sequence_number
chunk_info.sequence_header.sequence_number,
chunk_info.body_length
);
message_state.chunks.push(MessageChunkWithChunkInfo {
header: chunk_info,
Expand All @@ -210,6 +211,12 @@ impl TransportState {
.send(Err(StatusCode::BadCommunicationError));
}
MessageIsFinalType::Final => {
trace!(
"receive chunk final {}:{}. Length {}",
chunk_info.sequence_header.request_id,
chunk_info.sequence_header.sequence_number,
chunk_info.body_length
);
message_state.chunks.push(MessageChunkWithChunkInfo {
header: chunk_info,
data_with_header: chunk.data,
Expand Down
18 changes: 13 additions & 5 deletions lib/src/core/comms/secure_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ impl SecureChannel {
receiver_certificate_thumbprint,
)
};
debug!(
trace!(
"AsymmetricSecurityHeader = {:?}",
asymmetric_security_header
);
Expand Down Expand Up @@ -747,10 +747,10 @@ impl SecureChannel {
&mut decrypted_data,
)?;

// Now we need to strip off signature
// Value returned from symmetric_decrypt_and_verify is the end of the actual decrypted data.
Self::update_message_size_and_truncate(
decrypted_data,
decrypted_size - signature_size,
decrypted_size,
&self.decoding_options,
)?
} else {
Expand Down Expand Up @@ -1205,7 +1205,7 @@ impl SecureChannel {
&dst[signed_range.end..],
)?;

Ok(encrypted_range.end)
Ok(signed_range.end)
}
MessageSecurityMode::SignAndEncrypt => {
self.expect_supported_security_policy();
Expand Down Expand Up @@ -1251,12 +1251,20 @@ impl SecureChannel {
signature_range
);
let verification_key = self.verification_key();
let signature_start = signature_range.start;
self.security_policy.symmetric_verify_signature(
verification_key,
&dst[signed_range],
&dst[signature_range],
)?;
Ok(encrypted_range.end)

let key_size = key.key_length();

// Verify that the padding is correct and get the padded range.
let padding_range = self.verify_padding(dst, key_size, signature_start)?;

// Decrypted range minus padding and signature.
Ok(padding_range.start)
}
MessageSecurityMode::Invalid => {
// Use the security policy to decrypt the block using the token
Expand Down
2 changes: 1 addition & 1 deletion lib/src/core/comms/tcp_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub const CHUNK_INTERMEDIATE: u8 = b'C';
pub const CHUNK_FINAL_ERROR: u8 = b'A';

/// Minimum size in bytes than any single message chunk can be
pub const MIN_CHUNK_SIZE: usize = 8196;
pub const MIN_CHUNK_SIZE: usize = 8192;

/// Size in bytes of an OPC UA message header
pub const MESSAGE_HEADER_LEN: usize = 8;
Expand Down
3 changes: 2 additions & 1 deletion lib/src/core/tests/chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,8 @@ fn security_policy_symmetric_encrypt_decrypt() {
let decrypted_len = secure_channel2
.symmetric_decrypt_and_verify(&dst, 0..80, 20..100, &mut src2)
.unwrap();
assert_eq!(decrypted_len, 100);
// End is at 100 - signature (20) - 1
assert_eq!(decrypted_len, 79);

// Compare the data, not the signature
assert_eq!(&src[..80], &src2[..80]);
Expand Down
8 changes: 4 additions & 4 deletions lib/src/core/tests/hello.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ fn valid_buffer_sizes() {
endpoint_url: UAString::null(),
};
assert!(!h.is_valid_buffer_sizes());
h.receive_buffer_size = 8195;
h.receive_buffer_size = 8191;
assert!(!h.is_valid_buffer_sizes());
h.send_buffer_size = 8195;
h.send_buffer_size = 8191;
assert!(!h.is_valid_buffer_sizes());
h.receive_buffer_size = 8196;
h.receive_buffer_size = 8192;
assert!(!h.is_valid_buffer_sizes());
h.send_buffer_size = 8196;
h.send_buffer_size = 8192;
assert!(h.is_valid_buffer_sizes());
}
Loading