Skip to content

Commit

Permalink
Apply dest/target validation to VPC routes on PUT
Browse files Browse the repository at this point in the history
VPC custom route creation applied various validations on the contents of
a route. These include preventing mixed v4/v6 addresses (helpful) and
disallowing system-only targets such as subnets/vpcs (mandatory). This
PR ensures that these checks occur on modification of Default or Custom
routes by users.

Fixes #6003. Also fixes some typos identified by @elains-oxide.
  • Loading branch information
FelixMcFelix committed Jul 5, 2024
1 parent bb083ae commit 1d959d2
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 39 deletions.
82 changes: 51 additions & 31 deletions nexus/src/app/vpc_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,37 +204,11 @@ impl super::Nexus {
));
}

// Validate route destinations/targets at this stage:
// - mixed explicit v4 and v6 are disallowed.
// - users cannot specify 'Vpc' as a custom router dest/target.
// - users cannot specify 'Subnet' as a custom router target.
// - the only internet gateway we support today is 'outbound'.
match (&params.destination, &params.target) {
(RouteDestination::Ip(IpAddr::V4(_)), RouteTarget::Ip(IpAddr::V4(_)))
| (RouteDestination::Ip(IpAddr::V6(_)), RouteTarget::Ip(IpAddr::V6(_)))
| (RouteDestination::IpNet(IpNet::V4(_)), RouteTarget::Ip(IpAddr::V4(_)))
| (RouteDestination::IpNet(IpNet::V6(_)), RouteTarget::Ip(IpAddr::V6(_))) => {},

(RouteDestination::Ip(_), RouteTarget::Ip(_))
| (RouteDestination::IpNet(_), RouteTarget::Ip(_))
=> return Err(Error::invalid_request(
"cannot mix explicit IPv4 and IPv6 addresses between destination and target"
)),

(RouteDestination::Vpc(_), _) | (_, RouteTarget::Vpc(_)) => return Err(Error::invalid_request(
"VPCs cannot be used as a destination or target in custom routers"
)),

(_, RouteTarget::Subnet(_)) => return Err(Error::invalid_request(
"subnets cannot be used as a target in custom routers"
)),

(_, RouteTarget::InternetGateway(n)) if n.as_str() != "outbound" => return Err(Error::invalid_request(
"'outbound' is currently the only valid internet gateway"
)),

_ => {},
};
validate_user_route_targets(
&params.destination,
&params.target,
RouterRouteKind::Custom,
)?;

let id = Uuid::new_v4();
let route = db::model::RouterRoute::new(
Expand Down Expand Up @@ -297,6 +271,12 @@ impl super::Nexus {
}
}

validate_user_route_targets(
&params.destination,
&params.target,
db_route.kind.0,
)?;

let out = self
.db_datastore
.router_update_route(&opctx, &authz_route, params.clone().into())
Expand Down Expand Up @@ -356,3 +336,43 @@ impl super::Nexus {
Ok(())
}
}

/// Validate route destinations/targets for a user-specified route:
/// - mixed explicit v4 and v6 are disallowed.
/// - users cannot specify 'Vpc' as a custom/default router dest/target.
/// - users cannot specify 'Subnet' as a custom/default router target.
/// - the only internet gateway we support today is 'outbound'.
fn validate_user_route_targets(
dest: &RouteDestination,
target: &RouteTarget,
route_type: RouterRouteKind,
) -> Result<(), Error> {
match (dest, target) {
(RouteDestination::Ip(IpAddr::V4(_)), RouteTarget::Ip(IpAddr::V4(_)))
| (RouteDestination::Ip(IpAddr::V6(_)), RouteTarget::Ip(IpAddr::V6(_)))
| (RouteDestination::IpNet(IpNet::V4(_)), RouteTarget::Ip(IpAddr::V4(_)))
| (RouteDestination::IpNet(IpNet::V6(_)), RouteTarget::Ip(IpAddr::V6(_))) => {},

(RouteDestination::Ip(_), RouteTarget::Ip(_))
| (RouteDestination::IpNet(_), RouteTarget::Ip(_))
=> return Err(Error::invalid_request(
"cannot mix explicit IPv4 and IPv6 addresses between destination and target"
)),

(RouteDestination::Vpc(_), _) | (_, RouteTarget::Vpc(_)) => return Err(Error::invalid_request(
format!("users cannot specify VPCs as a destination or target in {route_type} routes")
)),

(_, RouteTarget::Subnet(_)) => return Err(Error::invalid_request(
format!("subnets cannot be used as a target in {route_type} routers")
)),

(_, RouteTarget::InternetGateway(n)) if n.as_str() != "outbound" => return Err(Error::invalid_request(
"'outbound' is currently the only valid internet gateway"
)),

_ => {},
};

Ok(())
}
6 changes: 3 additions & 3 deletions nexus/types/src/external_api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ pub struct SubnetSelector {
pub struct RouterSelector {
/// Name or ID of the project, only required if `vpc` is provided as a `Name`
pub project: Option<NameOrId>,
/// Name or ID of the VPC, only required if `subnet` is provided as a `Name`
/// Name or ID of the VPC, only required if `router` is provided as a `Name`
pub vpc: Option<NameOrId>,
/// Name or ID of the router
pub router: NameOrId,
Expand All @@ -269,7 +269,7 @@ pub struct RouterSelector {
pub struct OptionalRouterSelector {
/// Name or ID of the project, only required if `vpc` is provided as a `Name`
pub project: Option<NameOrId>,
/// Name or ID of the VPC, only required if `subnet` is provided as a `Name`
/// Name or ID of the VPC, only required if `router` is provided as a `Name`
pub vpc: Option<NameOrId>,
/// Name or ID of the router
pub router: Option<NameOrId>,
Expand All @@ -279,7 +279,7 @@ pub struct OptionalRouterSelector {
pub struct RouteSelector {
/// Name or ID of the project, only required if `vpc` is provided as a `Name`
pub project: Option<NameOrId>,
/// Name or ID of the VPC, only required if `subnet` is provided as a `Name`
/// Name or ID of the VPC, only required if `router` is provided as a `Name`
pub vpc: Option<NameOrId>,
/// Name or ID of the router, only required if `route` is provided as a `Name`
pub router: Option<NameOrId>,
Expand Down
10 changes: 5 additions & 5 deletions openapi/nexus.json
Original file line number Diff line number Diff line change
Expand Up @@ -8401,7 +8401,7 @@
{
"in": "query",
"name": "vpc",
"description": "Name or ID of the VPC, only required if `subnet` is provided as a `Name`",
"description": "Name or ID of the VPC, only required if `router` is provided as a `Name`",
"schema": {
"$ref": "#/components/schemas/NameOrId"
}
Expand Down Expand Up @@ -8458,7 +8458,7 @@
{
"in": "query",
"name": "vpc",
"description": "Name or ID of the VPC, only required if `subnet` is provided as a `Name`",
"description": "Name or ID of the VPC, only required if `router` is provided as a `Name`",
"schema": {
"$ref": "#/components/schemas/NameOrId"
}
Expand Down Expand Up @@ -8531,7 +8531,7 @@
{
"in": "query",
"name": "vpc",
"description": "Name or ID of the VPC, only required if `subnet` is provided as a `Name`",
"description": "Name or ID of the VPC, only required if `router` is provided as a `Name`",
"schema": {
"$ref": "#/components/schemas/NameOrId"
}
Expand Down Expand Up @@ -8591,7 +8591,7 @@
{
"in": "query",
"name": "vpc",
"description": "Name or ID of the VPC, only required if `subnet` is provided as a `Name`",
"description": "Name or ID of the VPC, only required if `router` is provided as a `Name`",
"schema": {
"$ref": "#/components/schemas/NameOrId"
}
Expand Down Expand Up @@ -8661,7 +8661,7 @@
{
"in": "query",
"name": "vpc",
"description": "Name or ID of the VPC, only required if `subnet` is provided as a `Name`",
"description": "Name or ID of the VPC, only required if `router` is provided as a `Name`",
"schema": {
"$ref": "#/components/schemas/NameOrId"
}
Expand Down

0 comments on commit 1d959d2

Please sign in to comment.