From 08d6ec74ee9ae409de15629919c382f4ec1ee2a4 Mon Sep 17 00:00:00 2001 From: Matthew Sanabria Date: Tue, 6 Oct 2026 16:36:45 -0400 Subject: [PATCH 1/9] nexus: /v1/system/certificates APIs Added the following system APIs to create, read, delete, and list certificates. ``` system_certificate_create POST /v1/system/certificates system_certificate_delete DELETE /v1/system/certificates/{certificate} system_certificate_list GET /v1/system/certificates system_certificate_view GET /v1/system/certificates/{certificate} ``` These APIs are mostly identical to the current `/v1/certificates` APIs, with the difference that callers must now specify the silo. This allows clients that authenticate to one silo to manage the certificates for another silo. For example, platform engineering teams that operate within a management silo to create silos for their users. Relates to SSE-498. Relates to https://github.com/oxidecomputer/terraform-provider-oxide/issues/875. --- nexus/db-lookup/src/lookup.rs | 3 +- .../src/db/datastore/certificate.rs | 28 +- nexus/external-api/output/nexus_tags.txt | 4 + nexus/external-api/src/lib.rs | 87 +++- nexus/src/app/certificate.rs | 77 +++- nexus/src/app/external_endpoints.rs | 2 +- nexus/src/external_api/http_entrypoints.rs | 103 +++++ nexus/tests/integration_tests/certificates.rs | 391 ++++++++++++++++++ nexus/tests/integration_tests/endpoints.rs | 27 ++ nexus/tests/integration_tests/unauthorized.rs | 6 + .../nexus-2026101300.0.0-b0ac51.json.gitstub | 1 + ....json => nexus-2026101500.0.0-7cfc90.json} | 218 +++++++++- openapi/nexus/nexus-latest.json | 2 +- 13 files changed, 913 insertions(+), 36 deletions(-) create mode 100644 openapi/nexus/nexus-2026101300.0.0-b0ac51.json.gitstub rename openapi/nexus/{nexus-2026101300.0.0-b0ac51.json => nexus-2026101500.0.0-7cfc90.json} (99%) diff --git a/nexus/db-lookup/src/lookup.rs b/nexus/db-lookup/src/lookup.rs index b59a5e79e3e..40a6725432d 100644 --- a/nexus/db-lookup/src/lookup.rs +++ b/nexus/db-lookup/src/lookup.rs @@ -880,7 +880,8 @@ lookup_resource! { ancestors = [ "Silo" ], lookup_by_name = true, soft_deletes = true, - primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ], + visible_outside_silo = true } lookup_resource! { diff --git a/nexus/db-queries/src/db/datastore/certificate.rs b/nexus/db-queries/src/db/datastore/certificate.rs index 8629e66b72f..6880684477e 100644 --- a/nexus/db-queries/src/db/datastore/certificate.rs +++ b/nexus/db-queries/src/db/datastore/certificate.rs @@ -19,7 +19,7 @@ use nexus_db_errors::public_error_from_diesel; use nexus_types::identity::Resource; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DeleteResult; -use omicron_common::api::external::InternalContext; +use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::ResourceType; use omicron_common::api::external::http_pagination::PaginatedBy; @@ -30,15 +30,20 @@ impl DataStore { pub async fn certificate_create( &self, opctx: &OpContext, + authz_silo: &authz::Silo, certificate: Certificate, ) -> CreateResult { use nexus_db_schema::schema::certificate::dsl; - let authz_silo = opctx - .authn - .silo_required() - .internal_context("creating a Certificate")?; - let authz_cert_list = authz::SiloCertificateList::new(authz_silo); + if certificate.silo_id != authz_silo.id() { + return Err(Error::internal_error(&format!( + "certificate silo {} does not match authorized silo {}", + certificate.silo_id, + authz_silo.id(), + ))); + } + let authz_cert_list = + authz::SiloCertificateList::new(authz_silo.clone()); opctx.authorize(authz::Action::CreateChild, &authz_cert_list).await?; let name = certificate.name().clone(); @@ -66,17 +71,14 @@ impl DataStore { opctx: &OpContext, kind: Option, pagparams: &PaginatedBy<'_>, - silo_only: bool, + authz_silo: Option<&authz::Silo>, ) -> ListResultVec { use nexus_db_schema::schema::certificate::dsl; - let silo = if silo_only { - let authz_silo = opctx - .authn - .silo_required() - .internal_context("listing Certificates")?; + let silo = if let Some(authz_silo) = authz_silo { let silo_id = authz_silo.id(); - let authz_cert_list = authz::SiloCertificateList::new(authz_silo); + let authz_cert_list = + authz::SiloCertificateList::new(authz_silo.clone()); opctx .authorize(authz::Action::ListChildren, &authz_cert_list) .await?; diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index 7b9bbd9edc6..e8f0de675f1 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -314,6 +314,10 @@ silo_user_view GET /v1/system/users/{user_id} silo_utilization_list GET /v1/system/utilization/silos silo_utilization_view GET /v1/system/utilization/silos/{silo} silo_view GET /v1/system/silos/{silo} +system_certificate_create POST /v1/system/certificates +system_certificate_delete DELETE /v1/system/certificates/{certificate} +system_certificate_list GET /v1/system/certificates +system_certificate_view GET /v1/system/certificates/{certificate} system_quotas_list GET /v1/system/silo-quotas user_builtin_list GET /v1/system/users-builtin user_builtin_view GET /v1/system/users-builtin/{user} diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 428bfc9227e..b7e34842be2 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -88,6 +88,7 @@ api_versions!([ // | date-based version should be at the top of the list. // v // (next_yyyy_mm_dd_nn, IDENT), + (2026_10_15_00, SYSTEM_CERTIFICATES), (2026_10_13_00, ARTIFACT_HASH_SCHEMA), (2026_09_15_00, REMOVE_SILO_DISCOVERABLE), (2026_09_11_00, ALERT_PAYLOAD), @@ -5078,11 +5079,10 @@ pub trait NexusExternalApi { // Certificates - /// List certificates for external endpoints + /// List TLS certificates for the current silo /// - /// Returns a list of TLS certificates used for the external API (for the - /// current Silo). These are sorted by creation date, with the most recent - /// certificates appearing first. + /// Returns a list of TLS certificates for the current silo, sorted by creation + /// date with the most recent certificates appearing first. #[endpoint { method = GET, path = "/v1/certificates", @@ -5096,10 +5096,10 @@ pub trait NexusExternalApi { HttpError, >; - /// Create system-wide x.509 certificate + /// Create TLS certificate for the current silo /// - /// This certificate is automatically used by the Oxide Control plane to serve - /// external connections. + /// This certificate is automatically used by the Oxide control plane to serve + /// API requests. #[endpoint { method = POST, path = "/v1/certificates", @@ -5110,9 +5110,9 @@ pub trait NexusExternalApi { new_cert: TypedBody, ) -> Result, HttpError>; - /// Fetch certificate + /// Fetch TLS certificate within the current silo /// - /// Returns the details of a specific certificate + /// Returns the details of a specific certificate. #[endpoint { method = GET, path = "/v1/certificates/{certificate}", @@ -5123,7 +5123,7 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result, HttpError>; - /// Delete certificate + /// Delete TLS certificate within the current silo /// /// Permanently delete a certificate. This operation cannot be undone. #[endpoint { @@ -5136,6 +5136,73 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result; + /// List TLS certificates for the specified silo + /// + /// Returns a list of TLS certificates for the specified silo, sorted by + /// creation date with the most recent certificates appearing first. + #[endpoint { + method = GET, + path = "/v1/system/certificates", + tags = ["system/silos"], + versions = VERSION_SYSTEM_CERTIFICATES.., + }] + async fn system_certificate_list( + rqctx: RequestContext, + query_params: Query>, + ) -> Result< + HttpResponseOk>, + HttpError, + >; + + /// Create TLS certificate for the specified silo + /// + /// This certificate is automatically used by the Oxide control plane to serve + /// API requests. + #[endpoint { + method = POST, + path = "/v1/system/certificates", + tags = ["system/silos"], + versions = VERSION_SYSTEM_CERTIFICATES.., + }] + async fn system_certificate_create( + rqctx: RequestContext, + query_params: Query, + new_cert: TypedBody, + ) -> Result, HttpError>; + + /// Fetch TLS certificate for the specified silo + /// + /// Returns the details of a specific certificate. + #[endpoint { + method = GET, + path = "/v1/system/certificates/{certificate}", + tags = ["system/silos"], + versions = VERSION_SYSTEM_CERTIFICATES.., + }] + async fn system_certificate_view( + rqctx: RequestContext, + path_params: Path, + query_params: Query, + ) -> Result, HttpError>; + + /// Delete TLS certificate for the specified silo + /// + /// Permanently delete a certificate. This operation cannot be undone. + /// + /// The silo must be specified if the certificate is specified by name, and + /// must not be specified if the certificate is specified by ID. + #[endpoint { + method = DELETE, + path = "/v1/system/certificates/{certificate}", + tags = ["system/silos"], + versions = VERSION_SYSTEM_CERTIFICATES.., + }] + async fn system_certificate_delete( + rqctx: RequestContext, + path_params: Path, + query_params: Query, + ) -> Result; + /// Create address lot #[endpoint { method = POST, diff --git a/nexus/src/app/certificate.rs b/nexus/src/app/certificate.rs index d31edaf57a1..63871ca3c17 100644 --- a/nexus/src/app/certificate.rs +++ b/nexus/src/app/certificate.rs @@ -14,8 +14,10 @@ use nexus_db_queries::db::model::ServiceKind; use nexus_types::external_api::certificate; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DeleteResult; +use omicron_common::api::external::Error; use omicron_common::api::external::InternalContext; use omicron_common::api::external::ListResultVec; +use omicron_common::api::external::LookupResult; use omicron_common::api::external::NameOrId; use omicron_common::api::external::http_pagination::PaginatedBy; use ref_cast::RefCast; @@ -36,6 +38,36 @@ impl super::Nexus { } } + /// Look up a certificate in any silo + /// + /// The silo must be provided if `certificate` is a name, and must not be + /// provided if `certificate` is an ID. + pub fn system_certificate_lookup<'a>( + &'a self, + opctx: &'a OpContext, + silo: Option, + certificate: NameOrId, + ) -> LookupResult> { + match (silo, certificate) { + (None, NameOrId::Id(id)) => { + Ok(LookupPath::new(opctx, &self.db_datastore) + .certificate_id(id)) + } + (Some(silo), NameOrId::Name(name)) => Ok(self + .silo_lookup(opctx, silo)? + .certificate_name_owned(name.into())), + (Some(_), NameOrId::Id(_)) => Err(Error::invalid_request( + "when providing certificate as an ID, silo should not be \ + specified", + )), + (None, NameOrId::Name(_)) => Err(Error::invalid_request( + "certificate should either be a UUID or silo should be \ + specified", + )), + } + } + + /// Create a certificate in the current user's silo pub(crate) async fn certificate_create( &self, opctx: &OpContext, @@ -45,6 +77,31 @@ impl super::Nexus { .authn .silo_required() .internal_context("creating a Certificate")?; + self.certificate_create_for_silo(opctx, &authz_silo, params).await + } + + /// Create a certificate in the specified silo + pub(crate) async fn system_certificate_create( + &self, + opctx: &OpContext, + silo_lookup: &lookup::Silo<'_>, + params: certificate::CertificateCreate, + ) -> CreateResult { + let (authz_silo,) = silo_lookup.lookup_for(authz::Action::Read).await?; + self.certificate_create_for_silo(opctx, &authz_silo, params).await + } + + async fn certificate_create_for_silo( + &self, + opctx: &OpContext, + authz_silo: &authz::Silo, + params: certificate::CertificateCreate, + ) -> CreateResult { + // Check this up front so that we don't use the elevated context below + // on behalf of a caller who can't create certificates in this silo. + let authz_cert_list = + authz::SiloCertificateList::new(authz_silo.clone()); + opctx.authorize(authz::Action::CreateChild, &authz_cert_list).await?; // The `opctx` we received is going to be checked for permission to // create a cert below in `db_datastore.certificate_create`, but first @@ -73,7 +130,7 @@ impl super::Nexus { )?; let cert = self .db_datastore - .certificate_create(opctx, new_certificate) + .certificate_create(opctx, authz_silo, new_certificate) .await?; match kind { @@ -94,8 +151,24 @@ impl super::Nexus { opctx: &OpContext, pagparams: &PaginatedBy<'_>, ) -> ListResultVec { + let authz_silo = opctx + .authn + .silo_required() + .internal_context("listing Certificates")?; + self.db_datastore + .certificate_list_for(opctx, None, pagparams, Some(&authz_silo)) + .await + } + + pub(crate) async fn system_certificates_list( + &self, + opctx: &OpContext, + silo_lookup: &lookup::Silo<'_>, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + let (authz_silo,) = silo_lookup.lookup_for(authz::Action::Read).await?; self.db_datastore - .certificate_list_for(opctx, None, pagparams, true) + .certificate_list_for(opctx, None, pagparams, Some(&authz_silo)) .await } diff --git a/nexus/src/app/external_endpoints.rs b/nexus/src/app/external_endpoints.rs index e4ec35ad01d..b73fac5b819 100644 --- a/nexus/src/app/external_endpoints.rs +++ b/nexus/src/app/external_endpoints.rs @@ -544,7 +544,7 @@ pub(crate) async fn read_all_endpoints( opctx, Some(ServiceKind::Nexus), &PaginatedBy::Id(p.current_pagparams()), - false, + None, ) .await?; paginator = p.found_batch(&batch, &|s: &Certificate| s.id()); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index ba14b79c1bf..8cba568b8af 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -4008,6 +4008,109 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } + async fn system_certificate_list( + rqctx: RequestContext, + query_params: Query>, + ) -> Result>, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let query = query_params.into_inner(); + let pag_params = data_page_params_for(&rqctx, &query)?; + let scan_params = ScanByNameOrId::from_query(&query)?; + let paginated_by = name_or_id_pagination(&pag_params, scan_params)?; + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let silo_lookup = + nexus.silo_lookup(&opctx, scan_params.selector.silo.clone())?; + let certs = nexus + .system_certificates_list(&opctx, &silo_lookup, &paginated_by) + .await? + .into_iter() + .map(|d| d.try_into()) + .collect::, Error>>()?; + Ok(HttpResponseOk(ScanByNameOrId::results_page( + &query, + certs, + &marker_for_name_or_id, + )?)) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn system_certificate_create( + rqctx: RequestContext, + query_params: Query, + new_cert: TypedBody, + ) -> Result, HttpError> { + audit_and_time(&rqctx, |opctx, nexus| async move { + let query = query_params.into_inner(); + let new_cert_params = new_cert.into_inner(); + let silo_lookup = nexus.silo_lookup(&opctx, query.silo)?; + let cert = nexus + .system_certificate_create( + &opctx, + &silo_lookup, + new_cert_params, + ) + .await?; + Ok(HttpResponseCreated(cert.try_into()?)) + }) + .await + } + + async fn system_certificate_view( + rqctx: RequestContext, + path_params: Path, + query_params: Query, + ) -> Result, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.context.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let opctx = + crate::context::op_context_for_external_api(&rqctx).await?; + let (.., cert) = nexus + .system_certificate_lookup( + &opctx, + query.silo, + path.certificate, + )? + .fetch() + .await?; + Ok(HttpResponseOk(cert.try_into()?)) + }; + apictx + .context + .external_latencies + .instrument_dropshot_handler(&rqctx, handler) + .await + } + + async fn system_certificate_delete( + rqctx: RequestContext, + path_params: Path, + query_params: Query, + ) -> Result { + audit_and_time(&rqctx, |opctx, nexus| async move { + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let certificate_lookup = nexus.system_certificate_lookup( + &opctx, + query.silo, + path.certificate, + )?; + nexus.certificate_delete(&opctx, certificate_lookup).await?; + Ok(HttpResponseDeleted()) + }) + .await + } + async fn networking_address_lot_create( rqctx: RequestContext, new_address_lot: TypedBody, diff --git a/nexus/tests/integration_tests/certificates.rs b/nexus/tests/integration_tests/certificates.rs index 664a740d66d..1913856d906 100644 --- a/nexus/tests/integration_tests/certificates.rs +++ b/nexus/tests/integration_tests/certificates.rs @@ -6,19 +6,28 @@ use display_error_chain::ErrorChainExt; use dropshot::HttpErrorResponseBody; +use dropshot::ResultsPage; use dropshot::test_util::ClientTestContext; use futures::TryStreamExt; use http::StatusCode; use http::method::Method; use internal_dns_types::names::DNS_ZONE_EXTERNAL_TESTING; +use nexus_db_lookup::LookupPath; +use nexus_db_queries::context::OpContext; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::resource_helpers::create_certificate; +use nexus_test_utils::resource_helpers::create_local_user; +use nexus_test_utils::resource_helpers::create_silo; use nexus_test_utils::resource_helpers::delete_certificate; +use nexus_test_utils::resource_helpers::grant_iam; +use nexus_test_utils::resource_helpers::test_params; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::certificate::{ Certificate, CertificateCreate, ServiceUsingCertificate, }; +use nexus_types::external_api::policy::{FleetRole, SiloRole}; +use nexus_types::external_api::silo::SiloIdentityMode; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::internal::nexus::Certificate as InternalCertificate; use omicron_test_utils::certificates::CertificateChain; @@ -301,6 +310,388 @@ async fn test_cannot_create_certificate_with_incorrect_subject_alt_name( } } +#[nexus_test] +async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + + // The privileged user lives in the default test silo. Create another silo + // to manage certificates in. + let other_silo = + create_silo(client, "other-silo", SiloIdentityMode::LocalOnly).await; + let other_silo_name = other_silo.identity.name.as_str(); + let other_silo_id = other_silo.identity.id; + let system_certs_url = + format!("/v1/system/certificates?silo={other_silo_name}"); + let system_certs_list = async |silo: &str| -> Vec { + NexusRequest::iter_collection_authn( + client, + "/v1/system/certificates", + &format!("silo={silo}"), + None, + ) + .await + .expect("failed to list certificates") + .all_items + }; + + assert!(system_certs_list(other_silo_name).await.is_empty()); + + // A certificate that's not valid for the target silo is rejected. + let bad_chain = CertificateChain::new(format!( + "{}.sys.{}", + cptestctx.silo_name, cptestctx.external_dns_zone_name + )); + let bad_params = CertificateCreate { + identity: IdentityMetadataCreateParams { + name: CERT_NAME.parse().unwrap(), + description: String::new(), + }, + cert: bad_chain.cert_chain_as_pem(), + key: bad_chain.end_cert_private_key_as_pem(), + service: ServiceUsingCertificate::ExternalApi, + }; + let error = NexusRequest::expect_failure_with_body( + client, + StatusCode::BAD_REQUEST, + Method::POST, + &system_certs_url, + &bad_params, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + assert!( + error.message.contains("Certificate not valid for"), + "unexpected error: {}", + error.message + ); + + // Create a certificate in the other silo. + let chain = CertificateChain::new(cptestctx.wildcard_silo_dns_name()); + let params = CertificateCreate { + cert: chain.cert_chain_as_pem(), + key: chain.end_cert_private_key_as_pem(), + ..bad_params + }; + let created: Certificate = + NexusRequest::objects_post(client, &system_certs_url, ¶ms) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap() + .await; + assert_eq!(created.identity.name, CERT_NAME); + + // It shows up in the other silo, not the caller's silo. + let list = system_certs_list(other_silo_name).await; + assert_eq!(list.len(), 1); + assert_eq!(list[0].identity.id, created.identity.id); + assert!(certs_list(client).await.is_empty()); + assert!(system_certs_list(cptestctx.silo_name.as_str()).await.is_empty()); + let datastore = cptestctx.server.server_context().nexus.datastore(); + let opctx = OpContext::for_tests( + cptestctx.logctx.log.new(slog::o!()), + datastore.clone(), + ); + let (.., db_cert) = LookupPath::new(&opctx, datastore) + .certificate_id(created.identity.id) + .fetch() + .await + .unwrap(); + assert_eq!(db_cert.silo_id, other_silo_id); + + // Names are unique per silo. + let error = NexusRequest::expect_failure_with_body( + client, + StatusCode::BAD_REQUEST, + Method::POST, + &system_certs_url, + ¶ms, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + assert_eq!( + error.message, + format!("already exists: certificate \"{CERT_NAME}\"") + ); + + // Fetch by name (with silo) and by ID (without silo). + let by_name_url = + format!("/v1/system/certificates/{CERT_NAME}?silo={other_silo_name}"); + let by_id_url = format!("/v1/system/certificates/{}", created.identity.id); + for url in [&by_name_url, &by_id_url] { + let fetched: Certificate = NexusRequest::object_get(client, url) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap() + .await; + assert_eq!(fetched.identity.id, created.identity.id); + } + + // The silo must be given with a name and must not be given with an ID. + for (url, expected) in [ + ( + format!("/v1/system/certificates/{CERT_NAME}"), + "certificate should either be a UUID or silo should be specified", + ), + ( + format!( + "/v1/system/certificates/{}?silo={other_silo_name}", + created.identity.id + ), + "when providing certificate as an ID, silo should not be \ + specified", + ), + ] { + for method in [Method::GET, Method::DELETE] { + let error = NexusRequest::expect_failure( + client, + StatusCode::BAD_REQUEST, + method, + &url, + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body::() + .unwrap(); + assert_eq!(error.message, expected); + } + } + + // A name in the wrong silo is not found. + NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + Method::GET, + &format!( + "/v1/system/certificates/{CERT_NAME}?silo={}", + cptestctx.silo_name + ), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + + // Delete by name, recreate, and delete by ID. + NexusRequest::object_delete(client, &by_name_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + assert!(system_certs_list(other_silo_name).await.is_empty()); + let created: Certificate = + NexusRequest::objects_post(client, &system_certs_url, ¶ms) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap() + .await; + NexusRequest::object_delete( + client, + &format!("/v1/system/certificates/{}", created.identity.id), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + assert!(system_certs_list(other_silo_name).await.is_empty()); +} + +#[nexus_test] +async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + + // Create a silo with a silo administrator. + let silo = + create_silo(client, "admin-silo", SiloIdentityMode::LocalOnly).await; + let silo_name = silo.identity.name.as_str(); + let admin_id = create_local_user( + client, + &silo, + &"admin".parse().unwrap(), + test_params::UserPassword::LoginDisallowed, + ) + .await + .id; + grant_iam( + client, + &format!("/v1/system/silos/{silo_name}"), + SiloRole::Admin, + admin_id, + AuthnMode::PrivilegedUser, + ) + .await; + + // Create a fleet viewer. Fleet viewers can read every silo but can't + // manage any silo's certificates. + let viewer_id = create_local_user( + client, + &silo, + &"fleet-viewer".parse().unwrap(), + test_params::UserPassword::LoginDisallowed, + ) + .await + .id; + grant_iam( + client, + "/v1/system", + FleetRole::Viewer, + viewer_id, + AuthnMode::PrivilegedUser, + ) + .await; + + let chain = CertificateChain::new(cptestctx.wildcard_silo_dns_name()); + let params = CertificateCreate { + identity: IdentityMetadataCreateParams { + name: CERT_NAME.parse().unwrap(), + description: String::new(), + }, + cert: chain.cert_chain_as_pem(), + key: chain.end_cert_private_key_as_pem(), + service: ServiceUsingCertificate::ExternalApi, + }; + + // Create a certificate in the default test silo as a fleet administrator. + let other_silo_name = cptestctx.silo_name.as_str(); + let other_url = format!("/v1/system/certificates?silo={other_silo_name}"); + let other_cert: Certificate = + NexusRequest::objects_post(client, &other_url, ¶ms) + .authn_as(AuthnMode::PrivilegedUser) + .execute_and_parse_unwrap() + .await; + + // The silo administrator can manage certificates in their own silo. + let url = format!("/v1/system/certificates?silo={silo_name}"); + let created: Certificate = + NexusRequest::objects_post(client, &url, ¶ms) + .authn_as(AuthnMode::SiloUser(admin_id)) + .execute_and_parse_unwrap() + .await; + let list: ResultsPage = NexusRequest::object_get(client, &url) + .authn_as(AuthnMode::SiloUser(admin_id)) + .execute_and_parse_unwrap() + .await; + assert_eq!(list.items.len(), 1); + assert_eq!(list.items[0].identity.id, created.identity.id); + let by_name_url = + format!("/v1/system/certificates/{CERT_NAME}?silo={silo_name}"); + let by_id_url = format!("/v1/system/certificates/{}", created.identity.id); + for url in [&by_name_url, &by_id_url] { + let fetched: Certificate = NexusRequest::object_get(client, url) + .authn_as(AuthnMode::SiloUser(admin_id)) + .execute_and_parse_unwrap() + .await; + assert_eq!(fetched.identity.id, created.identity.id); + } + + // But not in another silo, whether by silo, by name, or by ID. + NexusRequest::expect_failure_with_body( + client, + StatusCode::NOT_FOUND, + Method::POST, + &other_url, + ¶ms, + ) + .authn_as(AuthnMode::SiloUser(admin_id)) + .execute() + .await + .unwrap(); + let other_by_name_url = + format!("/v1/system/certificates/{CERT_NAME}?silo={other_silo_name}"); + let other_by_id_url = + format!("/v1/system/certificates/{}", other_cert.identity.id); + for (method, url) in [ + (Method::GET, &other_url), + (Method::GET, &other_by_name_url), + (Method::DELETE, &other_by_name_url), + (Method::GET, &other_by_id_url), + (Method::DELETE, &other_by_id_url), + ] { + NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + method, + url, + ) + .authn_as(AuthnMode::SiloUser(admin_id)) + .execute() + .await + .unwrap(); + } + + // A fleet viewer can't create certificates. This must fail on + // authorization before the certificate is validated against the silo's + // DNS names, so an invalid certificate also produces a 403. + let bad_chain = CertificateChain::new("bogus.example.com"); + let bad_params = CertificateCreate { + cert: bad_chain.cert_chain_as_pem(), + key: bad_chain.end_cert_private_key_as_pem(), + ..params.clone() + }; + for params in [¶ms, &bad_params] { + NexusRequest::expect_failure_with_body( + client, + StatusCode::FORBIDDEN, + Method::POST, + &other_url, + params, + ) + .authn_as(AuthnMode::SiloUser(viewer_id)) + .execute() + .await + .unwrap(); + } + + // A fleet viewer can't list, view, or delete certificates either. + NexusRequest::expect_failure( + client, + StatusCode::FORBIDDEN, + Method::GET, + &other_url, + ) + .authn_as(AuthnMode::SiloUser(viewer_id)) + .execute() + .await + .unwrap(); + for method in [Method::GET, Method::DELETE] { + NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + method, + &other_by_id_url, + ) + .authn_as(AuthnMode::SiloUser(viewer_id)) + .execute() + .await + .unwrap(); + } + + // Certificates in the silo can't be accessed by other silos' users. + for method in [Method::GET, Method::DELETE] { + NexusRequest::expect_failure( + client, + StatusCode::NOT_FOUND, + method, + &by_id_url, + ) + .authn_as(AuthnMode::UnprivilegedUser) + .execute() + .await + .unwrap(); + } + NexusRequest::object_delete(client, &by_id_url) + .authn_as(AuthnMode::SiloUser(admin_id)) + .execute() + .await + .unwrap(); +} + #[tokio::test] async fn test_silo_certificates() { // The goal of this test is to verify that Nexus serves the correct TLS diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index e836d718f3d..2eb034232ca 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -870,6 +870,10 @@ pub static DEMO_CERTIFICATE_NAME: LazyLock = pub const DEMO_CERTIFICATES_URL: &'static str = "/v1/certificates"; pub const DEMO_CERTIFICATE_URL: &'static str = "/v1/certificates/demo-certificate"; +pub const DEMO_SYSTEM_CERTIFICATES_URL: &'static str = + "/v1/system/certificates?silo=demo-silo"; +pub const DEMO_SYSTEM_CERTIFICATE_URL: &'static str = + "/v1/system/certificates/demo-certificate?silo=demo-silo"; pub static DEMO_CERTIFICATE: LazyLock = LazyLock::new(|| { CertificateChain::new(format!("*.sys.{DNS_ZONE_EXTERNAL_TESTING}")) }); @@ -3333,6 +3337,29 @@ pub static VERIFY_ENDPOINTS: LazyLock> = LazyLock::new( AllowedMethod::Delete, ], }, + VerifyEndpoint { + url: &DEMO_SYSTEM_CERTIFICATES_URL, + // Like the SAML identity provider endpoints, these refer to a + // non-default Silo that unprivileged users can't see. + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Post( + serde_json::to_value(&*DEMO_CERTIFICATE_CREATE) + .unwrap(), + ), + ], + }, + VerifyEndpoint { + url: &DEMO_SYSTEM_CERTIFICATE_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Delete, + ], + }, /* External Networking */ VerifyEndpoint { url: &DEMO_SWITCH_PORT_URL, diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 06223786b00..6053d04b4ea 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -535,6 +535,12 @@ static SETUP_REQUESTS: LazyLock> = LazyLock::new(|| { body: serde_json::to_value(&*DEMO_CERTIFICATE_CREATE).unwrap(), id_routes: vec![], }, + // Create a Certificate in a non-default Silo + SetupReq::Post { + url: &DEMO_SYSTEM_CERTIFICATES_URL, + body: serde_json::to_value(&*DEMO_CERTIFICATE_CREATE).unwrap(), + id_routes: vec![], + }, // Create a Support Bundle SetupReq::Post { url: &SUPPORT_BUNDLES_URL, diff --git a/openapi/nexus/nexus-2026101300.0.0-b0ac51.json.gitstub b/openapi/nexus/nexus-2026101300.0.0-b0ac51.json.gitstub new file mode 100644 index 00000000000..94322a54b9f --- /dev/null +++ b/openapi/nexus/nexus-2026101300.0.0-b0ac51.json.gitstub @@ -0,0 +1 @@ +4afe5893cfd6b197daba807bb11936a0ab0b7aba:openapi/nexus/nexus-2026101300.0.0-b0ac51.json diff --git a/openapi/nexus/nexus-2026101300.0.0-b0ac51.json b/openapi/nexus/nexus-2026101500.0.0-7cfc90.json similarity index 99% rename from openapi/nexus/nexus-2026101300.0.0-b0ac51.json rename to openapi/nexus/nexus-2026101500.0.0-7cfc90.json index 80a6496f6f1..d826ee9f5c1 100644 --- a/openapi/nexus/nexus-2026101300.0.0-b0ac51.json +++ b/openapi/nexus/nexus-2026101500.0.0-7cfc90.json @@ -7,7 +7,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "2026101300.0.0" + "version": "2026101500.0.0" }, "paths": { "/device/auth": { @@ -1971,8 +1971,8 @@ "tags": [ "silos" ], - "summary": "List certificates for external endpoints", - "description": "Returns a list of TLS certificates used for the external API (for the current Silo). These are sorted by creation date, with the most recent certificates appearing first.", + "summary": "List TLS certificates for the current silo", + "description": "Returns a list of TLS certificates for the current silo, sorted by creation date with the most recent certificates appearing first.", "operationId": "certificate_list", "parameters": [ { @@ -2029,8 +2029,8 @@ "tags": [ "silos" ], - "summary": "Create system-wide x.509 certificate", - "description": "This certificate is automatically used by the Oxide Control plane to serve external connections.", + "summary": "Create TLS certificate for the current silo", + "description": "This certificate is automatically used by the Oxide control plane to serve API requests.", "operationId": "certificate_create", "requestBody": { "content": { @@ -2067,8 +2067,8 @@ "tags": [ "silos" ], - "summary": "Fetch certificate", - "description": "Returns the details of a specific certificate", + "summary": "Fetch TLS certificate within the current silo", + "description": "Returns the details of a specific certificate.", "operationId": "certificate_view", "parameters": [ { @@ -2104,7 +2104,7 @@ "tags": [ "silos" ], - "summary": "Delete certificate", + "summary": "Delete TLS certificate within the current silo", "description": "Permanently delete a certificate. This operation cannot be undone.", "operationId": "certificate_delete", "parameters": [ @@ -7417,6 +7417,208 @@ } } }, + "/v1/system/certificates": { + "get": { + "tags": [ + "system/silos" + ], + "summary": "List TLS certificates for the specified silo", + "description": "Returns a list of TLS certificates for the specified silo, sorted by creation date with the most recent certificates appearing first.", + "operationId": "system_certificate_list", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "silo", + "description": "Name or ID of the silo", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameOrIdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CertificateResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [ + "silo" + ] + } + }, + "post": { + "tags": [ + "system/silos" + ], + "summary": "Create TLS certificate for the specified silo", + "description": "This certificate is automatically used by the Oxide control plane to serve API requests.", + "operationId": "system_certificate_create", + "parameters": [ + { + "in": "query", + "name": "silo", + "description": "Name or ID of the silo", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CertificateCreate" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Certificate" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/v1/system/certificates/{certificate}": { + "get": { + "tags": [ + "system/silos" + ], + "summary": "Fetch TLS certificate for the specified silo", + "description": "Returns the details of a specific certificate.", + "operationId": "system_certificate_view", + "parameters": [ + { + "in": "path", + "name": "certificate", + "description": "Name or ID of the certificate", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "silo", + "description": "Name or ID of the silo", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Certificate" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "delete": { + "tags": [ + "system/silos" + ], + "summary": "Delete TLS certificate for the specified silo", + "description": "Permanently delete a certificate. This operation cannot be undone.\n\nThe silo must be specified if the certificate is specified by name, and must not be specified if the certificate is specified by ID.", + "operationId": "system_certificate_delete", + "parameters": [ + { + "in": "path", + "name": "certificate", + "description": "Name or ID of the certificate", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "silo", + "description": "Name or ID of the silo", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "204": { + "description": "successful deletion" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/hardware/disk-adoption-request": { "put": { "tags": [ diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index 105abd38524..c7075d64c04 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026101300.0.0-b0ac51.json \ No newline at end of file +nexus-2026101500.0.0-7cfc90.json \ No newline at end of file From de0cb50a193e0b24b378946970ce7466d7756ed7 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 09:46:34 -0500 Subject: [PATCH 2/9] Use resource helpers in system certificate tests --- nexus/tests/integration_tests/certificates.rs | 195 ++++++++---------- 1 file changed, 85 insertions(+), 110 deletions(-) diff --git a/nexus/tests/integration_tests/certificates.rs b/nexus/tests/integration_tests/certificates.rs index 1913856d906..28a18eded9c 100644 --- a/nexus/tests/integration_tests/certificates.rs +++ b/nexus/tests/integration_tests/certificates.rs @@ -16,11 +16,19 @@ use nexus_db_lookup::LookupPath; use nexus_db_queries::context::OpContext; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; +use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::resource_helpers::create_certificate; use nexus_test_utils::resource_helpers::create_local_user; use nexus_test_utils::resource_helpers::create_silo; use nexus_test_utils::resource_helpers::delete_certificate; use nexus_test_utils::resource_helpers::grant_iam; +use nexus_test_utils::resource_helpers::object_create; +use nexus_test_utils::resource_helpers::object_create_error; +use nexus_test_utils::resource_helpers::object_delete; +use nexus_test_utils::resource_helpers::object_delete_error; +use nexus_test_utils::resource_helpers::object_get; +use nexus_test_utils::resource_helpers::object_get_error; +use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_test_utils::resource_helpers::test_params; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::certificate::{ @@ -323,15 +331,12 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { let system_certs_url = format!("/v1/system/certificates?silo={other_silo_name}"); let system_certs_list = async |silo: &str| -> Vec { - NexusRequest::iter_collection_authn( + objects_list_page_authz( client, - "/v1/system/certificates", - &format!("silo={silo}"), - None, + &format!("/v1/system/certificates?silo={silo}"), ) .await - .expect("failed to list certificates") - .all_items + .items }; assert!(system_certs_list(other_silo_name).await.is_empty()); @@ -350,19 +355,13 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { key: bad_chain.end_cert_private_key_as_pem(), service: ServiceUsingCertificate::ExternalApi, }; - let error = NexusRequest::expect_failure_with_body( + let error = object_create_error( client, - StatusCode::BAD_REQUEST, - Method::POST, &system_certs_url, &bad_params, + StatusCode::BAD_REQUEST, ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body::() - .unwrap(); + .await; assert!( error.message.contains("Certificate not valid for"), "unexpected error: {}", @@ -377,10 +376,7 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { ..bad_params }; let created: Certificate = - NexusRequest::objects_post(client, &system_certs_url, ¶ms) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap() - .await; + object_create(client, &system_certs_url, ¶ms).await; assert_eq!(created.identity.name, CERT_NAME); // It shows up in the other silo, not the caller's silo. @@ -402,19 +398,13 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { assert_eq!(db_cert.silo_id, other_silo_id); // Names are unique per silo. - let error = NexusRequest::expect_failure_with_body( + let error = object_create_error( client, - StatusCode::BAD_REQUEST, - Method::POST, &system_certs_url, ¶ms, + StatusCode::BAD_REQUEST, ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body::() - .unwrap(); + .await; assert_eq!( error.message, format!("already exists: certificate \"{CERT_NAME}\"") @@ -425,10 +415,7 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { format!("/v1/system/certificates/{CERT_NAME}?silo={other_silo_name}"); let by_id_url = format!("/v1/system/certificates/{}", created.identity.id); for url in [&by_name_url, &by_id_url] { - let fetched: Certificate = NexusRequest::object_get(client, url) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap() - .await; + let fetched: Certificate = object_get(client, url).await; assert_eq!(fetched.identity.id, created.identity.id); } @@ -447,59 +434,56 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { specified", ), ] { - for method in [Method::GET, Method::DELETE] { - let error = NexusRequest::expect_failure( - client, - StatusCode::BAD_REQUEST, - method, - &url, - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body::() - .unwrap(); + for error in [ + object_get_error(client, &url, StatusCode::BAD_REQUEST).await, + object_delete_error(client, &url, StatusCode::BAD_REQUEST).await, + ] { assert_eq!(error.message, expected); } } // A name in the wrong silo is not found. - NexusRequest::expect_failure( + object_get_error( client, - StatusCode::NOT_FOUND, - Method::GET, &format!( "/v1/system/certificates/{CERT_NAME}?silo={}", cptestctx.silo_name ), + StatusCode::NOT_FOUND, ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap(); + .await; // Delete by name, recreate, and delete by ID. - NexusRequest::object_delete(client, &by_name_url) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap(); + object_delete(client, &by_name_url).await; assert!(system_certs_list(other_silo_name).await.is_empty()); let created: Certificate = - NexusRequest::objects_post(client, &system_certs_url, ¶ms) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap() - .await; - NexusRequest::object_delete( + object_create(client, &system_certs_url, ¶ms).await; + object_delete( client, &format!("/v1/system/certificates/{}", created.identity.id), ) - .authn_as(AuthnMode::PrivilegedUser) + .await; + assert!(system_certs_list(other_silo_name).await.is_empty()); +} + +/// Make a request as `authn` and check that it fails with `status` +async fn expect_status( + client: &ClientTestContext, + authn: AuthnMode, + method: Method, + url: &str, + body: Option<&CertificateCreate>, + status: StatusCode, +) { + NexusRequest::new( + RequestBuilder::new(client, method, url) + .body(body) + .expect_status(Some(status)), + ) + .authn_as(authn) .execute() .await .unwrap(); - assert!(system_certs_list(other_silo_name).await.is_empty()); } #[nexus_test] @@ -526,6 +510,7 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { AuthnMode::PrivilegedUser, ) .await; + let admin = AuthnMode::SiloUser(admin_id); // Create a fleet viewer. Fleet viewers can read every silo but can't // manage any silo's certificates. @@ -545,6 +530,7 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { AuthnMode::PrivilegedUser, ) .await; + let viewer = AuthnMode::SiloUser(viewer_id); let chain = CertificateChain::new(cptestctx.wildcard_silo_dns_name()); let params = CertificateCreate { @@ -561,20 +547,17 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { let other_silo_name = cptestctx.silo_name.as_str(); let other_url = format!("/v1/system/certificates?silo={other_silo_name}"); let other_cert: Certificate = - NexusRequest::objects_post(client, &other_url, ¶ms) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap() - .await; + object_create(client, &other_url, ¶ms).await; // The silo administrator can manage certificates in their own silo. let url = format!("/v1/system/certificates?silo={silo_name}"); let created: Certificate = NexusRequest::objects_post(client, &url, ¶ms) - .authn_as(AuthnMode::SiloUser(admin_id)) + .authn_as(admin.clone()) .execute_and_parse_unwrap() .await; let list: ResultsPage = NexusRequest::object_get(client, &url) - .authn_as(AuthnMode::SiloUser(admin_id)) + .authn_as(admin.clone()) .execute_and_parse_unwrap() .await; assert_eq!(list.items.len(), 1); @@ -584,24 +567,22 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { let by_id_url = format!("/v1/system/certificates/{}", created.identity.id); for url in [&by_name_url, &by_id_url] { let fetched: Certificate = NexusRequest::object_get(client, url) - .authn_as(AuthnMode::SiloUser(admin_id)) + .authn_as(admin.clone()) .execute_and_parse_unwrap() .await; assert_eq!(fetched.identity.id, created.identity.id); } // But not in another silo, whether by silo, by name, or by ID. - NexusRequest::expect_failure_with_body( + expect_status( client, - StatusCode::NOT_FOUND, + admin.clone(), Method::POST, &other_url, - ¶ms, + Some(¶ms), + StatusCode::NOT_FOUND, ) - .authn_as(AuthnMode::SiloUser(admin_id)) - .execute() - .await - .unwrap(); + .await; let other_by_name_url = format!("/v1/system/certificates/{CERT_NAME}?silo={other_silo_name}"); let other_by_id_url = @@ -613,16 +594,15 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { (Method::GET, &other_by_id_url), (Method::DELETE, &other_by_id_url), ] { - NexusRequest::expect_failure( + expect_status( client, - StatusCode::NOT_FOUND, + admin.clone(), method, url, + None, + StatusCode::NOT_FOUND, ) - .authn_as(AuthnMode::SiloUser(admin_id)) - .execute() - .await - .unwrap(); + .await; } // A fleet viewer can't create certificates. This must fail on @@ -635,58 +615,53 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { ..params.clone() }; for params in [¶ms, &bad_params] { - NexusRequest::expect_failure_with_body( + expect_status( client, - StatusCode::FORBIDDEN, + viewer.clone(), Method::POST, &other_url, - params, + Some(params), + StatusCode::FORBIDDEN, ) - .authn_as(AuthnMode::SiloUser(viewer_id)) - .execute() - .await - .unwrap(); + .await; } // A fleet viewer can't list, view, or delete certificates either. - NexusRequest::expect_failure( + expect_status( client, - StatusCode::FORBIDDEN, + viewer.clone(), Method::GET, &other_url, + None, + StatusCode::FORBIDDEN, ) - .authn_as(AuthnMode::SiloUser(viewer_id)) - .execute() - .await - .unwrap(); + .await; for method in [Method::GET, Method::DELETE] { - NexusRequest::expect_failure( + expect_status( client, - StatusCode::NOT_FOUND, + viewer.clone(), method, &other_by_id_url, + None, + StatusCode::NOT_FOUND, ) - .authn_as(AuthnMode::SiloUser(viewer_id)) - .execute() - .await - .unwrap(); + .await; } // Certificates in the silo can't be accessed by other silos' users. for method in [Method::GET, Method::DELETE] { - NexusRequest::expect_failure( + expect_status( client, - StatusCode::NOT_FOUND, + AuthnMode::UnprivilegedUser, method, &by_id_url, + None, + StatusCode::NOT_FOUND, ) - .authn_as(AuthnMode::UnprivilegedUser) - .execute() - .await - .unwrap(); + .await; } NexusRequest::object_delete(client, &by_id_url) - .authn_as(AuthnMode::SiloUser(admin_id)) + .authn_as(admin) .execute() .await .unwrap(); From de50a0b33f7efedafe516260ceec2648119bc86c Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 08:45:53 -0500 Subject: [PATCH 3/9] Test certificate access for non-admin silo member --- nexus/tests/integration_tests/certificates.rs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/nexus/tests/integration_tests/certificates.rs b/nexus/tests/integration_tests/certificates.rs index 28a18eded9c..9e2bde0c130 100644 --- a/nexus/tests/integration_tests/certificates.rs +++ b/nexus/tests/integration_tests/certificates.rs @@ -648,6 +648,57 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { .await; } + // A silo collaborator can read their own silo, so these get past the silo + // lookup, but they still can't list, create, view, or delete certificates. + let collaborator_id = create_local_user( + client, + &silo, + &"collaborator".parse().unwrap(), + test_params::UserPassword::LoginDisallowed, + ) + .await + .id; + grant_iam( + client, + &format!("/v1/system/silos/{silo_name}"), + SiloRole::Collaborator, + collaborator_id, + AuthnMode::PrivilegedUser, + ) + .await; + let collaborator = AuthnMode::SiloUser(collaborator_id); + expect_status( + client, + collaborator.clone(), + Method::GET, + &url, + None, + StatusCode::FORBIDDEN, + ) + .await; + expect_status( + client, + collaborator.clone(), + Method::POST, + &url, + Some(¶ms), + StatusCode::FORBIDDEN, + ) + .await; + for url in [&by_name_url, &by_id_url] { + for method in [Method::GET, Method::DELETE] { + expect_status( + client, + collaborator.clone(), + method, + url, + None, + StatusCode::NOT_FOUND, + ) + .await; + } + } + // Certificates in the silo can't be accessed by other silos' users. for method in [Method::GET, Method::DELETE] { expect_status( From 5296de8e833a33b8169a295b68844faabafee875 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 08:51:51 -0500 Subject: [PATCH 4/9] Scope /v1/certificates/{id} to current silo --- nexus/db-lookup/src/lookup.rs | 3 ++ nexus/src/app/certificate.rs | 41 ++++++++++++++----- nexus/src/external_api/http_entrypoints.rs | 32 +++++++++------ nexus/tests/integration_tests/certificates.rs | 10 +++++ 4 files changed, 64 insertions(+), 22 deletions(-) diff --git a/nexus/db-lookup/src/lookup.rs b/nexus/db-lookup/src/lookup.rs index 40a6725432d..2be7ce3baab 100644 --- a/nexus/db-lookup/src/lookup.rs +++ b/nexus/db-lookup/src/lookup.rs @@ -875,6 +875,9 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", uuid_kind = BuiltInUserKind } ] } +// Visible outside the silo so fleet admins can manage any silo's certificates +// by ID. Lookups on behalf of the current silo must check the silo themselves +// (see `current_silo_certificate_fetch_for` in Nexus). lookup_resource! { name = "Certificate", ancestors = [ "Silo" ], diff --git a/nexus/src/app/certificate.rs b/nexus/src/app/certificate.rs index 63871ca3c17..b2626255613 100644 --- a/nexus/src/app/certificate.rs +++ b/nexus/src/app/certificate.rs @@ -7,6 +7,7 @@ use nexus_db_lookup::LookupPath; use nexus_db_lookup::lookup; use nexus_db_queries::authz; +use nexus_db_queries::authz::ApiResource; use nexus_db_queries::context::OpContext; use nexus_db_queries::db; use nexus_db_queries::db::model::Name; @@ -24,18 +25,39 @@ use ref_cast::RefCast; use uuid::Uuid; impl super::Nexus { - pub fn certificate_lookup<'a>( - &'a self, - opctx: &'a OpContext, - certificate: &'a NameOrId, - ) -> lookup::Certificate<'a> { - match certificate { + /// Fetch a certificate in the current user's silo + /// + /// `Certificate` is `visible_outside_silo` so that fleet admins can reach + /// other silos' certificates by ID through `/v1/system/certificates`. That + /// turns off the check that lookups of silo-scoped resources normally do + /// against the caller's silo, so a lookup by ID can find a certificate in + /// any silo the caller is authorized to see. The `/v1/certificates` + /// endpoints are scoped to the caller's silo, so they still need that + /// check: without it, a fleet admin could view or delete another silo's + /// certificate by ID through them. This does the check here instead. + pub(crate) async fn current_silo_certificate_fetch_for( + &self, + opctx: &OpContext, + certificate: &NameOrId, + action: authz::Action, + ) -> LookupResult<(authz::Certificate, db::model::Certificate)> { + let current_silo = opctx + .authn + .silo_required() + .internal_context("looking up Certificate")?; + let lookup = match certificate { NameOrId::Id(id) => { LookupPath::new(opctx, &self.db_datastore).certificate_id(*id) } NameOrId::Name(name) => LookupPath::new(opctx, &self.db_datastore) .certificate_name(Name::ref_cast(name)), + }; + let (authz_silo, authz_cert, db_cert) = + lookup.fetch_for(action).await?; + if authz_silo.id() != current_silo.id() { + return Err(authz_cert.not_found()); } + Ok((authz_cert, db_cert)) } /// Look up a certificate in any silo @@ -175,11 +197,10 @@ impl super::Nexus { pub(crate) async fn certificate_delete( &self, opctx: &OpContext, - certificate_lookup: lookup::Certificate<'_>, + authz_cert: &authz::Certificate, + db_cert: &db::model::Certificate, ) -> DeleteResult { - let (.., authz_cert, db_cert) = - certificate_lookup.fetch_for(authz::Action::Delete).await?; - self.db_datastore.certificate_delete(opctx, &authz_cert).await?; + self.db_datastore.certificate_delete(opctx, authz_cert).await?; match db_cert.service { ServiceKind::Nexus => { // See the comment in certificate_create() above. diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 8cba568b8af..be7f2798fc4 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -3978,9 +3978,12 @@ impl NexusExternalApi for NexusExternalApiImpl { let path = path_params.into_inner(); let opctx = crate::context::op_context_for_external_api(&rqctx).await?; - let (.., cert) = nexus - .certificate_lookup(&opctx, &path.certificate) - .fetch() + let (_, cert) = nexus + .current_silo_certificate_fetch_for( + &opctx, + &path.certificate, + authz::Action::Read, + ) .await?; Ok(HttpResponseOk(cert.try_into()?)) }; @@ -3997,12 +4000,14 @@ impl NexusExternalApi for NexusExternalApiImpl { ) -> Result { audit_and_time(&rqctx, |opctx, nexus| async move { let path = path_params.into_inner(); - nexus - .certificate_delete( + let (authz_cert, db_cert) = nexus + .current_silo_certificate_fetch_for( &opctx, - nexus.certificate_lookup(&opctx, &path.certificate), + &path.certificate, + authz::Action::Delete, ) .await?; + nexus.certificate_delete(&opctx, &authz_cert, &db_cert).await?; Ok(HttpResponseDeleted()) }) .await @@ -4100,12 +4105,15 @@ impl NexusExternalApi for NexusExternalApiImpl { audit_and_time(&rqctx, |opctx, nexus| async move { let path = path_params.into_inner(); let query = query_params.into_inner(); - let certificate_lookup = nexus.system_certificate_lookup( - &opctx, - query.silo, - path.certificate, - )?; - nexus.certificate_delete(&opctx, certificate_lookup).await?; + let (.., authz_cert, db_cert) = nexus + .system_certificate_lookup( + &opctx, + query.silo, + path.certificate, + )? + .fetch_for(authz::Action::Delete) + .await?; + nexus.certificate_delete(&opctx, &authz_cert, &db_cert).await?; Ok(HttpResponseDeleted()) }) .await diff --git a/nexus/tests/integration_tests/certificates.rs b/nexus/tests/integration_tests/certificates.rs index 9e2bde0c130..c1dddb38033 100644 --- a/nexus/tests/integration_tests/certificates.rs +++ b/nexus/tests/integration_tests/certificates.rs @@ -419,6 +419,16 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { assert_eq!(fetched.identity.id, created.identity.id); } + // The current-silo endpoints can't reach the other silo's certificate by + // ID, even for a fleet admin who can manage it through the system + // endpoints. + let current_silo_by_id_url = + format!("/v1/certificates/{}", created.identity.id); + object_get_error(client, ¤t_silo_by_id_url, StatusCode::NOT_FOUND) + .await; + object_delete_error(client, ¤t_silo_by_id_url, StatusCode::NOT_FOUND) + .await; + // The silo must be given with a name and must not be given with an ID. for (url, expected) in [ ( From b8524b1e8fb9b19b877ed06ce8f6cb423f681b90 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 09:07:57 -0500 Subject: [PATCH 5/9] Rename to silo_certificate_*, shorten docs --- nexus/external-api/output/nexus_tags.txt | 8 ++-- nexus/external-api/src/lib.rs | 39 +++++++------------ nexus/src/app/certificate.rs | 6 +-- nexus/src/external_api/http_entrypoints.rs | 28 ++++--------- ....json => nexus-2026101500.0.0-cda9b6.json} | 32 +++++++-------- openapi/nexus/nexus-latest.json | 2 +- 6 files changed, 44 insertions(+), 71 deletions(-) rename openapi/nexus/{nexus-2026101500.0.0-7cfc90.json => nexus-2026101500.0.0-cda9b6.json} (99%) diff --git a/nexus/external-api/output/nexus_tags.txt b/nexus/external-api/output/nexus_tags.txt index e8f0de675f1..3595b067ec8 100644 --- a/nexus/external-api/output/nexus_tags.txt +++ b/nexus/external-api/output/nexus_tags.txt @@ -299,6 +299,10 @@ scim_token_create POST /v1/system/scim/tokens scim_token_delete DELETE /v1/system/scim/tokens/{token_id} scim_token_list GET /v1/system/scim/tokens scim_token_view GET /v1/system/scim/tokens/{token_id} +silo_certificate_create POST /v1/system/certificates +silo_certificate_delete DELETE /v1/system/certificates/{certificate} +silo_certificate_list GET /v1/system/certificates +silo_certificate_view GET /v1/system/certificates/{certificate} silo_create POST /v1/system/silos silo_delete DELETE /v1/system/silos/{silo} silo_identity_provider_list GET /v1/system/identity-providers @@ -314,10 +318,6 @@ silo_user_view GET /v1/system/users/{user_id} silo_utilization_list GET /v1/system/utilization/silos silo_utilization_view GET /v1/system/utilization/silos/{silo} silo_view GET /v1/system/silos/{silo} -system_certificate_create POST /v1/system/certificates -system_certificate_delete DELETE /v1/system/certificates/{certificate} -system_certificate_list GET /v1/system/certificates -system_certificate_view GET /v1/system/certificates/{certificate} system_quotas_list GET /v1/system/silo-quotas user_builtin_list GET /v1/system/users-builtin user_builtin_view GET /v1/system/users-builtin/{user} diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index b7e34842be2..1f5e3e164f2 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -5079,10 +5079,7 @@ pub trait NexusExternalApi { // Certificates - /// List TLS certificates for the current silo - /// - /// Returns a list of TLS certificates for the current silo, sorted by creation - /// date with the most recent certificates appearing first. + /// List TLS certificates in current silo #[endpoint { method = GET, path = "/v1/certificates", @@ -5096,7 +5093,7 @@ pub trait NexusExternalApi { HttpError, >; - /// Create TLS certificate for the current silo + /// Create TLS certificate in current silo /// /// This certificate is automatically used by the Oxide control plane to serve /// API requests. @@ -5110,9 +5107,7 @@ pub trait NexusExternalApi { new_cert: TypedBody, ) -> Result, HttpError>; - /// Fetch TLS certificate within the current silo - /// - /// Returns the details of a specific certificate. + /// Fetch TLS certificate in current silo #[endpoint { method = GET, path = "/v1/certificates/{certificate}", @@ -5123,9 +5118,7 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result, HttpError>; - /// Delete TLS certificate within the current silo - /// - /// Permanently delete a certificate. This operation cannot be undone. + /// Delete TLS certificate in current silo #[endpoint { method = DELETE, path = "/v1/certificates/{certificate}", @@ -5136,17 +5129,14 @@ pub trait NexusExternalApi { path_params: Path, ) -> Result; - /// List TLS certificates for the specified silo - /// - /// Returns a list of TLS certificates for the specified silo, sorted by - /// creation date with the most recent certificates appearing first. + /// List TLS certificates in silo #[endpoint { method = GET, path = "/v1/system/certificates", tags = ["system/silos"], versions = VERSION_SYSTEM_CERTIFICATES.., }] - async fn system_certificate_list( + async fn silo_certificate_list( rqctx: RequestContext, query_params: Query>, ) -> Result< @@ -5154,7 +5144,7 @@ pub trait NexusExternalApi { HttpError, >; - /// Create TLS certificate for the specified silo + /// Create TLS certificate in silo /// /// This certificate is automatically used by the Oxide control plane to serve /// API requests. @@ -5164,30 +5154,29 @@ pub trait NexusExternalApi { tags = ["system/silos"], versions = VERSION_SYSTEM_CERTIFICATES.., }] - async fn system_certificate_create( + async fn silo_certificate_create( rqctx: RequestContext, query_params: Query, new_cert: TypedBody, ) -> Result, HttpError>; - /// Fetch TLS certificate for the specified silo + /// Fetch TLS certificate in silo /// - /// Returns the details of a specific certificate. + /// The silo must be specified if the certificate is specified by name, and + /// must not be specified if the certificate is specified by ID. #[endpoint { method = GET, path = "/v1/system/certificates/{certificate}", tags = ["system/silos"], versions = VERSION_SYSTEM_CERTIFICATES.., }] - async fn system_certificate_view( + async fn silo_certificate_view( rqctx: RequestContext, path_params: Path, query_params: Query, ) -> Result, HttpError>; - /// Delete TLS certificate for the specified silo - /// - /// Permanently delete a certificate. This operation cannot be undone. + /// Delete TLS certificate in silo /// /// The silo must be specified if the certificate is specified by name, and /// must not be specified if the certificate is specified by ID. @@ -5197,7 +5186,7 @@ pub trait NexusExternalApi { tags = ["system/silos"], versions = VERSION_SYSTEM_CERTIFICATES.., }] - async fn system_certificate_delete( + async fn silo_certificate_delete( rqctx: RequestContext, path_params: Path, query_params: Query, diff --git a/nexus/src/app/certificate.rs b/nexus/src/app/certificate.rs index b2626255613..ab0a4da691f 100644 --- a/nexus/src/app/certificate.rs +++ b/nexus/src/app/certificate.rs @@ -64,7 +64,7 @@ impl super::Nexus { /// /// The silo must be provided if `certificate` is a name, and must not be /// provided if `certificate` is an ID. - pub fn system_certificate_lookup<'a>( + pub fn silo_certificate_lookup<'a>( &'a self, opctx: &'a OpContext, silo: Option, @@ -103,7 +103,7 @@ impl super::Nexus { } /// Create a certificate in the specified silo - pub(crate) async fn system_certificate_create( + pub(crate) async fn silo_certificate_create( &self, opctx: &OpContext, silo_lookup: &lookup::Silo<'_>, @@ -182,7 +182,7 @@ impl super::Nexus { .await } - pub(crate) async fn system_certificates_list( + pub(crate) async fn silo_certificates_list( &self, opctx: &OpContext, silo_lookup: &lookup::Silo<'_>, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index be7f2798fc4..40f12a2816d 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -4013,7 +4013,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn system_certificate_list( + async fn silo_certificate_list( rqctx: RequestContext, query_params: Query>, ) -> Result>, HttpError> { @@ -4029,7 +4029,7 @@ impl NexusExternalApi for NexusExternalApiImpl { let silo_lookup = nexus.silo_lookup(&opctx, scan_params.selector.silo.clone())?; let certs = nexus - .system_certificates_list(&opctx, &silo_lookup, &paginated_by) + .silo_certificates_list(&opctx, &silo_lookup, &paginated_by) .await? .into_iter() .map(|d| d.try_into()) @@ -4047,7 +4047,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn system_certificate_create( + async fn silo_certificate_create( rqctx: RequestContext, query_params: Query, new_cert: TypedBody, @@ -4057,18 +4057,14 @@ impl NexusExternalApi for NexusExternalApiImpl { let new_cert_params = new_cert.into_inner(); let silo_lookup = nexus.silo_lookup(&opctx, query.silo)?; let cert = nexus - .system_certificate_create( - &opctx, - &silo_lookup, - new_cert_params, - ) + .silo_certificate_create(&opctx, &silo_lookup, new_cert_params) .await?; Ok(HttpResponseCreated(cert.try_into()?)) }) .await } - async fn system_certificate_view( + async fn silo_certificate_view( rqctx: RequestContext, path_params: Path, query_params: Query, @@ -4081,11 +4077,7 @@ impl NexusExternalApi for NexusExternalApiImpl { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let (.., cert) = nexus - .system_certificate_lookup( - &opctx, - query.silo, - path.certificate, - )? + .silo_certificate_lookup(&opctx, query.silo, path.certificate)? .fetch() .await?; Ok(HttpResponseOk(cert.try_into()?)) @@ -4097,7 +4089,7 @@ impl NexusExternalApi for NexusExternalApiImpl { .await } - async fn system_certificate_delete( + async fn silo_certificate_delete( rqctx: RequestContext, path_params: Path, query_params: Query, @@ -4106,11 +4098,7 @@ impl NexusExternalApi for NexusExternalApiImpl { let path = path_params.into_inner(); let query = query_params.into_inner(); let (.., authz_cert, db_cert) = nexus - .system_certificate_lookup( - &opctx, - query.silo, - path.certificate, - )? + .silo_certificate_lookup(&opctx, query.silo, path.certificate)? .fetch_for(authz::Action::Delete) .await?; nexus.certificate_delete(&opctx, &authz_cert, &db_cert).await?; diff --git a/openapi/nexus/nexus-2026101500.0.0-7cfc90.json b/openapi/nexus/nexus-2026101500.0.0-cda9b6.json similarity index 99% rename from openapi/nexus/nexus-2026101500.0.0-7cfc90.json rename to openapi/nexus/nexus-2026101500.0.0-cda9b6.json index d826ee9f5c1..5dcfbe7b9ac 100644 --- a/openapi/nexus/nexus-2026101500.0.0-7cfc90.json +++ b/openapi/nexus/nexus-2026101500.0.0-cda9b6.json @@ -1971,8 +1971,7 @@ "tags": [ "silos" ], - "summary": "List TLS certificates for the current silo", - "description": "Returns a list of TLS certificates for the current silo, sorted by creation date with the most recent certificates appearing first.", + "summary": "List TLS certificates in current silo", "operationId": "certificate_list", "parameters": [ { @@ -2029,7 +2028,7 @@ "tags": [ "silos" ], - "summary": "Create TLS certificate for the current silo", + "summary": "Create TLS certificate in current silo", "description": "This certificate is automatically used by the Oxide control plane to serve API requests.", "operationId": "certificate_create", "requestBody": { @@ -2067,8 +2066,7 @@ "tags": [ "silos" ], - "summary": "Fetch TLS certificate within the current silo", - "description": "Returns the details of a specific certificate.", + "summary": "Fetch TLS certificate in current silo", "operationId": "certificate_view", "parameters": [ { @@ -2104,8 +2102,7 @@ "tags": [ "silos" ], - "summary": "Delete TLS certificate within the current silo", - "description": "Permanently delete a certificate. This operation cannot be undone.", + "summary": "Delete TLS certificate in current silo", "operationId": "certificate_delete", "parameters": [ { @@ -7422,9 +7419,8 @@ "tags": [ "system/silos" ], - "summary": "List TLS certificates for the specified silo", - "description": "Returns a list of TLS certificates for the specified silo, sorted by creation date with the most recent certificates appearing first.", - "operationId": "system_certificate_list", + "summary": "List TLS certificates in silo", + "operationId": "silo_certificate_list", "parameters": [ { "in": "query", @@ -7490,9 +7486,9 @@ "tags": [ "system/silos" ], - "summary": "Create TLS certificate for the specified silo", + "summary": "Create TLS certificate in silo", "description": "This certificate is automatically used by the Oxide control plane to serve API requests.", - "operationId": "system_certificate_create", + "operationId": "silo_certificate_create", "parameters": [ { "in": "query", @@ -7539,9 +7535,9 @@ "tags": [ "system/silos" ], - "summary": "Fetch TLS certificate for the specified silo", - "description": "Returns the details of a specific certificate.", - "operationId": "system_certificate_view", + "summary": "Fetch TLS certificate in silo", + "description": "The silo must be specified if the certificate is specified by name, and must not be specified if the certificate is specified by ID.", + "operationId": "silo_certificate_view", "parameters": [ { "in": "path", @@ -7584,9 +7580,9 @@ "tags": [ "system/silos" ], - "summary": "Delete TLS certificate for the specified silo", - "description": "Permanently delete a certificate. This operation cannot be undone.\n\nThe silo must be specified if the certificate is specified by name, and must not be specified if the certificate is specified by ID.", - "operationId": "system_certificate_delete", + "summary": "Delete TLS certificate in silo", + "description": "The silo must be specified if the certificate is specified by name, and must not be specified if the certificate is specified by ID.", + "operationId": "silo_certificate_delete", "parameters": [ { "in": "path", diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index c7075d64c04..5195e9ffa59 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026101500.0.0-7cfc90.json \ No newline at end of file +nexus-2026101500.0.0-cda9b6.json \ No newline at end of file From 5cda99a72c6bccdc42d668791fa7b0b70b8733d2 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 10:34:13 -0500 Subject: [PATCH 6/9] Fix stale certificate create/delete comments --- nexus/src/app/certificate.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/nexus/src/app/certificate.rs b/nexus/src/app/certificate.rs index ab0a4da691f..f211f3db5c5 100644 --- a/nexus/src/app/certificate.rs +++ b/nexus/src/app/certificate.rs @@ -125,19 +125,16 @@ impl super::Nexus { authz::SiloCertificateList::new(authz_silo.clone()); opctx.authorize(authz::Action::CreateChild, &authz_cert_list).await?; - // The `opctx` we received is going to be checked for permission to - // create a cert below in `db_datastore.certificate_create`, but first - // we need to look up this silo's fully-qualified domain names in order - // to check that the cert we've been given is valid for this silo. - // Looking up DNS names requires reading the DNS configuration of the - // _rack_, which this user may not be able to do (even if they have - // permission to upload new certs, which almost certainly implies a - // silo-level admin). We'll use our `opctx_external_authn()` context, - // which is the same context used to create a silo. This is a higher - // privilege than the current user may have, but we believe it does not - // leak any information that a silo admin doesn't already know (the - // external DNS name(s) of the rack, which leads to their silo's DNS - // name(s)). + // To check that the cert we've been given is valid for this silo, we + // need to look up this silo's fully-qualified domain names. Looking up + // DNS names requires reading the DNS configuration of the _rack_, + // which this user may not be able to do (even if they have permission + // to upload new certs, which almost certainly implies a silo-level + // admin). We'll use our `opctx_external_authn()` context, which is the + // same context used to create a silo. This is a higher privilege than + // the current user may have, but we believe it does not leak any + // information that a silo admin doesn't already know (the external DNS + // name(s) of the rack, which leads to their silo's DNS name(s)). let silo_fq_dns_names = self .silo_fq_dns_names(self.opctx_external_authn(), authz_silo.id()) .await?; @@ -203,7 +200,7 @@ impl super::Nexus { self.db_datastore.certificate_delete(opctx, authz_cert).await?; match db_cert.service { ServiceKind::Nexus => { - // See the comment in certificate_create() above. + // See the comment in certificate_create_for_silo() above. self.background_tasks .activate(&self.background_tasks.task_external_endpoints); } From 54131c015d29a3a5f097bcde666a7a0c0ac4e76b Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 13:43:52 -0500 Subject: [PATCH 7/9] Move certificate delete lookups into app layer --- nexus/src/app/certificate.rs | 25 ++++++++++++++++++++++ nexus/src/external_api/http_entrypoints.rs | 20 ++++++----------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/nexus/src/app/certificate.rs b/nexus/src/app/certificate.rs index f211f3db5c5..ce7aa884450 100644 --- a/nexus/src/app/certificate.rs +++ b/nexus/src/app/certificate.rs @@ -192,6 +192,31 @@ impl super::Nexus { } pub(crate) async fn certificate_delete( + &self, + opctx: &OpContext, + certificate: &NameOrId, + ) -> DeleteResult { + let (authz_cert, db_cert) = self + .current_silo_certificate_fetch_for( + opctx, + certificate, + authz::Action::Delete, + ) + .await?; + self.certificate_delete_inner(opctx, &authz_cert, &db_cert).await + } + + pub(crate) async fn silo_certificate_delete( + &self, + opctx: &OpContext, + certificate_lookup: &lookup::Certificate<'_>, + ) -> DeleteResult { + let (.., authz_cert, db_cert) = + certificate_lookup.fetch_for(authz::Action::Delete).await?; + self.certificate_delete_inner(opctx, &authz_cert, &db_cert).await + } + + async fn certificate_delete_inner( &self, opctx: &OpContext, authz_cert: &authz::Certificate, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 40f12a2816d..d7ba6b6643b 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -4000,14 +4000,7 @@ impl NexusExternalApi for NexusExternalApiImpl { ) -> Result { audit_and_time(&rqctx, |opctx, nexus| async move { let path = path_params.into_inner(); - let (authz_cert, db_cert) = nexus - .current_silo_certificate_fetch_for( - &opctx, - &path.certificate, - authz::Action::Delete, - ) - .await?; - nexus.certificate_delete(&opctx, &authz_cert, &db_cert).await?; + nexus.certificate_delete(&opctx, &path.certificate).await?; Ok(HttpResponseDeleted()) }) .await @@ -4097,11 +4090,12 @@ impl NexusExternalApi for NexusExternalApiImpl { audit_and_time(&rqctx, |opctx, nexus| async move { let path = path_params.into_inner(); let query = query_params.into_inner(); - let (.., authz_cert, db_cert) = nexus - .silo_certificate_lookup(&opctx, query.silo, path.certificate)? - .fetch_for(authz::Action::Delete) - .await?; - nexus.certificate_delete(&opctx, &authz_cert, &db_cert).await?; + let certificate_lookup = nexus.silo_certificate_lookup( + &opctx, + query.silo, + path.certificate, + )?; + nexus.silo_certificate_delete(&opctx, &certificate_lookup).await?; Ok(HttpResponseDeleted()) }) .await From 1cc513e0f420c9d855b17b7adb7f334be4139483 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 13:46:51 -0500 Subject: [PATCH 8/9] Test upload with a target-only certificate --- nexus/tests/integration_tests/certificates.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/nexus/tests/integration_tests/certificates.rs b/nexus/tests/integration_tests/certificates.rs index c1dddb38033..48f3bcd7cb5 100644 --- a/nexus/tests/integration_tests/certificates.rs +++ b/nexus/tests/integration_tests/certificates.rs @@ -368,8 +368,11 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { error.message ); - // Create a certificate in the other silo. - let chain = CertificateChain::new(cptestctx.wildcard_silo_dns_name()); + // Create a certificate valid only for the other silo. + let chain = CertificateChain::new(format!( + "{other_silo_name}.sys.{}", + cptestctx.external_dns_zone_name + )); let params = CertificateCreate { cert: chain.cert_chain_as_pem(), key: chain.end_cert_private_key_as_pem(), From ea3163c516d220b624c550d1bdaed98dfff758fa Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 15 Oct 2026 14:16:09 -0500 Subject: [PATCH 9/9] save some lines in the tests with nicer expect_status helpers --- nexus/tests/integration_tests/certificates.rs | 143 ++++++------------ 1 file changed, 43 insertions(+), 100 deletions(-) diff --git a/nexus/tests/integration_tests/certificates.rs b/nexus/tests/integration_tests/certificates.rs index 48f3bcd7cb5..9ea39f86d8a 100644 --- a/nexus/tests/integration_tests/certificates.rs +++ b/nexus/tests/integration_tests/certificates.rs @@ -16,7 +16,6 @@ use nexus_db_lookup::LookupPath; use nexus_db_queries::context::OpContext; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; -use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::resource_helpers::create_certificate; use nexus_test_utils::resource_helpers::create_local_user; use nexus_test_utils::resource_helpers::create_silo; @@ -482,18 +481,36 @@ async fn test_system_certificates_crud(cptestctx: &ControlPlaneTestContext) { /// Make a request as `authn` and check that it fails with `status` async fn expect_status( client: &ClientTestContext, - authn: AuthnMode, + authn: &AuthnMode, method: Method, url: &str, - body: Option<&CertificateCreate>, - status: StatusCode, + status: u16, ) { - NexusRequest::new( - RequestBuilder::new(client, method, url) - .body(body) - .expect_status(Some(status)), + let status = StatusCode::from_u16(status).unwrap(); + NexusRequest::expect_failure(client, status, method, url) + .authn_as(authn.clone()) + .execute() + .await + .unwrap(); +} + +/// Make a POST as `authn` and check that it fails with `status` +async fn expect_post_status( + client: &ClientTestContext, + authn: &AuthnMode, + url: &str, + body: &CertificateCreate, + status: u16, +) { + let status = StatusCode::from_u16(status).unwrap(); + NexusRequest::expect_failure_with_body( + client, + status, + Method::POST, + url, + body, ) - .authn_as(authn) + .authn_as(authn.clone()) .execute() .await .unwrap(); @@ -587,15 +604,7 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { } // But not in another silo, whether by silo, by name, or by ID. - expect_status( - client, - admin.clone(), - Method::POST, - &other_url, - Some(¶ms), - StatusCode::NOT_FOUND, - ) - .await; + expect_post_status(client, &admin, &other_url, ¶ms, 404).await; let other_by_name_url = format!("/v1/system/certificates/{CERT_NAME}?silo={other_silo_name}"); let other_by_id_url = @@ -607,15 +616,7 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { (Method::GET, &other_by_id_url), (Method::DELETE, &other_by_id_url), ] { - expect_status( - client, - admin.clone(), - method, - url, - None, - StatusCode::NOT_FOUND, - ) - .await; + expect_status(client, &admin, method, url, 404).await; } // A fleet viewer can't create certificates. This must fail on @@ -627,39 +628,13 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { key: bad_chain.end_cert_private_key_as_pem(), ..params.clone() }; - for params in [¶ms, &bad_params] { - expect_status( - client, - viewer.clone(), - Method::POST, - &other_url, - Some(params), - StatusCode::FORBIDDEN, - ) - .await; - } + expect_post_status(client, &viewer, &other_url, ¶ms, 403).await; + expect_post_status(client, &viewer, &other_url, &bad_params, 403).await; // A fleet viewer can't list, view, or delete certificates either. - expect_status( - client, - viewer.clone(), - Method::GET, - &other_url, - None, - StatusCode::FORBIDDEN, - ) - .await; - for method in [Method::GET, Method::DELETE] { - expect_status( - client, - viewer.clone(), - method, - &other_by_id_url, - None, - StatusCode::NOT_FOUND, - ) - .await; - } + expect_status(client, &viewer, Method::GET, &other_url, 403).await; + expect_status(client, &viewer, Method::GET, &other_by_id_url, 404).await; + expect_status(client, &viewer, Method::DELETE, &other_by_id_url, 404).await; // A silo collaborator can read their own silo, so these get past the silo // lookup, but they still can't list, create, view, or delete certificates. @@ -680,50 +655,18 @@ async fn test_system_certificates_authz(cptestctx: &ControlPlaneTestContext) { ) .await; let collaborator = AuthnMode::SiloUser(collaborator_id); - expect_status( - client, - collaborator.clone(), - Method::GET, - &url, - None, - StatusCode::FORBIDDEN, - ) - .await; - expect_status( - client, - collaborator.clone(), - Method::POST, - &url, - Some(¶ms), - StatusCode::FORBIDDEN, - ) - .await; - for url in [&by_name_url, &by_id_url] { - for method in [Method::GET, Method::DELETE] { - expect_status( - client, - collaborator.clone(), - method, - url, - None, - StatusCode::NOT_FOUND, - ) - .await; - } - } + expect_status(client, &collaborator, Method::GET, &url, 403).await; + expect_post_status(client, &collaborator, &url, ¶ms, 403).await; + expect_status(client, &collaborator, Method::GET, &by_name_url, 404).await; + expect_status(client, &collaborator, Method::DELETE, &by_name_url, 404) + .await; + expect_status(client, &collaborator, Method::GET, &by_id_url, 404).await; + expect_status(client, &collaborator, Method::DELETE, &by_id_url, 404).await; // Certificates in the silo can't be accessed by other silos' users. - for method in [Method::GET, Method::DELETE] { - expect_status( - client, - AuthnMode::UnprivilegedUser, - method, - &by_id_url, - None, - StatusCode::NOT_FOUND, - ) - .await; - } + let unprivileged = AuthnMode::UnprivilegedUser; + expect_status(client, &unprivileged, Method::GET, &by_id_url, 404).await; + expect_status(client, &unprivileged, Method::DELETE, &by_id_url, 404).await; NexusRequest::object_delete(client, &by_id_url) .authn_as(admin) .execute()