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

Security1 #216

Merged
merged 9 commits into from Jun 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Next
Enforce max_message_size and max_chunk_count and tie up a number of p…
…laces where it might have gone wrong. Also start adding a decoding depth limit.
  • Loading branch information
locka99 committed Jun 5, 2022
commit 6fb683c5fec46c6dd347824491c4d93a229da695
12 changes: 12 additions & 0 deletions lib/src/client/builder.rs
Expand Up @@ -273,6 +273,18 @@ impl ClientBuilder {
self.config.session_name = session_name.into();
self
}

/// Set the maximum message size
pub fn max_message_size(mut self, max_message_size: usize) -> Self {
self.config.decoding_options.max_message_size = max_message_size;
self
}

/// Set the max chunk count
pub fn max_chunk_count(mut self, max_chunk_count: usize) -> Self {
self.config.decoding_options.max_chunk_count = max_chunk_count;
self
}
}

#[test]
Expand Down
2 changes: 2 additions & 0 deletions lib/src/client/client.rs
Expand Up @@ -427,10 +427,12 @@ impl Client {
let decoding_options = &self.config.decoding_options;
DecodingOptions {
max_chunk_count: decoding_options.max_chunk_count,
max_message_size: decoding_options.max_message_size,
max_string_length: decoding_options.max_string_length,
max_byte_string_length: decoding_options.max_byte_string_length,
max_array_length: decoding_options.max_array_length,
client_offset: Duration::zero(),
..Default::default()
}
}

Expand Down
3 changes: 2 additions & 1 deletion lib/src/client/comms/tcp_transport.rs
Expand Up @@ -132,6 +132,7 @@ impl ReadState {
let chunks_len = self.chunks.len();
if self.max_chunk_count > 0 && chunks_len > self.max_chunk_count {
error!("too many chunks {}> {}", chunks_len, self.max_chunk_count);
// TODO this code should return an error to be safe
//remove first
let first_req_id = *self.chunks.iter().next().unwrap().0;
self.chunks.remove(&first_req_id);
Expand Down Expand Up @@ -312,7 +313,7 @@ impl TcpTransport {
pub fn connect(&self, endpoint_url: &str) -> Result<(), StatusCode> {
debug_assert!(!self.is_connected(), "Should not try to connect when already connected");
let (host, port) =
hostname_port_from_url(endpoint_url, constants::DEFAULT_OPC_UA_SERVER_PORT)?;
hostname_port_from_url(endpoint_url, crate::core::constants::DEFAULT_OPC_UA_SERVER_PORT)?;

// Resolve the host name into a socket address
let addr = {
Expand Down
4 changes: 3 additions & 1 deletion lib/src/client/config.rs
Expand Up @@ -136,6 +136,8 @@ impl ClientEndpoint {
#[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
pub struct DecodingOptions {
/// Maximum size of a message chunk in bytes. 0 means no limit
pub max_message_size: usize,
/// Maximum number of chunks in a message. 0 means no limit
pub max_chunk_count: usize,
/// Maximum length in bytes (not chars!) of a string. 0 actually means 0, i.e. no string permitted
pub max_string_length: usize,
Expand Down Expand Up @@ -311,7 +313,6 @@ impl ClientConfig {
pki_dir.push(Self::PKI_DIR);

let decoding_options = crate::types::DecodingOptions::default();

ClientConfig {
application_name: application_name.into(),
application_uri: application_uri.into(),
Expand All @@ -334,6 +335,7 @@ impl ClientConfig {
max_string_length: decoding_options.max_string_length,
max_byte_string_length: decoding_options.max_byte_string_length,
max_chunk_count: decoding_options.max_chunk_count,
max_message_size: decoding_options.max_message_size,
},
performance: Performance {
ignore_clock_skew: false,
Expand Down
3 changes: 1 addition & 2 deletions lib/src/client/session/session_state.rs
Expand Up @@ -157,7 +157,6 @@ impl SessionState {
const SEND_BUFFER_SIZE: usize = 65535;
const RECEIVE_BUFFER_SIZE: usize = 65535;
const MAX_BUFFER_SIZE: usize = 65535;
const MAX_CHUNK_COUNT: usize = 0;

pub fn new(
ignore_clock_skew: bool,
Expand All @@ -176,7 +175,7 @@ impl SessionState {
send_buffer_size: Self::SEND_BUFFER_SIZE,
receive_buffer_size: Self::RECEIVE_BUFFER_SIZE,
max_message_size: Self::MAX_BUFFER_SIZE,
max_chunk_count: Self::MAX_CHUNK_COUNT,
max_chunk_count: constants::MAX_CHUNK_COUNT,
request_handle: Handle::new(Self::FIRST_REQUEST_HANDLE),
session_id: NodeId::null(),
authentication_token: NodeId::null(),
Expand Down
3 changes: 2 additions & 1 deletion lib/src/core/comms/message_chunk.rs
Expand Up @@ -158,7 +158,8 @@ impl BinaryEncoder<MessageChunk> for MessageChunk {
})?;

let message_size = chunk_header.message_size as usize;
if decoding_options.max_chunk_count > 0 && message_size > decoding_options.max_chunk_count {
if decoding_options.max_message_size > 0 && message_size > decoding_options.max_message_size
{
// Message_size should be sanity checked and rejected if too large.
Err(StatusCode::BadTcpMessageTooLarge)
} else {
Expand Down
44 changes: 28 additions & 16 deletions lib/src/core/comms/message_writer.rs
Expand Up @@ -66,25 +66,37 @@ impl MessageWriter {
&message,
)?;

// Sequence number monotonically increases per chunk
self.last_sent_sequence_number += chunks.len() as u32;
if self.max_chunk_count > 0 && chunks.len() > self.max_chunk_count {
error!(
"Cannot write message since {} chunks exceeds {} chunk limit",
chunks.len(),
self.max_chunk_count
);
Err(StatusCode::BadCommunicationError)
} else {
// Sequence number monotonically increases per chunk
self.last_sent_sequence_number += chunks.len() as u32;

// Send chunks
// Send chunks

// This max chunk size allows the message to be encoded to a chunk with header + encoding
// which is just slightly larger in size (up to 1024 bytes).
let max_chunk_count = self.buffer.get_ref().len() + 1024;
let mut data = vec![0u8; max_chunk_count];
for chunk in chunks {
trace!("Sending chunk {:?}", chunk);
let size = secure_channel.apply_security(&chunk, &mut data)?;
self.buffer.write(&data[..size]).map_err(|error| {
error!("Error while writing bytes to stream, connection broken, check error {:?}", error);
StatusCode::BadCommunicationError
})?;
// This max chunk size allows the message to be encoded to a chunk with header + encoding
// which is just slightly larger in size (up to 1024 bytes).
let data_buffer_size = self.buffer.get_ref().len() + 1024;
let mut data = vec![0u8; data_buffer_size];
for chunk in chunks {
trace!("Sending chunk {:?}", chunk);
let size = secure_channel.apply_security(&chunk, &mut data)?;
self.buffer.write(&data[..size]).map_err(|error| {
error!(
"Error while writing bytes to stream, connection broken, check error {:?}",
error
);
StatusCode::BadCommunicationError
})?;
}
trace!("Message written");
Ok(request_id)
}
trace!("Message written");
Ok(request_id)
}

pub fn next_request_id(&mut self) -> u32 {
Expand Down
20 changes: 7 additions & 13 deletions lib/src/core/comms/secure_channel.rs
Expand Up @@ -70,12 +70,14 @@ pub struct SecureChannel {
decoding_options: DecodingOptions,
}

impl From<(SecurityPolicy, MessageSecurityMode)> for SecureChannel {
fn from(v: (SecurityPolicy, MessageSecurityMode)) -> Self {
impl SecureChannel {
/// For testing purposes only
#[cfg(test)]
pub fn new_no_certificate_store() -> SecureChannel {
SecureChannel {
role: Role::Unknown,
security_policy: v.0,
security_mode: v.1,
security_policy: SecurityPolicy::None,
security_mode: MessageSecurityMode::None,
secure_channel_id: 0,
token_id: 0,
token_created_at: DateTime::now(),
Expand All @@ -90,14 +92,6 @@ impl From<(SecurityPolicy, MessageSecurityMode)> for SecureChannel {
decoding_options: DecodingOptions::default(),
}
}
}

impl SecureChannel {
/// For testing purposes only
#[cfg(test)]
pub fn new_no_certificate_store() -> SecureChannel {
(SecurityPolicy::None, MessageSecurityMode::None).into()
}

pub fn new(
certificate_store: Arc<RwLock<CertificateStore>>,
Expand Down Expand Up @@ -222,7 +216,7 @@ impl SecureChannel {
}

pub fn decoding_options(&self) -> DecodingOptions {
self.decoding_options
self.decoding_options.clone()
}

/// Test if the secure channel token needs to be renewed. The algorithm determines it needs
Expand Down
3 changes: 1 addition & 2 deletions lib/src/core/comms/security_header.rs
Expand Up @@ -88,8 +88,7 @@ impl BinaryEncoder<AsymmetricSecurityHeader> for AsymmetricSecurityHeader {

// validate sender_certificate_length < MaxCertificateSize
if sender_certificate.value.is_some()
&& sender_certificate.value.as_ref().unwrap().len()
>= constants::MAX_CERTIFICATE_LENGTH as usize
&& sender_certificate.value.as_ref().unwrap().len() >= constants::MAX_CERTIFICATE_LENGTH
{
error!("Sender certificate exceeds max certificate size");
Err(StatusCode::BadDecodingError)
Expand Down
6 changes: 3 additions & 3 deletions lib/src/core/comms/url.rs
Expand Up @@ -6,7 +6,7 @@

use url::Url;

use crate::types::{constants::DEFAULT_OPC_UA_SERVER_PORT, status_code::StatusCode};
use crate::types::status_code::StatusCode;

pub const OPC_TCP_SCHEME: &str = "opc.tcp";

Expand All @@ -16,7 +16,7 @@ fn opc_url_from_str(s: &str) -> Result<Url, ()> {
.map(|mut url| {
if url.port().is_none() {
// If no port is supplied, then treat it as the default port 4840
let _ = url.set_port(Some(DEFAULT_OPC_UA_SERVER_PORT));
let _ = url.set_port(Some(crate::core::constants::DEFAULT_OPC_UA_SERVER_PORT));
}
url
})
Expand Down Expand Up @@ -57,7 +57,7 @@ pub fn server_url_from_endpoint_url(endpoint_url: &str) -> std::result::Result<S
url.set_query(None);
if let Some(port) = url.port() {
// If the port is the default, strip it so the url string omits it.
if port == DEFAULT_OPC_UA_SERVER_PORT {
if port == crate::core::constants::DEFAULT_OPC_UA_SERVER_PORT {
let _ = url.set_port(None);
}
}
Expand Down
7 changes: 7 additions & 0 deletions lib/src/core/mod.rs
Expand Up @@ -93,6 +93,13 @@ pub mod debug {
#[cfg(test)]
pub mod tests;

pub mod constants {
/// Default OPC UA port number. Used by a discovery server. Other servers would normally run
/// on a different port. So OPC UA for Rust does not use this nr by default but it is used
/// implicitly in opc.tcp:// urls and elsewhere.
pub const DEFAULT_OPC_UA_SERVER_PORT: u16 = 4840;
}

pub mod comms;
pub mod config;
pub mod handle;
Expand Down
5 changes: 3 additions & 2 deletions lib/src/core/tests/chunk.rs
Expand Up @@ -36,7 +36,7 @@ fn sample_secure_channel_request_data_security_none() -> MessageChunk {

// Decode chunk from stream
stream.set_position(0);
let decoding_options = DecodingOptions::default();
let decoding_options = DecodingOptions::test();
let chunk = MessageChunk::decode(&mut stream, &decoding_options).unwrap();

println!(
Expand Down Expand Up @@ -102,6 +102,7 @@ fn chunk_multi_encode_decode() {
max_string_length: 65535,
max_byte_string_length: 65535,
max_array_length: 20000, // Need to bump this up because large response uses a large array
..Default::default()
});

let response = make_large_read_response();
Expand Down Expand Up @@ -152,7 +153,7 @@ fn chunk_multi_chunk_intermediate_final() {
.unwrap();
assert!(chunks.len() > 1);

let decoding_options = DecodingOptions::default();
let decoding_options = DecodingOptions::test();

// All chunks except the last should be intermediate, the last should be final
for (i, chunk) in chunks.iter().enumerate() {
Expand Down
4 changes: 2 additions & 2 deletions lib/src/core/tests/comms.rs
Expand Up @@ -24,7 +24,7 @@ fn ack_data() -> Vec<u8> {
#[test]
pub fn hello() {
let mut stream = Cursor::new(hello_data());
let decoding_options = DecodingOptions::default();
let decoding_options = DecodingOptions::test();
let hello = HelloMessage::decode(&mut stream, &decoding_options).unwrap();
println!("hello = {:?}", hello);
assert_eq!(hello.message_header.message_type, MessageType::Hello);
Expand All @@ -43,7 +43,7 @@ pub fn hello() {
#[test]
pub fn acknowledge() {
let mut stream = Cursor::new(ack_data());
let decoding_options = DecodingOptions::default();
let decoding_options = DecodingOptions::test();
let ack = AcknowledgeMessage::decode(&mut stream, &decoding_options).unwrap();
println!("ack = {:?}", ack);
assert_eq!(ack.message_header.message_type, MessageType::Acknowledge);
Expand Down
2 changes: 1 addition & 1 deletion lib/src/core/tests/mod.rs
Expand Up @@ -36,7 +36,7 @@ where
println!("encoded bytes = {:?}", actual);
let mut stream = Cursor::new(actual);

let decoding_options = DecodingOptions::default();
let decoding_options = DecodingOptions::test();
let new_value: T = T::decode(&mut stream, &decoding_options).unwrap();
println!("new value = {:?}", new_value);
assert_eq!(value, new_value);
Expand Down
22 changes: 17 additions & 5 deletions lib/src/server/builder.rs
Expand Up @@ -308,36 +308,48 @@ impl ServerBuilder {
}

/// Set the maximum number of subscriptions in a session
pub fn max_subscriptions(mut self, max_subscriptions: u32) -> Self {
pub fn max_subscriptions(mut self, max_subscriptions: usize) -> Self {
self.config.limits.max_subscriptions = max_subscriptions;
self
}

/// Set the maximum number of monitored items per subscription
pub fn max_monitored_items_per_sub(mut self, max_monitored_items_per_sub: u32) -> Self {
pub fn max_monitored_items_per_sub(mut self, max_monitored_items_per_sub: usize) -> Self {
self.config.limits.max_monitored_items_per_sub = max_monitored_items_per_sub;
self
}

/// Set the max array length in elements
pub fn max_array_length(mut self, max_array_length: u32) -> Self {
pub fn max_array_length(mut self, max_array_length: usize) -> Self {
self.config.limits.max_array_length = max_array_length;
self
}

/// Set the max string length in characters, i.e. if you set max to 1000 characters, then with
/// UTF-8 encoding potentially that's 4000 bytes.
pub fn max_string_length(mut self, max_string_length: u32) -> Self {
pub fn max_string_length(mut self, max_string_length: usize) -> Self {
self.config.limits.max_string_length = max_string_length;
self
}

/// Set the max bytestring length in bytes
pub fn max_byte_string_length(mut self, max_byte_string_length: u32) -> Self {
pub fn max_byte_string_length(mut self, max_byte_string_length: usize) -> Self {
self.config.limits.max_byte_string_length = max_byte_string_length;
self
}

/// Set the maximum message size
pub fn max_message_size(mut self, max_message_size: usize) -> Self {
self.config.limits.max_message_size = max_message_size;
self
}

/// Set the max chunk count
pub fn max_chunk_count(mut self, max_chunk_count: usize) -> Self {
self.config.limits.max_chunk_count = max_chunk_count;
self
}

/// Sets the server to automatically trust client certs. This subverts the
/// authentication during handshake, so only do this if you understand the risks.
pub fn trust_client_certs(mut self) -> Self {
Expand Down