Skip to content

Commit

Permalink
Always use the supplied hostname when connecting (locka99#90)
Browse files Browse the repository at this point in the history 
When you use the function `connect_to_endpoint_id` the code uses a function called `find_matching_endpoint` which substitutes the advertised hostname with the one the client supplied.

But when using `connect_to_endpoint` it uses a function called `find_server_endpoint` which does successfully match even if the hostname is different, but in that case it doesn’t substitude advertised hostname.

This can cause issues when using IP addresses i.c.w. a NAT connection, have a difference in the actual hostname vs the used DNS name or when the server uses a hostname that is not resolvable so you need to connect using an IP address.

It seems this was also an issue with `connect_to_endpoint_id` as there is a mention of this in the code:

```
// Issue locka99#16, locka99#17 - the server may advertise an endpoint whose hostname is inaccessible
// to the client so substitute the advertised hostname with the one the client supplied.
```

So I figured we could just update the code to use that same code path in `connect_to_endpoint` as well. Hope this is indeed OK and that there isn’t a specific reason for not allowing this when using `connect_to_endpoint`.

Thanks!
  • Loading branch information
@svanharmelen
svanharmelen authored Mar 1, 2021
1 parent bbdef77 commit ec5647e
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 81 deletions.
66 changes: 3 additions & 63 deletions client/src/client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::{
use opcua_core::{
comms::url::{
hostname_from_url, is_opc_ua_binary_url, is_valid_opc_ua_url, server_url_from_endpoint_url,
url_matches, url_matches_except_host, url_with_replaced_hostname,
url_matches_except_host, url_with_replaced_hostname,
},
config::Config,
};
Expand Down Expand Up @@ -259,7 +259,7 @@ impl Client {
// Find the server endpoint that matches the one desired
let security_policy = SecurityPolicy::from_str(endpoint.security_policy_uri.as_ref())
.map_err(|_| StatusCode::BadSecurityPolicyRejected)?;
let server_endpoint = Client::find_server_endpoint(
let server_endpoint = Client::find_matching_endpoint(
&server_endpoints,
endpoint.endpoint_url.as_ref(),
security_policy,
Expand Down Expand Up @@ -431,7 +431,7 @@ impl Client {
/// fn main() {
/// let mut client = Client::new(ClientConfig::load(&PathBuf::from("./myclient.conf")).unwrap());
/// if let Ok(endpoints) = client.get_server_endpoints_from_url("opc.tcp://foo:1234") {
/// if let Some(endpoint) = Client::find_server_endpoint(&endpoints, "opc.tcp://foo:1234/mypath", SecurityPolicy::None, MessageSecurityMode::None) {
/// if let Some(endpoint) = Client::find_matching_endpoint(&endpoints, "opc.tcp://foo:1234/mypath", SecurityPolicy::None, MessageSecurityMode::None) {
/// //...
/// }
/// }
Expand Down Expand Up @@ -602,66 +602,6 @@ impl Client {
}
}

/// Finds a matching endpoint, one that most closely matches the host, path, security policy
/// and security mode used as inputs. The function will fallback to omit the host in its
/// comparison if no exact match is found.
///
/// # Example
///
/// ```no_run
/// use opcua_client::prelude::*;
/// let endpoints = [
/// EndpointDescription::from("opc.tcp://foo:123"),
/// EndpointDescription::from("opc.tcp://foo:456")
/// ];
/// // Some result, including for hostname mismatch
/// assert!(Client::find_server_endpoint(&endpoints, "opc.tcp://foo:123", SecurityPolicy::None, MessageSecurityMode::None).is_some());
/// assert!(Client::find_server_endpoint(&endpoints, "opc.tcp://bar:123", SecurityPolicy::None, MessageSecurityMode::None).is_some());
/// assert!(Client::find_server_endpoint(&endpoints, "opc.tcp://foo:456", SecurityPolicy::None, MessageSecurityMode::None).is_some());
/// // None
/// assert!(Client::find_server_endpoint(&endpoints, "opc.tcp://foo:222", SecurityPolicy::None, MessageSecurityMode::None).is_none());
/// assert!(Client::find_server_endpoint(&endpoints, "opc.tcp://foo:123", SecurityPolicy::Basic128Rsa15, MessageSecurityMode::None).is_none());
/// assert!(Client::find_server_endpoint(&endpoints, "opc.tcp://foo:123", SecurityPolicy::None, MessageSecurityMode::Sign).is_none());
/// ```
///
pub fn find_server_endpoint<T>(
endpoints: &[EndpointDescription],
endpoint_url: T,
security_policy: SecurityPolicy,
security_mode: MessageSecurityMode,
) -> Option<EndpointDescription>
where
T: Into<String>,
{
// Iterate the supplied endpoints looking for the closest match.
let security_policy_uri = security_policy.to_uri();
let endpoint_url = endpoint_url.into();

// Do an exact match first
let result = endpoints
.iter()
.find(|e| {
e.security_mode == security_mode
&& e.security_policy_uri.as_ref() == security_policy_uri
&& url_matches(e.endpoint_url.as_ref(), &endpoint_url)
})
.cloned();

// If something was found, return it, otherwise try a fuzzier match that ignores the hostname.
if result.is_some() {
result
} else {
endpoints
.iter()
.find(|e| {
e.security_mode == security_mode
&& e.security_policy_uri.as_ref() == security_policy_uri
&& url_matches_except_host(e.endpoint_url.as_ref(), &endpoint_url)
})
.cloned()
}
}

/// Determine if we recognize the security of this endpoint
fn is_supported_endpoint(&self, endpoint: &EndpointDescription) -> bool {
if let Ok(security_policy) = SecurityPolicy::from_str(endpoint.security_policy_uri.as_ref())
Expand Down
18 changes: 0 additions & 18 deletions core/src/comms/url.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,6 @@ pub fn url_with_replaced_hostname(url: &str, hostname: &str) -> Result<String, (
Ok(url.into_string())
}

/// Test if the two urls match exactly. Strings are fed into a url parser and compared to resolve
/// ambiguities like paths, case sensitive portions, encoding etc.
pub fn url_matches(url1: &str, url2: &str) -> bool {
if let Ok(url1) = opc_url_from_str(url1) {
if let Ok(url2) = opc_url_from_str(url2) {
return url1 == url2;
} else {
error!("Cannot parse url \"{}\"", url2);
}
} else {
error!("Cannot parse url \"{}\"", url1);
}
false
}

/// Test if the two urls match except for the hostname. Can be used by a server whose endpoint doesn't
/// exactly match the incoming connection, e.g. 127.0.0.1 vs localhost.
pub fn url_matches_except_host(url1: &str, url2: &str) -> bool {
Expand Down Expand Up @@ -134,9 +119,6 @@ mod tests {

#[test]
fn url_matches_test() {
assert!(url_matches("opc.tcp://foo/", "opc.tcp://foo:4840/"));
assert!(!url_matches("opc.tcp://foo/", "opc.tcp://foo:4841/"));
assert!(!url_matches("opc.tcp://foo/xyz", "opc.tcp://bar/xyz"));
assert!(url_matches_except_host(
"opc.tcp://localhost/xyz",
"opc.tcp://127.0.0.1/xyz"
Expand Down

0 comments on commit ec5647e

Please sign in to comment.