From 1d959d2680c4659f4ef38a486d559d2fb627da24 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 5 Jul 2024 10:58:31 +0100 Subject: [PATCH] Apply dest/target validation to VPC routes on PUT 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. --- nexus/src/app/vpc_router.rs | 82 ++++++++++++++++---------- nexus/types/src/external_api/params.rs | 6 +- openapi/nexus.json | 10 ++-- 3 files changed, 59 insertions(+), 39 deletions(-) diff --git a/nexus/src/app/vpc_router.rs b/nexus/src/app/vpc_router.rs index fdc834a14c2..b15edf124dd 100644 --- a/nexus/src/app/vpc_router.rs +++ b/nexus/src/app/vpc_router.rs @@ -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 (¶ms.destination, ¶ms.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( + ¶ms.destination, + ¶ms.target, + RouterRouteKind::Custom, + )?; let id = Uuid::new_v4(); let route = db::model::RouterRoute::new( @@ -297,6 +271,12 @@ impl super::Nexus { } } + validate_user_route_targets( + ¶ms.destination, + ¶ms.target, + db_route.kind.0, + )?; + let out = self .db_datastore .router_update_route(&opctx, &authz_route, params.clone().into()) @@ -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(()) +} diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 6d92f2b1ba5..1776045ffdb 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -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, - /// 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, /// Name or ID of the router pub router: NameOrId, @@ -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, - /// 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, /// Name or ID of the router pub router: Option, @@ -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, - /// 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, /// Name or ID of the router, only required if `route` is provided as a `Name` pub router: Option, diff --git a/openapi/nexus.json b/openapi/nexus.json index ac38d1703ad..589c3a223e6 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -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" } @@ -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" } @@ -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" } @@ -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" } @@ -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" }