-
Notifications
You must be signed in to change notification settings - Fork 497
storage: Deny localhost/private-ips in storage connections #26186
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
Changes from 5 commits
87073a4
07049d6
108e666
789c8d1
a6e4449
0bc46ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| // Copyright Materialize, Inc. and contributors. All rights reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License in the LICENSE file at the | ||
| // root of this repository, or online at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| use std::io; | ||
| use std::net::IpAddr; | ||
|
|
||
| use tokio::net::lookup_host; | ||
|
|
||
| const DUMMY_PORT: u16 = 11111; | ||
|
|
||
| /// Resolves a host address and ensures it is a global address when `enforce_global` is set. | ||
| /// This parameter is useful when connecting to user-defined unverified addresses. | ||
| pub async fn resolve_address( | ||
| mut host: &str, | ||
| enforce_global: bool, | ||
| ) -> Result<Vec<IpAddr>, io::Error> { | ||
| // `net::lookup_host` requires a port to be specified, but we don't care about the port. | ||
| let mut port = DUMMY_PORT; | ||
| // If a port is already specified, use it and remove it from the host. | ||
| if let Some(idx) = host.find(':') { | ||
| if let Ok(p) = host[idx + 1..].parse() { | ||
| port = p; | ||
| host = &host[..idx]; | ||
| } | ||
| } | ||
|
|
||
| let mut addrs = lookup_host((host, port)).await?; | ||
| let mut ips = Vec::new(); | ||
| while let Some(addr) = addrs.next() { | ||
| let ip = addr.ip(); | ||
| if enforce_global && !is_global(ip) { | ||
| Err(io::Error::new( | ||
| io::ErrorKind::AddrNotAvailable, | ||
| "address is not global", | ||
| ))? | ||
| } else { | ||
| ips.push(ip); | ||
| } | ||
| } | ||
|
|
||
| if ips.len() == 0 { | ||
| Err(io::Error::new( | ||
| io::ErrorKind::AddrNotAvailable, | ||
| "no addresses found", | ||
| ))? | ||
| } | ||
| Ok(ips) | ||
| } | ||
|
|
||
| fn is_global(addr: IpAddr) -> bool { | ||
| // TODO: Switch to `addr.is_global()` once stable: https://github.com/rust-lang/rust/issues/27709 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These ones seem to cover more cases, so probably worth making a tracking issue in our repo for this! (actually let me know if you want help on the process of trying to push these through stabilization!)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good call - made a tracking issue https://github.com/MaterializeInc/materialize/issues/26649 |
||
| match addr { | ||
| IpAddr::V4(ip) => { | ||
| !(ip.is_unspecified() || ip.is_private() || ip.is_loopback() || ip.is_link_local()) | ||
| } | ||
| IpAddr::V6(ip) => !(ip.is_loopback() || ip.is_unspecified()), | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |||||||||
| // the Business Source License, use of this software will be governed | ||||||||||
| // by the Apache License, Version 2.0. | ||||||||||
|
|
||||||||||
| use std::net::IpAddr; | ||||||||||
| use std::time::Duration; | ||||||||||
|
|
||||||||||
| use mz_ore::future::{InTask, OreFutureExt}; | ||||||||||
|
|
@@ -38,7 +39,7 @@ macro_rules! bail_generic { | |||||||||
| #[derive(Debug, PartialEq, Clone)] | ||||||||||
| pub enum TunnelConfig { | ||||||||||
| /// Establish a direct TCP connection to the database host. | ||||||||||
| Direct, | ||||||||||
| Direct { resolved_ips: Option<Vec<IpAddr>> }, | ||||||||||
| /// Establish a TCP connection to the database via an SSH tunnel. | ||||||||||
| /// This means first establishing an SSH connection to a bastion host, | ||||||||||
| /// and then opening a separate connection from that host to the database. | ||||||||||
|
|
@@ -226,7 +227,24 @@ impl Config { | |||||||||
| })?; | ||||||||||
|
|
||||||||||
| match &self.tunnel { | ||||||||||
| TunnelConfig::Direct => { | ||||||||||
| TunnelConfig::Direct { resolved_ips } => { | ||||||||||
| if let Some(ips) = resolved_ips { | ||||||||||
| let host = match postgres_config.get_hosts() { | ||||||||||
| [Host::Tcp(host)] => host, | ||||||||||
| _ => bail_generic!( | ||||||||||
| "only TCP connections to a single PostgreSQL server are supported" | ||||||||||
| ), | ||||||||||
| } | ||||||||||
| .to_owned(); | ||||||||||
| // The number of 'host' and 'hostaddr' values must be the same. | ||||||||||
|
||||||||||
| // The number of 'host' and 'hostaddr' values must be the same. | |
| // Associate each resolved ip with the exact same, singular host, for tls verification. | |
| // We are required to do this dance because `tokio-postgres` enforces that the number of 'host' and 'hostaddr' values must be the same. | |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you document what
Nonemeans? (here and for pg)