From 5d25c73c39e73f5e9fb01c0ee803f3e8081f32fa Mon Sep 17 00:00:00 2001 From: Ryan Cammer Date: Thu, 26 Jan 2023 19:51:32 +0000 Subject: [PATCH] Resolve BOUN-557 "Increase cert issuance test cov" --- .gitignore | 3 + rs/Cargo.lock | 71 +++--- .../certificate_issuer/BUILD.bazel | 10 +- .../certificate_issuer/Cargo.toml | 2 + .../certificate_issuer/src/acme.rs | 4 + .../certificate_issuer/src/certificate.rs | 4 +- .../certificate_issuer/src/check.rs | 5 +- .../certificate_issuer/src/dns.rs | 4 + .../certificate_issuer/src/work.rs | 217 ++++++++++++++++++ .../certificate_orchestrator/BUILD.bazel | 8 + .../certificate_orchestrator/Cargo.toml | 1 + .../certificate_orchestrator/src/acl.rs | 52 +++++ .../certificate_orchestrator/src/id.rs | 39 +++- .../certificate_orchestrator/src/lib.rs | 10 +- .../src/registration.rs | 131 ++++++++++- .../certificate_orchestrator/src/work.rs | 61 ++++- .../src/lib.rs | 6 +- .../certificate_syncer/BUILD.bazel | 11 +- .../certificate_syncer/Cargo.toml | 2 + .../certificate_syncer/src/http.rs | 2 + .../certificate_syncer/src/import.rs | 59 ++++- .../certificate_syncer/src/persist.rs | 71 ++++++ .../certificate_syncer/src/render.rs | 22 +- 23 files changed, 742 insertions(+), 53 deletions(-) diff --git a/.gitignore b/.gitignore index a347c99de55..b675c615a56 100644 --- a/.gitignore +++ b/.gitignore @@ -127,3 +127,6 @@ user.bazelrc # Intellij .idea .clwb + +# OS X +.DS_Store diff --git a/rs/Cargo.lock b/rs/Cargo.lock index 53bdaa2c1c8..caca04344cb 100644 --- a/rs/Cargo.lock +++ b/rs/Cargo.lock @@ -623,7 +623,7 @@ dependencies = [ "cfg-if 1.0.0", "libc", "miniz_oxide", - "object 0.30.1", + "object 0.30.2", "rustc-demangle", ] @@ -1431,6 +1431,7 @@ dependencies = [ "axum", "candid", "certificate_orchestrator_interface", + "cfg-if 1.0.0", "chacha20poly1305", "chrono", "clap 4.0.32", @@ -1442,9 +1443,10 @@ dependencies = [ "ic-agent", "ic-utils 0.22.0", "instant-acme", + "mockall 0.11.3", "opentelemetry 0.18.0", "opentelemetry-prometheus 0.11.0", - "pem 1.1.0", + "pem 1.1.1", "prometheus 0.13.3", "rcgen", "reqwest", @@ -1472,12 +1474,14 @@ dependencies = [ "futures", "hyper", "hyper-rustls", + "mockall 0.11.3", "nix 0.26.1", "opentelemetry 0.18.0", "opentelemetry-prometheus 0.11.0", "prometheus 0.13.3", "serde", "serde_json", + "tempfile", "thiserror", "tokio", "tracing", @@ -1493,6 +1497,7 @@ dependencies = [ "bincode", "candid", "certificate_orchestrator_interface", + "cfg-if 1.0.0", "hex", "ic-cdk", "ic-cdk-macros", @@ -2449,7 +2454,7 @@ dependencies = [ "hashbrown 0.12.3", "lock_api", "once_cell", - "parking_lot_core 0.9.5", + "parking_lot_core 0.9.6", ] [[package]] @@ -4101,7 +4106,7 @@ dependencies = [ "k256", "leb128", "mime", - "pem 1.1.0", + "pem 1.1.1", "pkcs8 0.9.0", "rand 0.8.5", "reqwest", @@ -5013,7 +5018,7 @@ dependencies = [ "hex", "k256", "lazy_static", - "pem 1.1.0", + "pem 1.1.1", "rand 0.8.5", "simple_asn1 0.6.2", "wycheproof", @@ -11157,9 +11162,9 @@ dependencies = [ [[package]] name = "object" -version = "0.30.1" +version = "0.30.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8d864c91689fdc196779b98dba0aceac6118594c2df6ee5d943eb6a8df4d107a" +checksum = "2b8c786513eb403643f2a88c244c2aaa270ef2153f55094587d0c48a3cf22a83" dependencies = [ "memchr", ] @@ -11479,7 +11484,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3742b2c103b9f06bc9fff0a37ff4912935851bee6d36f3c02bcc755bcfec228f" dependencies = [ "lock_api", - "parking_lot_core 0.9.5", + "parking_lot_core 0.9.6", ] [[package]] @@ -11498,9 +11503,9 @@ dependencies = [ [[package]] name = "parking_lot_core" -version = "0.9.5" +version = "0.9.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7ff9f3fef3968a3ec5945535ed654cb38ff72d7495a25619e2247fb15a2ed9ba" +checksum = "ba1ef8814b5c993410bb3adfad7a5ed269563e4a2f90c41f5d85be7fb47133bf" dependencies = [ "cfg-if 1.0.0", "libc", @@ -11577,9 +11582,9 @@ dependencies = [ [[package]] name = "pem" -version = "1.1.0" +version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03c64931a1a212348ec4f3b4362585eca7159d0d09cbdf4a7f74f02173596fd4" +checksum = "a8835c273a76a90455d7344889b0964598e3316e2a79ede8e36f16bdcf2228b8" dependencies = [ "base64 0.13.1", ] @@ -11616,9 +11621,9 @@ checksum = "478c572c3d73181ff3c2539045f6eb99e5491218eae919370993b890cdbdd98e" [[package]] name = "pest" -version = "2.5.2" +version = "2.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f6e86fb9e7026527a0d46bc308b841d73170ef8f443e1807f6ef88526a816d4" +checksum = "4257b4a04d91f7e9e6290be5d3da4804dd5784fafde3a497d73eb2b4a158c30a" dependencies = [ "thiserror", "ucd-trie", @@ -11626,9 +11631,9 @@ dependencies = [ [[package]] name = "pest_derive" -version = "2.5.2" +version = "2.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96504449aa860c8dcde14f9fba5c58dc6658688ca1fe363589d6327b8662c603" +checksum = "241cda393b0cdd65e62e07e12454f1f25d57017dcc514b1514cd3c4645e3a0a6" dependencies = [ "pest", "pest_generator", @@ -11636,9 +11641,9 @@ dependencies = [ [[package]] name = "pest_generator" -version = "2.5.2" +version = "2.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "798e0220d1111ae63d66cb66a5dcb3fc2d986d520b98e49e1852bfdb11d7c5e7" +checksum = "46b53634d8c8196302953c74d5352f33d0c512a9499bd2ce468fc9f4128fa27c" dependencies = [ "pest", "pest_meta", @@ -11649,20 +11654,20 @@ dependencies = [ [[package]] name = "pest_meta" -version = "2.5.2" +version = "2.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "984298b75898e30a843e278a9f2452c31e349a073a0ce6fd950a12a74464e065" +checksum = "0ef4f1332a8d4678b41966bb4cc1d0676880e84183a1ecc3f4b69f03e99c7a51" dependencies = [ "once_cell", "pest", - "sha1", + "sha2 0.10.6", ] [[package]] name = "pest_vm" -version = "2.5.2" +version = "2.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec7dfa14fc981e3de5bfab285c6cf36f8332377707b19aaefc8912538db3ce8c" +checksum = "249f7a7c5c1c61668bb9f12d7193ea873180acc2f2844dc2c21468e074bc5c7e" dependencies = [ "pest", "pest_meta", @@ -12148,9 +12153,9 @@ dependencies = [ [[package]] name = "prost" -version = "0.11.5" +version = "0.11.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c01db6702aa05baa3f57dec92b8eeeeb4cb19e894e73996b32a4093289e54592" +checksum = "21dc42e00223fc37204bd4aa177e69420c604ca4a183209a8f9de30c6d934698" dependencies = [ "bytes", "prost-derive", @@ -12158,9 +12163,9 @@ dependencies = [ [[package]] name = "prost-build" -version = "0.11.5" +version = "0.11.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb5320c680de74ba083512704acb90fe00f28f79207286a848e730c45dd73ed6" +checksum = "a3f8ad728fb08fe212df3c05169e940fbb6d9d16a877ddde14644a983ba2012e" dependencies = [ "bytes", "heck 0.4.0", @@ -12180,9 +12185,9 @@ dependencies = [ [[package]] name = "prost-derive" -version = "0.11.5" +version = "0.11.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8842bad1a5419bca14eac663ba798f6bc19c413c2fdceb5f3ba3b0932d96720" +checksum = "8bda8c0881ea9f722eb9629376db3d0b903b462477c1aafcb0566610ac28ac5d" dependencies = [ "anyhow", "itertools", @@ -12193,9 +12198,9 @@ dependencies = [ [[package]] name = "prost-types" -version = "0.11.5" +version = "0.11.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "017f79637768cde62820bc2d4fe0e45daaa027755c323ad077767c6c5f173091" +checksum = "a5e0526209433e96d83d750dd81a99118edbc55739e7e61a46764fd2ad537788" dependencies = [ "bytes", "prost", @@ -12548,7 +12553,7 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ffbe84efe2f38dea12e9bfc1f65377fdf03e53a18cb3b995faedf7934c7e785b" dependencies = [ - "pem 1.1.0", + "pem 1.1.1", "ring", "time 0.3.17", "yasna", @@ -14410,7 +14415,7 @@ dependencies = [ "on_wire", "openssh-keys", "openssl", - "pem 1.1.0", + "pem 1.1.1", "phantom_newtype", "proptest", "prost", diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/BUILD.bazel b/rs/boundary_node/certificate_issuance/certificate_issuer/BUILD.bazel index 5255ba3ee1a..a9ca129072d 100644 --- a/rs/boundary_node/certificate_issuance/certificate_issuer/BUILD.bazel +++ b/rs/boundary_node/certificate_issuance/certificate_issuer/BUILD.bazel @@ -1,4 +1,4 @@ -load("@rules_rust//rust:defs.bzl", "rust_binary") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_test") package(default_visibility = ["//visibility:public"]) @@ -18,6 +18,7 @@ DEPENDENCIES = [ "@crate_index//:ic-agent", "@crate_index//:ic-utils", "@crate_index//:instant-acme", + "@crate_index//:mockall", "@crate_index//:opentelemetry_0_18_0", "@crate_index//:opentelemetry_prometheus_0_11_0", "@crate_index//:pem", @@ -46,3 +47,10 @@ rust_binary( version = "0.1.0", deps = DEPENDENCIES, ) + +rust_test( + name = "certificate_issuer_test", + crate = ":certificate-issuer", + proc_macro_deps = MACRO_DEPENDENCIES, + deps = DEPENDENCIES, +) diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/Cargo.toml b/rs/boundary_node/certificate_issuance/certificate_issuer/Cargo.toml index 4048cf3f429..85465267509 100644 --- a/rs/boundary_node/certificate_issuance/certificate_issuer/Cargo.toml +++ b/rs/boundary_node/certificate_issuance/certificate_issuer/Cargo.toml @@ -11,6 +11,7 @@ anyhow = "1.0.66" async-trait = "0.1.58" axum = { version = "0.6.1", features = ["json"] } candid = "0.8.4" +cfg-if = "1.0.0" chacha20poly1305 = "0.10.0" chrono = { version = "0.4.19", default-features = false, features = ["clock"] } clap = { version = "4.0.18", features = ["derive"] } @@ -22,6 +23,7 @@ hyper-rustls = "0.23.0" ic-agent = "0.22.0" ic-utils = { version = "0.22.0", features = ["raw"] } instant-acme = "0.1.0" +mockall = "0.11.3" opentelemetry = "0.18.0" opentelemetry-prometheus = "0.11.0" pem = "1.1.0" diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/acme.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/acme.rs index 461c0d221f1..1fb1b933698 100644 --- a/rs/boundary_node/certificate_issuance/certificate_issuer/src/acme.rs +++ b/rs/boundary_node/certificate_issuance/certificate_issuer/src/acme.rs @@ -3,18 +3,22 @@ use async_trait::async_trait; use instant_acme::{ Account, Authorization, Challenge, ChallengeType, Identifier, NewOrder, OrderStatus, }; +use mockall::automock; use rcgen::{Certificate, CertificateParams, DistinguishedName}; +#[automock] #[async_trait] pub trait Order: Sync + Send { async fn order(&self, name: &str) -> Result; } +#[automock] #[async_trait] pub trait Ready: Sync + Send { async fn ready(&self, name: &str) -> Result<(), Error>; } +#[automock] #[async_trait] pub trait Finalize: Sync + Send { async fn finalize(&self, name: &str) -> Result<(String, String), Error>; diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/certificate.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/certificate.rs index 475d761bfec..d4fea392ec8 100644 --- a/rs/boundary_node/certificate_issuance/certificate_issuer/src/certificate.rs +++ b/rs/boundary_node/certificate_issuance/certificate_issuer/src/certificate.rs @@ -8,11 +8,12 @@ use futures::{stream, StreamExt, TryStreamExt}; use garcon::Delay; use ic_agent::Agent; use ifc::EncryptedPair; +use mockall::automock; use serde::Serialize; use crate::encode::{Decode, Encode}; -#[derive(Serialize)] +#[derive(Debug, PartialEq, Serialize)] pub struct Pair( pub Vec, // Private Key pub Vec, // Certificate Chain @@ -26,6 +27,7 @@ pub enum UploadError { UnexpectedError(#[from] anyhow::Error), } +#[automock] #[async_trait] pub trait Upload: Sync + Send { async fn upload(&self, id: &str, pair: Pair) -> Result<(), UploadError>; diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/check.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/check.rs index 90797657e32..2fef086f3e9 100644 --- a/rs/boundary_node/certificate_issuance/certificate_issuer/src/check.rs +++ b/rs/boundary_node/certificate_issuance/certificate_issuer/src/check.rs @@ -2,13 +2,12 @@ use anyhow::anyhow; use async_trait::async_trait; use candid::Principal; use ic_agent::Agent; -use std::sync::Arc; -use trust_dns_resolver::{error::ResolveErrorKind, proto::rr::RecordType}; - use ic_utils::{ call::SyncCall, interfaces::http_request::{HttpRequestCanister, HttpResponse}, }; +use std::sync::Arc; +use trust_dns_resolver::{error::ResolveErrorKind, proto::rr::RecordType}; use crate::dns::Resolve; diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/dns.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/dns.rs index fb501ae340e..94910b48db1 100644 --- a/rs/boundary_node/certificate_issuance/certificate_issuer/src/dns.rs +++ b/rs/boundary_node/certificate_issuance/certificate_issuer/src/dns.rs @@ -1,9 +1,11 @@ use anyhow::Error; use async_trait::async_trait; +use mockall::automock; use trust_dns_resolver::{ error::ResolveError, lookup::Lookup, proto::rr::RecordType, TokioAsyncResolver, }; +#[automock] #[async_trait] pub trait Resolve: Sync + Send { async fn lookup(&self, name: &str, record_type: RecordType) -> Result; @@ -24,11 +26,13 @@ pub enum Record { Txt(String), } +#[automock] #[async_trait] pub trait Create: Sync + Send { async fn create(&self, zone: &str, name: &str, record: Record) -> Result<(), Error>; } +#[automock] #[async_trait] pub trait Delete: Sync + Send { async fn delete(&self, zone: &str, name: &str) -> Result<(), Error>; diff --git a/rs/boundary_node/certificate_issuance/certificate_issuer/src/work.rs b/rs/boundary_node/certificate_issuance/certificate_issuer/src/work.rs index 496d6565ca2..bdceda6aea4 100644 --- a/rs/boundary_node/certificate_issuance/certificate_issuer/src/work.rs +++ b/rs/boundary_node/certificate_issuance/certificate_issuer/src/work.rs @@ -349,3 +349,220 @@ impl Process for Processor { } } } + +#[cfg(test)] +mod tests { + use super::*; + + use anyhow::Error; + use mockall::predicate; + use trust_dns_resolver::{ + lookup::Lookup, + proto::{op::Query, rr::Record as TrustRecord}, + Name, + }; + + use crate::{ + acme::{MockFinalize, MockOrder, MockReady}, + certificate::MockUpload, + dns::{MockCreate, MockDelete, MockResolve, Record}, + }; + + #[tokio::test] + async fn test_process_order() -> Result<(), Error> { + let id: String = "id".into(); + + let task = Task { + name: "name".into(), + action: Action::Order, + }; + + let mut resolver = MockResolve::new(); + resolver.expect_lookup().never(); + + let mut acme_order = MockOrder::new(); + acme_order + .expect_order() + .times(1) + .with(predicate::eq("name")) + .returning(|_| Ok("token".into())); + + let mut acme_ready = MockReady::new(); + acme_ready.expect_ready().never(); + + let mut acme_finalize = MockFinalize::new(); + acme_finalize.expect_finalize().never(); + + let mut dns_creator = MockCreate::new(); + dns_creator + .expect_create() + .times(1) + .with( + predicate::eq("delegation"), + predicate::eq("_acme-challenge.name"), + predicate::eq(Record::Txt("token".into())), + ) + .returning(|_, _, _| Ok(())); + + let mut dns_deleter = MockDelete::new(); + dns_deleter.expect_delete().never(); + + let mut certificate_uploader = MockUpload::new(); + certificate_uploader.expect_upload().never(); + + let processor = Processor::new( + "delegation".into(), // delegation_domain + Box::new(resolver), // resolver + Box::new(acme_order), // acme_order + Box::new(acme_ready), // acme_ready + Box::new(acme_finalize), // acme_finalize + Box::new(dns_creator), // dns_creator + Box::new(dns_deleter), // dns_deleter + Box::new(certificate_uploader), // certificate_uploader + ); + + match processor.process(&id, &task).await { + Err(ProcessError::AwaitingDnsPropogation) => Ok(()), + other => Err(anyhow!( + "expected AwaitingDnsPropogation but got {:?}", + other + )), + } + } + + #[tokio::test] + async fn test_process_ready() -> Result<(), Error> { + let id: String = "id".into(); + + let task = Task { + name: "name".into(), + action: Action::Ready, + }; + + let mut resolver = MockResolve::new(); + resolver + .expect_lookup() + .times(1) + .with( + predicate::eq("_acme-challenge.name.delegation"), + predicate::eq(RecordType::TXT), + ) + .returning(|_, _| { + Ok({ + let q = Query::new(); + + let mut r = TrustRecord::new(); + r.set_name(Name::from_utf8("token")?); + r.set_record_type(RecordType::TXT); + + Lookup::new_with_max_ttl(q, Arc::new([r])) + }) + }); + + let mut acme_order = MockOrder::new(); + acme_order.expect_order().never(); + + let mut acme_ready = MockReady::new(); + acme_ready + .expect_ready() + .times(1) + .with(predicate::eq("name")) + .returning(|_| Ok(())); + + let mut acme_finalize = MockFinalize::new(); + acme_finalize.expect_finalize().never(); + + let mut dns_creator = MockCreate::new(); + dns_creator.expect_create().never(); + + let mut dns_deleter = MockDelete::new(); + dns_deleter.expect_delete().never(); + + let mut certificate_uploader = MockUpload::new(); + certificate_uploader.expect_upload().never(); + + let processor = Processor::new( + "delegation".into(), // delegation_domain + Box::new(resolver), // resolver + Box::new(acme_order), // acme_order + Box::new(acme_ready), // acme_ready + Box::new(acme_finalize), // acme_finalize + Box::new(dns_creator), // dns_creator + Box::new(dns_deleter), // dns_deleter + Box::new(certificate_uploader), // certificate_uploader + ); + + match processor.process(&id, &task).await { + Err(ProcessError::AwaitingAcmeOrderReady) => Ok(()), + other => Err(anyhow!( + "expected AwaitingAcmeOrderReady but got {:?}", + other + )), + } + } + + #[tokio::test] + async fn test_process_certificate() -> Result<(), Error> { + let id: String = "id".into(); + + let task = Task { + name: "name".into(), + action: Action::Certificate, + }; + + let mut resolver = MockResolve::new(); + resolver.expect_lookup().never(); + + let mut acme_order = MockOrder::new(); + acme_order.expect_order().never(); + + let mut acme_ready = MockReady::new(); + acme_ready.expect_ready().never(); + + let mut acme_finalize = MockFinalize::new(); + acme_finalize + .expect_finalize() + .times(1) + .with(predicate::eq("name")) + .returning(|_| Ok(("cert".into(), "key".into()))); + + let mut dns_creator = MockCreate::new(); + dns_creator.expect_create().never(); + + let mut dns_deleter = MockDelete::new(); + dns_deleter + .expect_delete() + .times(1) + .with( + predicate::eq("delegation"), + predicate::eq("_acme-challenge.name"), + ) + .returning(|_, _| Ok(())); + + let mut certificate_uploader = MockUpload::new(); + certificate_uploader + .expect_upload() + .times(1) + .with( + predicate::eq("id"), + predicate::eq(Pair("key".into(), "cert".into())), + ) + .returning(|_, _| Ok(())); + + let processor = Processor::new( + "delegation".into(), // delegation_domain + Box::new(resolver), // resolver + Box::new(acme_order), // acme_order + Box::new(acme_ready), // acme_ready + Box::new(acme_finalize), // acme_finalize + Box::new(dns_creator), // dns_creator + Box::new(dns_deleter), // dns_deleter + Box::new(certificate_uploader), // certificate_uploader + ); + + match processor.process(&id, &task).await { + Ok(()) => Ok(()), + other => Err(anyhow!("expected Ok(()) but got {:?}", other)), + } + } +} diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator/BUILD.bazel b/rs/boundary_node/certificate_issuance/certificate_orchestrator/BUILD.bazel index ef996c1ab39..9e35ae2a2f2 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator/BUILD.bazel +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator/BUILD.bazel @@ -1,4 +1,5 @@ load("//bazel:canisters.bzl", "rust_canister") +load("@rules_rust//rust:defs.bzl", "rust_test") package(default_visibility = ["//visibility:public"]) @@ -32,3 +33,10 @@ rust_canister( service_file = ":interface.did", deps = DEPENDENCIES, ) + +rust_test( + name = "certificate_orchestrator_test", + crate = ":_wasm_certificate_orchestrator", + proc_macro_deps = MACRO_DEPENDENCIES, + deps = DEPENDENCIES, +) diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator/Cargo.toml b/rs/boundary_node/certificate_issuance/certificate_orchestrator/Cargo.toml index ab82d20bf7a..1ac39171e08 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator/Cargo.toml +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator/Cargo.toml @@ -14,6 +14,7 @@ anyhow = "1.0.66" async-trait = "0.1.58" bincode = "1.3.3" candid = "0.8.4" +cfg-if = "1.0.0" hex = "0.4.3" ic-cdk = { version = "0.6.8", features = ["timers"] } ic-cdk-macros = "0.6.8" diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/acl.rs b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/acl.rs index 25b16d672f6..9bc10f92ec3 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/acl.rs +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/acl.rs @@ -52,3 +52,55 @@ impl Authorize for Box { } pub struct WithAuthorize(pub T, pub A); + +#[cfg(test)] +mod tests { + use super::*; + use crate::{acl::AuthorizeError, ROOT_PRINCIPALS}; + use candid::Principal; + + #[test] + fn authorizer_empty() { + let p = &Principal::from_text("llx5h-dqaaa-aaaag-abckq-cai").unwrap(); + + let result = Authorizer::new(&ROOT_PRINCIPALS).authorize(p); + + match result { + Err(AuthorizeError::Unauthorized) => {} + _ => panic!("expected unauthorized error, got {result:?}"), + } + } + + #[test] + fn authorizer_authorized() { + let p = &Principal::from_text("llx5h-dqaaa-aaaag-abckq-cai").unwrap(); + + ROOT_PRINCIPALS + .with(|m| m.borrow_mut().insert(p.to_text(), ())) + .unwrap(); + + let result = Authorizer::new(&ROOT_PRINCIPALS).authorize(p); + + match result { + Ok(()) => {} + _ => panic!("expected unauthorized error, got {result:?}"), + } + } + + #[test] + fn authorizer_unauthorized() { + let p1 = &Principal::from_text("llx5h-dqaaa-aaaag-abckq-cai").unwrap(); + let p2 = &Principal::from_text("bb7pg-paaaa-aaaap-aav3q-cai").unwrap(); + + ROOT_PRINCIPALS + .with(|m| m.borrow_mut().insert(p1.to_text(), ())) + .unwrap(); + + let result = Authorizer::new(&ROOT_PRINCIPALS).authorize(p2); + + match result { + Err(AuthorizeError::Unauthorized) => {} + _ => panic!("expected unauthorized error, got {result:?}"), + } + } +} diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/id.rs b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/id.rs index 686ee85c9a0..562967582e6 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/id.rs +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/id.rs @@ -1,7 +1,14 @@ use certificate_orchestrator_interface::Id; -use ic_cdk::api::time; use sha2::{Digest, Sha256}; +cfg_if::cfg_if! { + if #[cfg(test)] { + use tests::time; + } else { + use ic_cdk::api::time; + } +} + use crate::{LocalRef, StableValue}; pub trait Generate { @@ -34,3 +41,33 @@ impl Generate for Generator { hex::encode(id) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ID_COUNTER, ID_SEED}; + + pub fn time() -> u64 { + 0 + } + + #[test] + fn generate() { + // Initialize ID seed + ID_SEED.with(|s| s.borrow_mut().insert((), 0).expect("failed to insert")); + + let g = Generator::new(&ID_COUNTER, &ID_SEED); + + // Assumed to be sha256("000") + assert_eq!( + g.generate(), + "2ac9a6746aca543af8dff39894cfe8173afba21eb01c6fae33d52947222855ef" + ); + + // Assumed to be sha256("100") due to COUNTER_ID being incremented + assert_eq!( + g.generate(), + "ad57366865126e55649ecb23ae1d48887544976efea46a48eb5d85a6eeb4d306" + ); + } +} diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/lib.rs b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/lib.rs index da5e0eb94e9..a4c05599f13 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/lib.rs +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/lib.rs @@ -54,9 +54,9 @@ const ID_COUNTER_LEN: u32 = size_of::() as u32; const ID_SEED_LEN: u32 = size_of::() as u32; const REGISTRATION_ID_LEN: u32 = 64 * BYTE; const REGISTRATION_LEN: u32 = 128; -const ENCRYPED_PRIVATE_KEY_LEN: u32 = KB; // 1 * KB -const ENCRYPED_CERTIFICATE_LEN: u32 = 8 * KB; -const ENCRYPTED_PAIR_LEN: u32 = ENCRYPED_PRIVATE_KEY_LEN + ENCRYPED_CERTIFICATE_LEN; +const ENCRYPTED_PRIVATE_KEY_LEN: u32 = KB; // 1 * KB +const ENCRYPTED_CERTIFICATE_LEN: u32 = 8 * KB; +const ENCRYPTED_PAIR_LEN: u32 = ENCRYPTED_PRIVATE_KEY_LEN + ENCRYPTED_CERTIFICATE_LEN; const REGISTRATION_EXPIRATION_TTL: Duration = Duration::from_secs(6 * 3600); // 6 Hours const IN_PROGRESS_TTL: Duration = Duration::from_secs(10 * 60); // 10 Minutes @@ -73,7 +73,7 @@ const MEMORY_ID_ID_COUNTER: u8 = 2; const MEMORY_ID_ID_SEED: u8 = 3; const MEMORY_ID_REGISTRATIONS: u8 = 4; const MEMORY_ID_NAMES: u8 = 5; -const MEMORY_ID_ENCRPYTED_CERTIFICATES: u8 = 6; +const MEMORY_ID_ENCRYPTED_CERTIFICATES: u8 = 6; const MEMORY_ID_TASKS: u8 = 7; const MEMORY_ID_EXPIRATIONS: u8 = 8; const MEMORY_ID_RETRIES: u8 = 9; @@ -178,7 +178,7 @@ thread_local! { thread_local! { static ENCRYPTED_CERTIFICATES: RefCell> = RefCell::new( StableBTreeMap::init( - MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(MEMORY_ID_ENCRPYTED_CERTIFICATES))), + MEMORY_MANAGER.with(|m| m.borrow().get(MemoryId::new(MEMORY_ID_ENCRYPTED_CERTIFICATES))), REGISTRATION_ID_LEN, // MAX_KEY_SIZE, ENCRYPTED_PAIR_LEN, // MAX_VALUE_SIZE ) diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/registration.rs b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/registration.rs index 61b8b677a40..55a82779475 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/registration.rs +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/registration.rs @@ -3,10 +3,18 @@ use std::cmp::Reverse; use anyhow::anyhow; use candid::Principal; use certificate_orchestrator_interface::{Id, Name, NameError, Registration, State}; -use ic_cdk::{api::time, caller}; +use ic_cdk::caller; use ic_stable_structures::StableBTreeMap; use priority_queue::PriorityQueue; +cfg_if::cfg_if! { + if #[cfg(test)] { + use tests::time; + } else { + use ic_cdk::api::time; + } +} + use crate::{ acl::{Authorize, AuthorizeError, WithAuthorize}, id::Generate, @@ -310,3 +318,124 @@ impl Expire for Expirer { }) } } + +#[cfg(test)] +mod tests { + use anyhow::Error; + + use super::*; + use crate::{EXPIRATIONS, ID_GENERATOR, NAMES, REGISTRATIONS, RETRIES}; + + pub fn time() -> u64 { + 0 + } + + #[test] + fn get_empty() { + let getter = Getter::new(®ISTRATIONS); + + match getter.get(&String::from("id")) { + Err(GetError::NotFound) => {} + other => panic!("expected NotFound but got {other:?}"), + }; + } + + #[test] + fn get_ok() -> Result<(), Error> { + let reg = Registration { + name: Name::try_from("name")?, + canister: Principal::from_text("aaaaa-aa")?, + state: State::Available, + }; + + REGISTRATIONS.with(|regs| { + regs.borrow_mut().insert("id".into(), reg.clone()).unwrap(); + }); + + let getter = Getter::new(®ISTRATIONS); + + let out = match getter.get(&String::from("id")) { + Ok(reg) => reg, + other => panic!("expected registration but got {other:?}"), + }; + + assert_eq!(out, reg); + + Ok(()) + } + + #[test] + fn create_ok() -> Result<(), Error> { + crate::ID_SEED.with(|s| { + s.borrow_mut() + .insert((), 0) + .expect("failed to insert id seed") + }); + + let creator = Creator::new(&ID_GENERATOR, ®ISTRATIONS, &NAMES, &EXPIRATIONS); + + let id = creator.create( + "name", // name + &Principal::from_text("aaaaa-aa")?, // canister + )?; + + // Check regsitration + let reg = REGISTRATIONS + .with(|regs| regs.borrow().get(&id)) + .expect("expected registration to exist but none found"); + + assert_eq!( + reg, + Registration { + name: Name::try_from("name")?, + canister: Principal::from_text("aaaaa-aa")?, + state: State::PendingOrder, + } + ); + + // Check name + let iid = NAMES + .with(|names| { + names + .borrow() + .get(&Name::try_from("name").expect("failed to create name")) + }) + .expect("expected name mapping to exist but none found"); + + assert_eq!(id, iid, "expected ids to match"); + + Ok(()) + } + + #[test] + fn update_ok() -> Result<(), Error> { + let reg = Registration { + name: Name::try_from("name")?, + canister: Principal::from_text("aaaaa-aa")?, + state: State::PendingOrder, + }; + + REGISTRATIONS + .with(|regs| regs.borrow_mut().insert("id".into(), reg)) + .expect("failed to insert"); + + Updater::new(®ISTRATIONS, &EXPIRATIONS, &RETRIES) + .update("id".into(), State::PendingChallengeResponse)?; + + // Check registration + let reg = REGISTRATIONS + .with(|regs| regs.borrow().get(&String::from("id"))) + .expect("expected registration to exist but none found"); + + assert_eq!( + reg, + Registration { + name: Name::try_from("name")?, + canister: Principal::from_text("aaaaa-aa")?, + state: State::PendingChallengeResponse, + } + ); + + Ok(()) + } +} diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/work.rs b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/work.rs index 407d1ddd745..c1af635def1 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/work.rs +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator/src/work.rs @@ -1,10 +1,18 @@ use std::cmp::Reverse; use certificate_orchestrator_interface::{Id, Registration}; -use ic_cdk::{api::time, caller}; +use ic_cdk::caller; use ic_stable_structures::StableBTreeMap; use priority_queue::PriorityQueue; +cfg_if::cfg_if! { + if #[cfg(test)] { + use tests::time as time; + } else { + use ic_cdk::api::time; + } +} + use crate::{ acl::{Authorize, AuthorizeError, WithAuthorize}, LocalRef, Memory, IN_PROGRESS_TTL, @@ -227,3 +235,54 @@ impl Retry for Retrier { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + use crate::{RETRIES, TASKS}; + + pub fn time() -> u64 { + 0 + } + + #[test] + fn dispense_empty() { + match Dispenser::new(&TASKS, &RETRIES).dispense() { + Err(DispenseError::NoTasksAvailable) => {} + _ => panic!("Not the error that was expected."), + }; + } + + #[test] + fn dispense_ok() { + TASKS.with(|t| { + t.borrow_mut().push( + "id".into(), // item + Reverse(0), // priority + ) + }); + + let id = match Dispenser::new(&TASKS, &RETRIES).dispense() { + Ok(id) => id, + other => panic!("expected id but got {other:?}"), + }; + + assert_eq!(id, "id"); + } + + #[test] + fn dispense_unavailable() { + TASKS.with(|t| { + t.borrow_mut().push( + "id".into(), // item + Reverse(1), // priority + ) + }); + + match Dispenser::new(&TASKS, &RETRIES).dispense() { + Err(DispenseError::NoTasksAvailable) => {} + other => panic!("expected NoTasksAvailable but got {other:?}"), + }; + } +} diff --git a/rs/boundary_node/certificate_issuance/certificate_orchestrator_interface/src/lib.rs b/rs/boundary_node/certificate_issuance/certificate_orchestrator_interface/src/lib.rs index dea3b1a5e1b..32f8ba8ff55 100644 --- a/rs/boundary_node/certificate_issuance/certificate_orchestrator_interface/src/lib.rs +++ b/rs/boundary_node/certificate_issuance/certificate_orchestrator_interface/src/lib.rs @@ -24,7 +24,7 @@ impl Storable for EncryptedPair { } } -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, PartialEq, Deserialize)] pub struct Name(String); pub const NAME_MAX_LEN: u32 = 64; @@ -73,7 +73,7 @@ impl Storable for Name { } } -#[derive(Debug, CandidType, Clone, PartialEq, Eq, Deserialize)] +#[derive(Debug, CandidType, Clone, PartialEq, Deserialize)] pub enum State { #[serde(rename = "failed")] Failed(String), @@ -101,7 +101,7 @@ impl Storable for State { } } -#[derive(Debug, CandidType, Deserialize)] +#[derive(Debug, CandidType, Clone, PartialEq, Deserialize)] pub struct Registration { pub name: Name, pub canister: Principal, diff --git a/rs/boundary_node/certificate_issuance/certificate_syncer/BUILD.bazel b/rs/boundary_node/certificate_issuance/certificate_syncer/BUILD.bazel index 2d6d224d270..00ebc3859bf 100644 --- a/rs/boundary_node/certificate_issuance/certificate_syncer/BUILD.bazel +++ b/rs/boundary_node/certificate_issuance/certificate_syncer/BUILD.bazel @@ -1,4 +1,4 @@ -load("@rules_rust//rust:defs.bzl", "rust_binary") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_test") package(default_visibility = ["//visibility:public"]) @@ -10,12 +10,14 @@ DEPENDENCIES = [ "@crate_index//:futures", "@crate_index//:hyper-rustls", "@crate_index//:hyper", + "@crate_index//:mockall", "@crate_index//:nix", "@crate_index//:opentelemetry_0_18_0", "@crate_index//:opentelemetry_prometheus_0_11_0", "@crate_index//:prometheus", "@crate_index//:serde_json", "@crate_index//:serde", + "@crate_index//:tempfile", "@crate_index//:thiserror", "@crate_index//:tokio", "@crate_index//:tracing-subscriber", @@ -33,3 +35,10 @@ rust_binary( version = "0.1.0", deps = DEPENDENCIES, ) + +rust_test( + name = "certificate-syncer_test", + crate = ":certificate-syncer", + proc_macro_deps = MACRO_DEPENDENCIES, + deps = DEPENDENCIES, +) diff --git a/rs/boundary_node/certificate_issuance/certificate_syncer/Cargo.toml b/rs/boundary_node/certificate_issuance/certificate_syncer/Cargo.toml index 4ad814b6114..1f022b1157e 100644 --- a/rs/boundary_node/certificate_issuance/certificate_syncer/Cargo.toml +++ b/rs/boundary_node/certificate_issuance/certificate_syncer/Cargo.toml @@ -12,12 +12,14 @@ clap = { version = "4.0.29", features = ["derive"] } futures = "0.3.25" hyper = { version = "0.14.23", features = ["full"] } hyper-rustls = "0.23.2" +mockall = "0.11.3" nix = { version = "0.26.1", features = ["signal"] } opentelemetry = "0.18.0" opentelemetry-prometheus = "0.11.0" prometheus = "0.13.3" serde = { version = "1.0.150", features = ["serde_derive"] } serde_json = "1.0.89" +tempfile = "3.3.0" thiserror = "1.0.37" tokio = { version = "1.23.0", features = ["full"] } tracing = "0.1.37" diff --git a/rs/boundary_node/certificate_issuance/certificate_syncer/src/http.rs b/rs/boundary_node/certificate_issuance/certificate_syncer/src/http.rs index da831e85073..5b2da88cc90 100644 --- a/rs/boundary_node/certificate_issuance/certificate_syncer/src/http.rs +++ b/rs/boundary_node/certificate_issuance/certificate_syncer/src/http.rs @@ -2,11 +2,13 @@ use std::time::Instant; use async_trait::async_trait; use hyper::{client::connect::Connect, Body, Request, Response}; +use mockall::automock; use opentelemetry::{Context, KeyValue}; use tracing::info; use crate::metrics::{MetricParams, WithMetrics}; +#[automock] #[async_trait] pub trait HttpClient: Send + Sync { async fn request(&self, req: Request) -> Result, hyper::Error>; diff --git a/rs/boundary_node/certificate_issuance/certificate_syncer/src/import.rs b/rs/boundary_node/certificate_issuance/certificate_syncer/src/import.rs index 17d7ecdc198..7148f495088 100644 --- a/rs/boundary_node/certificate_issuance/certificate_syncer/src/import.rs +++ b/rs/boundary_node/certificate_issuance/certificate_syncer/src/import.rs @@ -19,13 +19,13 @@ pub enum ImportError { UnexpectedError(#[from] anyhow::Error), } -#[derive(Clone, PartialEq, Eq, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct Pair( pub Vec, // Private Key pub Vec, // Certificate Chain ); -#[derive(Clone, PartialEq, Eq, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct Package { pub name: String, pub canister: Principal, @@ -113,3 +113,58 @@ impl Import for WithMetrics { out } } + +#[cfg(test)] +mod tests { + use super::*; + + use anyhow::Error; + use hyper::Response; + use mockall::predicate; + use std::{str::FromStr, sync::Arc}; + + use crate::http::MockHttpClient; + + #[tokio::test] + async fn test_import() -> Result<(), Error> { + let mut http_client = MockHttpClient::new(); + http_client + .expect_request() + .times(1) + .with(predicate::function(|req: &Request| { + req.method().eq("GET") && req.uri().to_string().eq("http://certificates/") + })) + .returning(|_| { + Ok(Response::new(Body::from( + r#"[ + { + "name": "name", + "canister": "aaaaa-aa", + "pair": [ + [1, 2, 3], + [4, 5, 6] + ] + } + ]"#, + ))) + }); + + let importer = CertificatesImporter::new( + Arc::new(http_client), // http_client + Uri::from_str("http://certificates")?, // exporter_uri + ); + + let out = importer.import().await?; + + assert_eq!( + out, + vec![Package { + name: "name".into(), + canister: Principal::from_text("aaaaa-aa")?, + pair: Pair(vec![1, 2, 3], vec![4, 5, 6]), + }], + ); + + Ok(()) + } +} diff --git a/rs/boundary_node/certificate_issuance/certificate_syncer/src/persist.rs b/rs/boundary_node/certificate_issuance/certificate_syncer/src/persist.rs index 3e7005c5481..fad48210bdd 100644 --- a/rs/boundary_node/certificate_issuance/certificate_syncer/src/persist.rs +++ b/rs/boundary_node/certificate_issuance/certificate_syncer/src/persist.rs @@ -18,6 +18,7 @@ use crate::{ render::{Context, Render}, }; +#[derive(Debug, PartialEq)] pub enum PersistStatus { Completed, SkippedUnchanged, @@ -232,3 +233,73 @@ impl Persist for WithEmpty { self.0.persist(pkgs).await } } + +#[cfg(test)] +mod tests { + use super::*; + + use candid::Principal; + use std::sync::Arc; + + use crate::import::Pair; + use crate::render::Renderer; + + #[tokio::test] + async fn test_persist() -> Result<(), Error> { + use tempfile::tempdir; + + let tmp_dir = tempdir()?; + + let renderer = Renderer::new("{name}|{ssl_certificate_key_path}|{ssl_certificate_path}"); + + let persister = Persister::new( + Arc::new(renderer), // renderer + tmp_dir.path().join("certs"), // certificates_path + tmp_dir.path().join("conf"), // configuration_path + tmp_dir.path().join("mappings"), // domain_mappings_path + ); + + // Run persister + let out = persister + .persist(&[Package { + name: "test".into(), + canister: Principal::from_text("aaaaa-aa")?, + pair: Pair( + "key".to_string().into_bytes(), + "cert".to_string().into_bytes(), + ), + }]) + .await?; + + assert_eq!(out, PersistStatus::Completed); + + // Check certs + assert_eq!( + std::fs::read_to_string(tmp_dir.path().join("certs/test-key.pem"))?, + "key" + ); + + assert_eq!( + std::fs::read_to_string(tmp_dir.path().join("certs/test.pem"))?, + "cert" + ); + + // Check config + assert_eq!( + std::fs::read_to_string(tmp_dir.path().join("conf"))?, + format!( + "test|{}|{}", + tmp_dir.path().join("certs/test-key.pem").display(), + tmp_dir.path().join("certs/test.pem").display(), + ) + ); + + // Check mappings + assert_eq!( + std::fs::read_to_string(tmp_dir.path().join("mappings"))?, + "let domain_mappings = {\"test\":\"aaaaa-aa\"}; export default domain_mappings;", + ); + + Ok(()) + } +} diff --git a/rs/boundary_node/certificate_issuance/certificate_syncer/src/render.rs b/rs/boundary_node/certificate_issuance/certificate_syncer/src/render.rs index 2263ea37534..f7fd1a0f359 100644 --- a/rs/boundary_node/certificate_issuance/certificate_syncer/src/render.rs +++ b/rs/boundary_node/certificate_issuance/certificate_syncer/src/render.rs @@ -6,7 +6,7 @@ use serde::Serialize; use crate::metrics::{MetricParams, WithMetrics}; -#[derive(Serialize)] +#[derive(Debug, PartialEq, Serialize)] pub struct Context<'a> { pub name: &'a str, pub ssl_certificate_key_path: &'a str, @@ -63,3 +63,23 @@ impl Render for WithMetrics { out } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_render() { + let r = Renderer::new("{name}|{ssl_certificate_key_path}|{ssl_certificate_path}"); + + let out = r + .render(&Context { + name: "1", + ssl_certificate_key_path: "2", + ssl_certificate_path: "3", + }) + .expect("failed to render"); + + assert_eq!(out, "1|2|3"); + } +}