From 2a6ab9966071c7cde16b2809413a304319793a9c Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Sun, 25 Oct 2020 16:10:32 -0700 Subject: [PATCH 01/11] Merge service binaries into a single binary. aziot-{cert,identity,key}d are now symlinks to a new aziotd binary. This new binary looks at its `argv[0]` to figure out which service it's being invoked as, and runs the corresponding service's code. One advantage of this approach is that the common code for reading config env vars, binding the service connector and creating an HTTP server can be made common to all services. The second advantage is that the three services can now share a single copy of tokio, hyper, etc dependencies, that previously were statically linked into each individual binary. This has savings for both disk space as well as memory usage (from sharing code pages). Making the startup sequence code common also means all services now consistently use `env_logger` to set up logging. keyd and certd have been updated to use the `log` crate instead of `{e,}println!` Other changes: - Add support for reading multiple config files. The base file is still config.toml. In addition any files found under `config.d` will be applied as patches to the base config. This will be needed in the near future when `iotedge init` needs to add additional preloaded keys and certs to keyd's and certd's config. The same or a similar mechanism will also be useful for third-party packages to register principals with identityd. - Make naming of servers vs services vs APIs clearer. hyper owns the server. The server uses instances of a service to handle requests. The service implements the HTTP routes using an inner API. - Fix some tabs that rustfmt did not convert. - Don't run `cargo fmt -- --check` with `--quiet` regardless of the verbose flag, because it doesn't print errors if `--quiet` is specified. - Separate the rlib components of aziot-keys into aziot-keys-common. This is to avoid building aziot-keys as an rlib, because it triggers a cargo bug that causes it to be rebuilt over and over again even if it isn't modified. There were three things causing it to be built as an rlib: - Tests. The test target now excludes aziot-keys. - aziot used the `PreloadedKeyLocation` type from the crate. That type is now in the aziot-keys-common crate. - aziotd had it as a dependency to force it to be built first, because it has extern C fns that link to it. This is now enforced by the makefile instead. --- Cargo.lock | 57 ++- Cargo.toml | 3 + Makefile | 36 +- aziot/Cargo.toml | 2 +- aziot/src/init.rs | 8 +- aziotd/Cargo.toml | 20 + aziotd/src/logging.rs | 57 +++ aziotd/src/main.rs | 243 ++++++++++ cert/aziot-certd/Cargo.toml | 11 +- cert/aziot-certd/src/error.rs | 3 - cert/aziot-certd/src/http/create.rs | 12 +- .../src/http/get_or_import_or_delete.rs | 26 +- cert/aziot-certd/src/http/mod.rs | 25 +- cert/aziot-certd/src/lib.rs | 214 +++++---- cert/aziot-certd/src/main.rs | 90 ---- contrib/centos/aziot-identity-service.spec | 10 +- docs/dev/running/aziot-certd.md | 6 +- docs/dev/running/aziot-identityd.md | 11 +- docs/dev/running/aziot-keyd.md | 6 +- http-common/Cargo.toml | 1 + http-common/src/server.rs | 426 +++++++++--------- identity/aziot-identityd/Cargo.toml | 8 +- identity/aziot-identityd/src/app.rs | 48 -- .../http/create_or_list_module_identity.rs | 28 +- .../src/http/get_caller_identity.rs | 18 +- .../src/http/get_device_identity.rs | 18 +- .../src/http/get_trust_bundle.rs | 18 +- .../get_update_or_delete_module_identity.rs | 44 +- identity/aziot-identityd/src/http/mod.rs | 51 ++- .../src/http/reprovision_device.rs | 18 +- identity/aziot-identityd/src/lib.rs | 303 ++++++++++++- identity/aziot-identityd/src/logging.rs | 55 --- identity/aziot-identityd/src/main.rs | 305 ------------- key/aziot-key-openssl-engine/Cargo.toml | 1 + key/aziot-key-openssl-engine/src/lib.rs | 4 +- key/aziot-keyd/Cargo.toml | 12 +- key/aziot-keyd/src/http/create_derived_key.rs | 14 +- .../src/http/create_key_if_not_exists.rs | 14 +- .../src/http/create_key_pair_if_not_exists.rs | 14 +- key/aziot-keyd/src/http/decrypt.rs | 14 +- key/aziot-keyd/src/http/encrypt.rs | 14 +- key/aziot-keyd/src/http/export_derived_key.rs | 14 +- .../src/http/get_key_pair_public_parameter.rs | 14 +- key/aziot-keyd/src/http/load.rs | 16 +- key/aziot-keyd/src/http/mod.rs | 25 +- key/aziot-keyd/src/http/sign.rs | 14 +- key/aziot-keyd/src/keys.rs | 5 +- key/aziot-keyd/src/lib.rs | 99 +++- key/aziot-keyd/src/main.rs | 77 ---- key/aziot-keys-common/Cargo.toml | 10 + key/aziot-keys-common/src/lib.rs | 63 +++ key/aziot-keys/Cargo.toml | 4 +- key/aziot-keys/src/implementation.rs | 53 +-- key/aziot-keys/src/lib.rs | 5 - pkcs11/pkcs11-openssl-engine/Cargo.toml | 1 + pkcs11/pkcs11-openssl-engine/src/lib.rs | 4 +- 56 files changed, 1445 insertions(+), 1227 deletions(-) create mode 100644 aziotd/Cargo.toml create mode 100644 aziotd/src/logging.rs create mode 100644 aziotd/src/main.rs delete mode 100644 cert/aziot-certd/src/main.rs delete mode 100644 identity/aziot-identityd/src/app.rs delete mode 100644 identity/aziot-identityd/src/logging.rs delete mode 100644 identity/aziot-identityd/src/main.rs delete mode 100644 key/aziot-keyd/src/main.rs create mode 100644 key/aziot-keys-common/Cargo.toml create mode 100644 key/aziot-keys-common/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index f298b1756..e2bca10ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -74,7 +74,7 @@ dependencies = [ "aziot-certd", "aziot-identityd", "aziot-keyd", - "aziot-keys", + "aziot-keys-common", "backtrace", "base64", "bytes", @@ -122,7 +122,6 @@ dependencies = [ "aziot-key-common", "aziot-key-common-http", "aziot-key-openssl-engine", - "backtrace", "base64", "foreign-types-shared", "futures-util", @@ -132,6 +131,7 @@ dependencies = [ "hyper", "hyper-openssl", "lazy_static", + "log", "openssl", "openssl-build", "openssl-sys", @@ -240,8 +240,6 @@ dependencies = [ "aziot-key-openssl-engine", "base64", "chrono", - "clap", - "env_logger", "futures-util", "http", "http-common", @@ -313,6 +311,7 @@ dependencies = [ "aziot-key-common", "base64", "foreign-types-shared", + "log", "openssl", "openssl-build", "openssl-errors", @@ -343,14 +342,14 @@ dependencies = [ "async-trait", "aziot-key-common", "aziot-key-common-http", - "aziot-keys", - "backtrace", + "aziot-keys-common", "base64", "futures-util", "http", "http-common", "hyper", "lazy_static", + "log", "openssl", "openssl-sys", "percent-encoding", @@ -366,10 +365,12 @@ dependencies = [ name = "aziot-keys" version = "0.1.0" dependencies = [ + "aziot-keys-common", "foreign-types-shared", "hex", "hmac", "lazy_static", + "log", "openssl", "openssl-sys", "openssl-sys2", @@ -381,6 +382,31 @@ dependencies = [ "url", ] +[[package]] +name = "aziot-keys-common" +version = "0.1.0" +dependencies = [ + "pkcs11", + "url", +] + +[[package]] +name = "aziotd" +version = "0.1.0" +dependencies = [ + "aziot-certd", + "aziot-identityd", + "aziot-keyd", + "backtrace", + "env_logger", + "http-common", + "hyper", + "log", + "serde", + "tokio", + "toml", +] + [[package]] name = "backtrace" version = "0.3.50" @@ -568,9 +594,9 @@ dependencies = [ [[package]] name = "env_logger" -version = "0.5.13" +version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15b0a4d2e39f8420210be8b27eeda28029729e2fd4291019455016c348240c38" +checksum = "54532e3223c5af90a6a757c90b5c5521564b07e5e7a958681bcd2afad421cdcd" dependencies = [ "atty", "humantime", @@ -785,6 +811,7 @@ dependencies = [ "http", "hyper", "lazy_static", + "log", "nix", "percent-encoding", "serde", @@ -801,12 +828,9 @@ checksum = "cd179ae861f0c2e53da70d892f5f3029f9594be0c41dc5269cd371691b1dc2f9" [[package]] name = "humantime" -version = "1.3.0" +version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df004cfca50ef23c36850aaaa59ad52cc70d0e90243c3c7737a4dd32dc7a3c4f" -dependencies = [ - "quick-error", -] +checksum = "3c1ad908cc71012b7bea4d0c53ba96a8cba9962f048fa68d143376143d863b7a" [[package]] name = "hyper" @@ -1285,6 +1309,7 @@ name = "pkcs11-openssl-engine" version = "0.1.0" dependencies = [ "foreign-types-shared", + "log", "openssl", "openssl-build", "openssl-errors", @@ -1380,12 +1405,6 @@ dependencies = [ "unicode-xid", ] -[[package]] -name = "quick-error" -version = "1.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" - [[package]] name = "quote" version = "1.0.7" diff --git a/Cargo.toml b/Cargo.toml index 255ba15db..e042a0472 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,8 @@ members = [ "aziot", + "aziotd", + "cert/aziot-cert-client-async", "cert/aziot-cert-common", "cert/aziot-cert-common-http", @@ -25,6 +27,7 @@ members = [ "key/aziot-key-openssl-engine", "key/aziot-key-openssl-engine-shared", "key/aziot-keys", + "key/aziot-keys-common", "openssl2", "openssl-build", diff --git a/Makefile b/Makefile index f2fc82f81..8670876d8 100644 --- a/Makefile +++ b/Makefile @@ -87,13 +87,20 @@ default: $(BINDGEN_EXTRA_FLAGS); \ mv key/aziot-keyd/src/keys.generated.rs.tmp key/aziot-keyd/src/keys.generated.rs; \ fi + + # aziot-keys must be built before aziot-keyd is, because aziot-keyd needs to link to it. + # But we can't do this with Cargo dependencies because of a cargo issue that causes spurious rebuilds. + # So instead we do it manually. + # + # See the doc header of the aziot-keys-common crate for more info. + $(CARGO) build \ + -p aziot-keys \ + $(CARGO_PROFILE) --target $(CARGO_TARGET) $(CARGO_VERBOSE) + $(CARGO) build \ -p aziot \ - -p aziot-certd \ - -p aziot-identityd \ - -p aziot-keyd \ + -p aziotd \ -p aziot-key-openssl-engine-shared \ - -p aziot-keys \ $(CARGO_PROFILE) --target $(CARGO_TARGET) $(CARGO_VERBOSE) @@ -145,7 +152,7 @@ test: default iotedged pkcs11-test test: target/openapi-schema-validated test: set -o pipefail; \ - $(CARGO) test --all --exclude aziot-key-openssl-engine-shared \ + $(CARGO) test --all --exclude aziot-key-openssl-engine-shared --exclude aziot-keys \ $(CARGO_PROFILE) --target $(CARGO_TARGET) $(CARGO_VERBOSE) 2>&1 | \ grep -v 'running 0 tests' | grep -v '0 passed; 0 failed' | grep '.' @@ -168,7 +175,7 @@ test: $(CARGO) clippy --all --exclude aziot-key-openssl-engine-shared --tests $(CARGO_PROFILE) --target $(CARGO_TARGET) $(CARGO_VERBOSE) $(CARGO) clippy --all --examples $(CARGO_PROFILE) --target $(CARGO_TARGET) $(CARGO_VERBOSE) - $(CARGO) fmt --all $(CARGO_VERBOSE) -- --check + $(CARGO) fmt --all -- --check find . -name 'Makefile' -or -name '*.c' -or -name '*.md' -or -name '*.rs' -or -name '*.toml' -or -name '*.txt' | \ grep -v '^\./target/' | \ @@ -203,7 +210,7 @@ dist: # Copy source files cp -R \ - ./aziot ./cert ./http-common ./identity ./iotedged ./key ./openssl-build ./openssl-sys2 ./openssl2 ./pkcs11 \ + ./aziot ./aziotd ./cert ./http-common ./identity ./iotedged ./key ./openssl-build ./openssl-sys2 ./openssl2 ./pkcs11 \ /tmp/aziot-identity-service-$(PACKAGE_VERSION) cp ./Cargo.toml ./Cargo.lock ./CODE_OF_CONDUCT.md ./CONTRIBUTING.md ./LICENSE ./Makefile ./README.md ./rust-toolchain ./SECURITY.md /tmp/aziot-identity-service-$(PACKAGE_VERSION) @@ -292,18 +299,25 @@ install-common: # Ref: https://www.gnu.org/software/make/manual/html_node/DESTDIR.html # Binaries - $(INSTALL_PROGRAM) -D target/$(CARGO_TARGET)/$(CARGO_PROFILE_DIRECTORY)/aziot-certd $(DESTDIR)$(libexecdir)/aziot-identity-service/aziot-certd - $(INSTALL_PROGRAM) -D target/$(CARGO_TARGET)/$(CARGO_PROFILE_DIRECTORY)/aziot-keyd $(DESTDIR)$(libexecdir)/aziot-identity-service/aziot-keyd - $(INSTALL_PROGRAM) -D target/$(CARGO_TARGET)/$(CARGO_PROFILE_DIRECTORY)/aziot-identityd $(DESTDIR)$(libexecdir)/aziot-identity-service/aziot-identityd + $(INSTALL_PROGRAM) -D target/$(CARGO_TARGET)/$(CARGO_PROFILE_DIRECTORY)/aziotd $(DESTDIR)$(libexecdir)/aziot-identity-service/aziotd + ln -s $(libexecdir)/aziot-identity-service/aziotd $(DESTDIR)$(libexecdir)/aziot-identity-service/aziot-certd + ln -s $(libexecdir)/aziot-identity-service/aziotd $(DESTDIR)$(libexecdir)/aziot-identity-service/aziot-identityd + ln -s $(libexecdir)/aziot-identity-service/aziotd $(DESTDIR)$(libexecdir)/aziot-identity-service/aziot-keyd + $(INSTALL_PROGRAM) -D target/$(CARGO_TARGET)/$(CARGO_PROFILE_DIRECTORY)/aziot $(DESTDIR)$(bindir)/aziot # libaziot-keys $(INSTALL_PROGRAM) -D target/$(CARGO_TARGET)/$(CARGO_PROFILE_DIRECTORY)/libaziot_keys.so $(DESTDIR)$(libdir)/libaziot_keys.so - # Default configs + # Default configs and config directories $(INSTALL_DATA) -D cert/aziot-certd/config/unix/default.toml $(DESTDIR)$(sysconfdir)/aziot/certd/config.toml.default + $(INSTALL) -d -m 0700 $(DESTDIR)$(sysconfdir)/aziot/certd/config.d + $(INSTALL_DATA) -D identity/aziot-identityd/config/unix/default.toml $(DESTDIR)$(sysconfdir)/aziot/identityd/config.toml.default + $(INSTALL) -d -m 0700 $(DESTDIR)$(sysconfdir)/aziot/identityd/config.d + $(INSTALL_DATA) -D key/aziot-keyd/config/unix/default.toml $(DESTDIR)$(sysconfdir)/aziot/keyd/config.toml.default + $(INSTALL) -d -m 0700 $(DESTDIR)$(sysconfdir)/aziot/keyd/config.d # Home directories $(INSTALL) -d -m 0700 $(DESTDIR)$(localstatedir)/lib/aziot/certd diff --git a/aziot/Cargo.toml b/aziot/Cargo.toml index 318cdea01..6898dd3f5 100644 --- a/aziot/Cargo.toml +++ b/aziot/Cargo.toml @@ -17,7 +17,7 @@ url = "2" aziot-certd = { path = "../cert/aziot-certd" } aziot-identityd = { path = "../identity/aziot-identityd" } aziot-keyd = { path = "../key/aziot-keyd" } -aziot-keys = { path = "../key/aziot-keys" } +aziot-keys-common = { path = "../key/aziot-keys-common" } [dev-dependencies] bytes = "0.5" diff --git a/aziot/src/init.rs b/aziot/src/init.rs index 64b7d9d6c..f580dc68e 100644 --- a/aziot/src/init.rs +++ b/aziot/src/init.rs @@ -318,7 +318,7 @@ fn run_inner(stdin: &mut impl Reader) -> Result { } if preloaded_device_id_pk_bytes.is_some() { - let device_id_pk_uri = aziot_keys::PreloadedKeyLocation::Filesystem { + let device_id_pk_uri = aziot_keys_common::PreloadedKeyLocation::Filesystem { path: "/var/secrets/aziot/keyd/device-id".into(), }; keyd_config @@ -935,8 +935,8 @@ fn parse_manual_connection_string( )) } -fn parse_preloaded_key_location(value: &str) -> Option { - match value.parse::() { +fn parse_preloaded_key_location(value: &str) -> Option { + match value.parse::() { Ok(value) => Some(value), Err(err) => { @@ -947,7 +947,7 @@ fn parse_preloaded_key_location(value: &str) -> Option() + .parse::() }); match value { Ok(value) => Some(value), diff --git a/aziotd/Cargo.toml b/aziotd/Cargo.toml new file mode 100644 index 000000000..b884e686e --- /dev/null +++ b/aziotd/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "aziotd" +version = "0.1.0" +authors = ["Azure IoT Edge Devs"] +edition = "2018" + + +[dependencies] +backtrace = "0.3" +env_logger = "0.8" +hyper = "0.13" +log = "0.4" +serde = "1" +tokio = { version = "0.2", features = ["macros"] } +toml = "0.5" + +aziot-certd = { path = "../cert/aziot-certd" } +aziot-identityd = { path = "../identity/aziot-identityd" } +aziot-keyd = { path = "../key/aziot-keyd" } +http-common = { path = "../http-common" } diff --git a/aziotd/src/logging.rs b/aziotd/src/logging.rs new file mode 100644 index 000000000..d760d9b51 --- /dev/null +++ b/aziotd/src/logging.rs @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft. All rights reserved. + +// use std::env; +// use std::io::Write; + +// use log::{Level, LevelFilter}; + +const LOG_LEVEL_ENV_VAR: &str = "AZIOT_LOG"; + +pub(crate) fn init() { + env_logger::Builder::new() + .format(|fmt, record| { + use std::io::Write; + + let level = match record.level() { + log::Level::Trace => "TRCE", + log::Level::Debug => "DBUG", + log::Level::Info => "INFO", + log::Level::Warn => "WARN", + log::Level::Error => "ERR!", + }; + let timestamp = fmt.timestamp(); + + if record.level() >= log::Level::Debug { + writeln!( + fmt, + "<{}>{} [{}] - [{}] {}", + to_syslog_level(record.level()), + timestamp, + level, + record.target(), + record.args() + ) + } else { + writeln!( + fmt, + "<{}>{} [{}] - {}", + to_syslog_level(record.level()), + timestamp, + level, + record.args() + ) + } + }) + .filter_level(log::LevelFilter::Info) + .parse_env(LOG_LEVEL_ENV_VAR) + .init(); +} + +fn to_syslog_level(level: log::Level) -> i8 { + match level { + log::Level::Error => 3, + log::Level::Warn => 4, + log::Level::Info => 6, + log::Level::Debug | log::Level::Trace => 7, + } +} diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs new file mode 100644 index 000000000..ec0ce3922 --- /dev/null +++ b/aziotd/src/main.rs @@ -0,0 +1,243 @@ +// Copyright (c) Microsoft. All rights reserved. + +#![deny(rust_2018_idioms, warnings)] +#![deny(clippy::all, clippy::pedantic)] +#![allow(clippy::default_trait_access, clippy::let_unit_value)] + +mod logging; + +#[tokio::main] +async fn main() { + logging::init(); + + if let Err(err) = main_inner().await { + log::error!("{}", err.0); + + let mut source = std::error::Error::source(&err.0); + while let Some(err) = source { + log::error!("caused by: {}", err); + source = std::error::Error::source(err); + } + + log::error!("{:?}", err.1); + + std::process::exit(1); + } +} + +async fn main_inner() -> Result<(), Error> { + let argv0 = std::env::args_os() + .next() + .ok_or_else(|| ErrorKind::GetProcessName("argv[0] not set".into()))?; + + // argv[0] could be a single component like "aziot-certd", or a path that ends with "aziot-certd", + // so parse it as a Path and get the last component. This does the right thing in either case. + let argv0 = std::path::Path::new(&argv0); + let process_name = argv0.file_name().ok_or_else(|| { + ErrorKind::GetProcessName( + format!( + "could not extract process name from argv[0] {:?}", + argv0.display(), + ) + .into(), + ) + })?; + + match process_name.to_str() { + Some("aziot-certd") => { + run( + aziot_certd::main, + "AZIOT_CERTD_CONFIG", + "/etc/aziot/certd/config.toml", + "AZIOT_CERTD_CONFIG_DIR", + "/etc/aziot/certd/config.d", + ) + .await? + } + + Some("aziot-identityd") => { + run( + aziot_identityd::main, + "AZIOT_IDENTITYD_CONFIG", + "/etc/aziot/identityd/config.toml", + "AZIOT_IDENTITYD_CONFIG_DIR", + "/etc/aziot/identityd/config.d", + ) + .await? + } + + Some("aziot-keyd") => { + run( + aziot_keyd::main, + "AZIOT_CERTD_CONFIG", + "/etc/aziot/keyd/config.toml", + "AZIOT_CERTD_CONFIG_DIR", + "/etc/aziot/keyd/config.d", + ) + .await? + } + _ => { + return Err(ErrorKind::GetProcessName( + format!("unrecognized process name {:?}", process_name).into(), + ) + .into()) + } + } + + Ok(()) +} + +async fn run( + main: TMain, + config_env_var: &str, + config_file_default: &str, + config_directory_env_var: &str, + config_directory_default: &str, +) -> Result<(), Error> +where + TMain: FnOnce(TConfig) -> TFuture, + TConfig: serde::de::DeserializeOwned, + TFuture: std::future::Future< + Output = Result<(http_common::Connector, TServer), Box>, + >, + TServer: hyper::service::Service< + hyper::Request, + Response = hyper::Response, + Error = std::convert::Infallible, + > + Clone + + Send + + 'static, + >>::Future: Send, +{ + log::info!("Starting service..."); + log::info!( + "Version - {}", + option_env!("PACKAGE_VERSION").unwrap_or("dev build"), + ); + + let config_path: std::path::PathBuf = + std::env::var_os(config_env_var).map_or_else(|| config_file_default.into(), Into::into); + + let config = std::fs::read(&config_path) + .map_err(|err| ErrorKind::ReadConfig(Some(config_path.clone()), Box::new(err)))?; + let mut config: toml::Value = toml::from_slice(&config) + .map_err(|err| ErrorKind::ReadConfig(Some(config_path), Box::new(err)))?; + + let config_directory_path: std::path::PathBuf = std::env::var_os(config_directory_env_var) + .map_or_else(|| config_directory_default.into(), Into::into); + + match std::fs::read_dir(&config_directory_path) { + Ok(entries) => { + for entry in entries { + let entry = entry.map_err(|err| { + ErrorKind::ReadConfig(Some(config_directory_path.clone()), Box::new(err)) + })?; + + let entry_file_type = entry.file_type().map_err(|err| { + ErrorKind::ReadConfig(Some(config_directory_path.clone()), Box::new(err)) + })?; + if !entry_file_type.is_file() { + continue; + } + + let patch_path = entry.path(); + if patch_path.extension().and_then(std::ffi::OsStr::to_str) != Some(".toml") { + continue; + } + + let patch = std::fs::read(&patch_path).map_err(|err| { + ErrorKind::ReadConfig(Some(patch_path.clone()), Box::new(err)) + })?; + let patch: toml::Value = toml::from_slice(&patch) + .map_err(|err| ErrorKind::ReadConfig(Some(patch_path), Box::new(err)))?; + merge_toml(&mut config, patch); + } + } + + Err(err) if err.kind() == std::io::ErrorKind::NotFound => (), + + Err(err) => { + return Err(ErrorKind::ReadConfig(Some(config_directory_path), Box::new(err)).into()) + } + } + + let config: TConfig = serde::Deserialize::deserialize(config) + .map_err(|err| ErrorKind::ReadConfig(None, Box::new(err)))?; + + let (connector, server) = main(config).await.map_err(ErrorKind::Service)?; + + log::info!("Starting server..."); + + let incoming = connector + .incoming() + .await + .map_err(|err| ErrorKind::Service(Box::new(err)))?; + let server = hyper::Server::builder(incoming).serve(hyper::service::make_service_fn(|_| { + let server = server.clone(); + async move { Ok::<_, std::convert::Infallible>(server.clone()) } + })); + let () = server + .await + .map_err(|err| ErrorKind::Service(Box::new(err)))?; + + log::info!("Stopped server."); + + Ok(()) +} + +fn merge_toml(base: &mut toml::Value, patch: toml::Value) { + // Similar to JSON patch, except that maps are called tables, and + // there is no equivalent of null that can be used to remove keys from an object. + + if let toml::Value::Table(base) = base { + if let toml::Value::Table(patch) = patch { + for (k, v) in patch { + merge_toml(base.entry(k).or_insert(toml::Value::Boolean(false)), v); + } + + return; + } + } + + *base = patch; +} + +#[derive(Debug)] +struct Error(ErrorKind, backtrace::Backtrace); + +#[derive(Debug)] +enum ErrorKind { + GetProcessName(std::borrow::Cow<'static, str>), + ReadConfig(Option, Box), + Service(Box), +} + +impl std::fmt::Display for ErrorKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ErrorKind::GetProcessName(message) => write!(f, "could not read argv[0]: {}", message), + ErrorKind::ReadConfig(Some(path), _) => { + write!(f, "could not read config from {}", path.display()) + } + ErrorKind::ReadConfig(None, _) => f.write_str("could not read config"), + ErrorKind::Service(_) => f.write_str("service encountered an error"), + } + } +} + +impl std::error::Error for ErrorKind { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + #[allow(clippy::match_same_arms)] + match self { + ErrorKind::GetProcessName(_) => None, + ErrorKind::ReadConfig(_, err) => Some(&**err), + ErrorKind::Service(err) => Some(&**err), + } + } +} + +impl From for Error { + fn from(err: ErrorKind) -> Self { + Error(err, Default::default()) + } +} diff --git a/cert/aziot-certd/Cargo.toml b/cert/aziot-certd/Cargo.toml index d6a05bf4f..45ee10bd3 100644 --- a/cert/aziot-certd/Cargo.toml +++ b/cert/aziot-certd/Cargo.toml @@ -5,9 +5,9 @@ authors = ["Azure IoT Edge Devs"] edition = "2018" build = "build/main.rs" + [dependencies] async-trait = "0.1" -backtrace = "0.3" base64 = "0.12" foreign-types-shared = "0.1" futures-util = "0.3" @@ -16,14 +16,14 @@ http = "0.2" hyper = "0.13" hyper-openssl = "0.8" lazy_static = "1" +log = "0.4" openssl = "0.10" openssl-sys = "0.9" percent-encoding = "2" regex = "1" serde = { version = "1", features = ["derive"] } serde_json = "1" -tokio = { version = "0.2", features = ["macros"] } -toml = "0.5" +tokio = "0.2" url = { version = "2", features = ["serde"] } aziot-cert-common-http = { path = "../aziot-cert-common-http" } @@ -34,5 +34,10 @@ aziot-key-openssl-engine = { path = "../../key/aziot-key-openssl-engine" } http-common = { path = "../../http-common" } openssl2 = { path = "../../openssl2" } + [build-dependencies] openssl-build = { path = "../../openssl-build/" } + + +[dev-dependencies] +toml = "0.5" diff --git a/cert/aziot-certd/src/error.rs b/cert/aziot-certd/src/error.rs index 7926df198..d44308240 100644 --- a/cert/aziot-certd/src/error.rs +++ b/cert/aziot-certd/src/error.rs @@ -42,7 +42,6 @@ pub enum InternalError { DeleteFile(std::io::Error), GetPath(Box), LoadKeyOpensslEngine(openssl2::Error), - ReadConfig(Box), ReadFile(std::io::Error), } @@ -57,7 +56,6 @@ impl std::fmt::Display for InternalError { InternalError::LoadKeyOpensslEngine(_) => { f.write_str("could not load aziot-key-openssl-engine") } - InternalError::ReadConfig(_) => f.write_str("could not read config"), InternalError::ReadFile(_) => f.write_str("could not read cert file"), } } @@ -71,7 +69,6 @@ impl std::error::Error for InternalError { InternalError::DeleteFile(err) => Some(err), InternalError::GetPath(err) => Some(&**err), InternalError::LoadKeyOpensslEngine(err) => Some(err), - InternalError::ReadConfig(err) => Some(&**err), InternalError::ReadFile(err) => Some(err), } } diff --git a/cert/aziot-certd/src/http/create.rs b/cert/aziot-certd/src/http/create.rs index fd6915a36..bf9580674 100644 --- a/cert/aziot-certd/src/http/create.rs +++ b/cert/aziot-certd/src/http/create.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_cert_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -42,8 +42,8 @@ impl http_common::server::Route for Route { message: "missing request body".into(), })?; - let pem = aziot_certd::Server::create_cert( - self.inner, + let pem = crate::Api::create_cert( + self.api, body.cert_id, body.csr.0, body.issuer.map( diff --git a/cert/aziot-certd/src/http/get_or_import_or_delete.rs b/cert/aziot-certd/src/http/get_or_import_or_delete.rs index dba6a8b52..23c21c555 100644 --- a/cert/aziot-certd/src/http/get_or_import_or_delete.rs +++ b/cert/aziot-certd/src/http/get_or_import_or_delete.rs @@ -7,7 +7,7 @@ lazy_static::lazy_static! { } pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, cert_id: String, } @@ -18,9 +18,9 @@ impl http_common::server::Route for Route { &((aziot_cert_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -32,7 +32,7 @@ impl http_common::server::Route for Route { .ok()?; Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), cert_id: cert_id.into_owned(), }) } @@ -43,10 +43,10 @@ impl http_common::server::Route for Route { self, _body: Option, ) -> http_common::server::RouteResponse> { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - if let Err(err) = inner.delete_cert(&self.cert_id) { + if let Err(err) = api.delete_cert(&self.cert_id) { return Err(super::to_http_error(&err)); } @@ -55,10 +55,10 @@ impl http_common::server::Route for Route { type GetResponse = aziot_cert_common_http::get_cert::Response; async fn get(self) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let pem = inner.get_cert(&self.cert_id); + let pem = api.get_cert(&self.cert_id); let pem = match pem { Ok(pem) => pem, Err(err) => return Err(super::to_http_error(&err)), @@ -79,10 +79,10 @@ impl http_common::server::Route for Route { self, body: Self::PutBody, ) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - match inner.import_cert(&self.cert_id, &body.pem.0) { + match api.import_cert(&self.cert_id, &body.pem.0) { Ok(()) => (), Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/cert/aziot-certd/src/http/mod.rs b/cert/aziot-certd/src/http/mod.rs index fd5bbe5b6..3846a09d4 100644 --- a/cert/aziot-certd/src/http/mod.rs +++ b/cert/aziot-certd/src/http/mod.rs @@ -3,12 +3,13 @@ mod create; mod get_or_import_or_delete; -pub(crate) struct Server { - pub(crate) inner: std::sync::Arc>, +#[derive(Clone)] +pub struct Service { + pub(crate) api: std::sync::Arc>, } -http_common::make_server! { - server: Server, +http_common::make_service! { + service: Service, api_version: aziot_cert_common_http::ApiVersion, routes: [ create::Route, @@ -16,23 +17,31 @@ http_common::make_server! { ], } -fn to_http_error(err: &aziot_certd::Error) -> http_common::server::Error { +fn to_http_error(err: &crate::Error) -> http_common::server::Error { let error_message = http_common::server::error_to_message(err); // TODO: When we get distributed tracing, associate these logs with the tracing ID. for line in error_message.split('\n') { - eprintln!("!!! {}", line); + log::log!( + if matches!(err, crate::Error::Internal(_)) { + log::Level::Info + } else { + log::Level::Error + }, + "!!! {}", + line, + ); } match err { // Do not use error_message because we don't want to leak internal errors to the client. // Just return the top-level error, ie "internal error" - aziot_certd::Error::Internal(_) => http_common::server::Error { + crate::Error::Internal(_) => http_common::server::Error { status_code: hyper::StatusCode::INTERNAL_SERVER_ERROR, message: err.to_string().into(), }, - aziot_certd::Error::InvalidParameter(_, _) => http_common::server::Error { + crate::Error::InvalidParameter(_, _) => http_common::server::Error { status_code: hyper::StatusCode::BAD_REQUEST, message: error_message.into(), }, diff --git a/cert/aziot-certd/src/lib.rs b/cert/aziot-certd/src/lib.rs index b80ac8361..211527f4f 100644 --- a/cert/aziot-certd/src/lib.rs +++ b/cert/aziot-certd/src/lib.rs @@ -18,43 +18,67 @@ pub use config::{ }; mod error; -pub use error::{Error, InternalError}; +use error::{Error, InternalError}; mod est; -pub struct Server { - homedir_path: std::path::PathBuf, - cert_issuance: crate::config::CertIssuance, - preloaded_certs: std::collections::BTreeMap, - - key_client: std::sync::Arc, - key_engine: openssl2::FunctionalEngine, -} +mod http; + +pub async fn main( + config: Config, +) -> Result<(http_common::Connector, http::Service), Box> { + let Config { + homedir_path, + cert_issuance, + preloaded_certs, + endpoints: + Endpoints { + aziot_certd: connector, + aziot_keyd: key_connector, + }, + } = config; + + let api = { + let key_client = { + let key_client = aziot_key_client::Client::new( + aziot_key_common_http::ApiVersion::V2020_09_01, + key_connector, + ); + let key_client = std::sync::Arc::new(key_client); + key_client + }; -impl Server { - pub fn new( - homedir_path: std::path::PathBuf, - cert_issuance: crate::config::CertIssuance, - preloaded_certs: std::collections::BTreeMap, - key_client: std::sync::Arc, - ) -> Result { let key_engine = aziot_key_openssl_engine::load(key_client.clone()) .map_err(|err| Error::Internal(InternalError::LoadKeyOpensslEngine(err)))?; - Ok(Server { + Api { homedir_path, cert_issuance, preloaded_certs, key_client, key_engine, - }) - } + } + }; + let api = std::sync::Arc::new(futures_util::lock::Mutex::new(api)); + + let service = http::Service { api }; + + Ok((connector, service)) } -impl Server { +struct Api { + homedir_path: std::path::PathBuf, + cert_issuance: crate::config::CertIssuance, + preloaded_certs: std::collections::BTreeMap, + + key_client: std::sync::Arc, + key_engine: openssl2::FunctionalEngine, +} + +impl Api { pub async fn create_cert( - this: std::sync::Arc>, + this: std::sync::Arc>, id: String, csr: Vec, issuer: Option<(String, aziot_key_common::KeyHandle)>, @@ -177,7 +201,7 @@ fn load_inner(path: &std::path::Path) -> Result>, Error> { } fn create_cert<'a>( - server: &'a mut Server, + api: &'a mut Api, id: &'a str, csr: &'a [u8], issuer: Option<(&'a str, &'a aziot_key_common::KeyHandle)>, @@ -187,13 +211,13 @@ fn create_cert<'a>( // and the recursive call is for the outer boxed-future-returning fn. async fn create_cert_inner( - server: &mut Server, + api: &mut Api, id: &str, csr: &[u8], issuer: Option<(&str, &aziot_key_common::KeyHandle)>, ) -> Result, Error> { // Look up issuance options for this certificate ID. - let cert_options = server.cert_issuance.certs.get(id); + let cert_options = api.cert_issuance.certs.get(id); if let Some((issuer_id, issuer_private_key)) = issuer { // Issuer is explicitly specified, so load it and use it to sign the CSR. @@ -270,7 +294,7 @@ fn create_cert<'a>( let issuer_private_key = std::ffi::CString::new(issuer_private_key.0.clone()) .map_err(|err| Error::invalid_parameter("issuer.privateKeyHandle", err))?; - let issuer_private_key = server + let issuer_private_key = api .key_engine .load_private_key(&issuer_private_key) .map_err(|err| Error::Internal(InternalError::CreateCert(Box::new(err))))?; @@ -290,8 +314,7 @@ fn create_cert<'a>( } else { // Load the issuer and use it to sign the CSR. - let issuer_path = - get_path(&server.homedir_path, &server.preloaded_certs, issuer_id)?; + let issuer_path = get_path(&api.homedir_path, &api.preloaded_certs, issuer_id)?; let issuer_x509_pem = load_inner(&issuer_path) .map_err(|err| Error::Internal(InternalError::CreateCert(Box::new(err))))? .ok_or_else(|| Error::invalid_parameter("issuer.certId", "not found"))?; @@ -317,7 +340,7 @@ fn create_cert<'a>( x509 }; - let path = get_path(&server.homedir_path, &server.preloaded_certs, id)?; + let path = get_path(&api.homedir_path, &api.preloaded_certs, id)?; std::fs::write(path, &x509) .map_err(|err| Error::Internal(InternalError::CreateCert(Box::new(err))))?; @@ -335,12 +358,12 @@ fn create_cert<'a>( auth, trusted_certs, urls, - } = server.cert_issuance.est.as_ref().ok_or_else(|| { + } = api.cert_issuance.est.as_ref().ok_or_else(|| { Error::Internal(InternalError::CreateCert( format!( - "cert {:?} is configured to be issued by EST, but EST is not configured", - id, - ) + "cert {:?} is configured to be issued by EST, but EST is not configured", + id, + ) .into(), )) })?; @@ -351,9 +374,9 @@ fn create_cert<'a>( .ok_or_else(|| { Error::Internal(InternalError::CreateCert( format!( - "cert {:?} is configured to be issued by EST, but the EST endpoint URL for it is not configured", - id, - ) + "cert {:?} is configured to be issued by EST, but the EST endpoint URL for it is not configured", + id, + ) .into(), )) })?; @@ -367,20 +390,17 @@ fn create_cert<'a>( let mut trusted_certs_x509 = vec![]; for trusted_cert in trusted_certs { - let pem = get_cert_inner( - &server.homedir_path, - &server.preloaded_certs, - trusted_cert, - )? - .ok_or_else(|| { - Error::Internal(InternalError::CreateCert( - format!( + let pem = + get_cert_inner(&api.homedir_path, &api.preloaded_certs, trusted_cert)? + .ok_or_else(|| { + Error::Internal(InternalError::CreateCert( + format!( "cert_issuance.est.trusted_certs contains unreadable cert {:?}", trusted_cert, ) - .into(), - )) - })?; + .into(), + )) + })?; let x509 = openssl::x509::X509::stack_from_pem(&pem).map_err(|err| { Error::Internal(InternalError::CreateCert(Box::new(err))) })?; @@ -397,12 +417,12 @@ fn create_cert<'a>( // Try to load the EST identity cert. let identity = match get_cert_inner( - &server.homedir_path, - &server.preloaded_certs, + &api.homedir_path, + &api.preloaded_certs, identity_cert, ) { Ok(Some(identity_cert)) => { - match server.key_client.load_key_pair(identity_private_key) { + match api.key_client.load_key_pair(identity_private_key) { Ok(identity_private_key) => { Ok((identity_cert, identity_private_key)) } @@ -428,7 +448,7 @@ fn create_cert<'a>( err, ))) })?; - let identity_private_key = server + let identity_private_key = api .key_engine .load_private_key(&identity_private_key) .map_err(|err| { @@ -444,8 +464,7 @@ fn create_cert<'a>( ) .await?; - let path = - get_path(&server.homedir_path, &server.preloaded_certs, id)?; + let path = get_path(&api.homedir_path, &api.preloaded_certs, id)?; std::fs::write(path, &x509).map_err(|err| { Error::Internal(InternalError::CreateCert(Box::new(err))) })?; @@ -460,24 +479,24 @@ fn create_cert<'a>( bootstrap_identity_private_key, )) = bootstrap_identity { - match get_cert_inner(&server.homedir_path, &server.preloaded_certs, bootstrap_identity_cert) { - Ok(Some(bootstrap_identity_cert)) => match server.key_client.load_key_pair(bootstrap_identity_private_key) { - Ok(bootstrap_identity_private_key) => Ok((bootstrap_identity_cert, bootstrap_identity_private_key)), - Err(err) => Err(format!("could not get EST bootstrap identity cert private key: {}", err)), - }, - - Ok(None) => Err(format!( - "could not get EST bootstrap identity cert: {}", - std::io::Error::from(std::io::ErrorKind::NotFound), - )), - - Err(err) => Err(format!("could not get EST bootstrap identity cert: {}", err)), - } + match get_cert_inner(&api.homedir_path, &api.preloaded_certs, bootstrap_identity_cert) { + Ok(Some(bootstrap_identity_cert)) => match api.key_client.load_key_pair(bootstrap_identity_private_key) { + Ok(bootstrap_identity_private_key) => Ok((bootstrap_identity_cert, bootstrap_identity_private_key)), + Err(err) => Err(format!("could not get EST bootstrap identity cert private key: {}", err)), + }, + + Ok(None) => Err(format!( + "could not get EST bootstrap identity cert: {}", + std::io::Error::from(std::io::ErrorKind::NotFound), + )), + + Err(err) => Err(format!("could not get EST bootstrap identity cert: {}", err)), + } } else { Err(format!( "cert {:?} is configured to be issued by EST, \ - but EST identity could not be obtained \ - and EST bootstrap identity is not configured; {}", + but EST identity could not be obtained \ + and EST bootstrap identity is not configured; {}", id, identity_err, )) }; @@ -489,7 +508,7 @@ fn create_cert<'a>( )) => { // Create a CSR for the new EST identity cert. - let identity_key_pair_handle = server + let identity_key_pair_handle = api .key_client .create_key_pair_if_not_exists( identity_private_key, @@ -510,7 +529,7 @@ fn create_cert<'a>( Box::new(err), )) })?; - let identity_public_key = server + let identity_public_key = api .key_engine .load_public_key(&identity_key_pair_handle) .map_err(|err| { @@ -518,7 +537,7 @@ fn create_cert<'a>( Box::new(err), )) })?; - let identity_private_key = server + let identity_private_key = api .key_engine .load_private_key(&identity_key_pair_handle) .map_err(|err| { @@ -623,12 +642,12 @@ fn create_cert<'a>( })?; let identity_url = - urls.get(identity_cert) - .or_else(|| urls.get("default")) - .ok_or_else(|| Error::Internal(InternalError::CreateCert(format!( - "cert {:?} is configured to be issued by EST, but the EST endpoint URL for the EST identity is not configured", - id, - ).into())))?; + urls.get(identity_cert) + .or_else(|| urls.get("default")) + .ok_or_else(|| Error::Internal(InternalError::CreateCert(format!( + "cert {:?} is configured to be issued by EST, but the EST endpoint URL for the EST identity is not configured", + id, + ).into())))?; // Request the new EST identity cert using the EST bootstrap identity cert. @@ -643,7 +662,7 @@ fn create_cert<'a>( )) }, )?; - let bootstrap_identity_private_key = server + let bootstrap_identity_private_key = api .key_engine .load_private_key(&bootstrap_identity_private_key) .map_err(|err| { @@ -665,8 +684,8 @@ fn create_cert<'a>( .await?; let path = get_path( - &server.homedir_path, - &server.preloaded_certs, + &api.homedir_path, + &api.preloaded_certs, identity_cert, )?; std::fs::write(path, &x509).map_err(|err| { @@ -677,19 +696,19 @@ fn create_cert<'a>( // EST identity cert was obtained and persisted successfully. Now recurse to retry the original cert request. - let x509 = create_cert(server, id, csr, issuer).await?; + let x509 = create_cert(api, id, csr, issuer).await?; Ok(x509) } Err(bootstrap_identity_err) => { // Neither EST identity cert nor EST bootstrap identity cert could be obtained. Err(Error::Internal(InternalError::CreateCert(format!( - "cert {:?} is configured to be issued by EST, but neither EST identity nor EST bootstrap identity could be obtained; \ - {} {}", - id, - identity_err, - bootstrap_identity_err, - ).into()))) + "cert {:?} is configured to be issued by EST, but neither EST identity nor EST bootstrap identity could be obtained; \ + {} {}", + id, + identity_err, + bootstrap_identity_err, + ).into()))) } } } @@ -706,7 +725,7 @@ fn create_cert<'a>( ) .await?; - let path = get_path(&server.homedir_path, &server.preloaded_certs, id)?; + let path = get_path(&api.homedir_path, &api.preloaded_certs, id)?; std::fs::write(path, &x509).map_err(|err| { Error::Internal(InternalError::CreateCert(Box::new(err))) })?; @@ -718,10 +737,10 @@ fn create_cert<'a>( config::CertIssuanceMethod::LocalCa => { // Indirect reference to the local CA. Look it up. - let (issuer_cert, issuer_private_key) = match &server.cert_issuance.local_ca { + let (issuer_cert, issuer_private_key) = match &api.cert_issuance.local_ca { Some(config::LocalCa { cert, pk }) => { let private_key = - server.key_client.load_key_pair(pk).map_err(|err| { + api.key_client.load_key_pair(pk).map_err(|err| { Error::Internal(InternalError::CreateCert(Box::new(err))) })?; (cert.clone(), private_key) @@ -730,9 +749,9 @@ fn create_cert<'a>( None => { return Err(Error::Internal(InternalError::CreateCert( format!( - "cert {:?} is configured to be issued by local CA, but local CA is not configured", - id, - ) + "cert {:?} is configured to be issued by local CA, but local CA is not configured", + id, + ) .into(), ))) } @@ -740,9 +759,8 @@ fn create_cert<'a>( // Recurse with the local CA set explicitly as the issuer parameter. - let x509 = - create_cert(server, id, csr, Some((&issuer_cert, &issuer_private_key))) - .await?; + let x509 = create_cert(api, id, csr, Some((&issuer_cert, &issuer_private_key))) + .await?; Ok(x509) } @@ -750,20 +768,20 @@ fn create_cert<'a>( // Since the client did not give us their private key handle, we assume that the key is named the same as the cert. // // TODO: Is there a way to not have to assume this? - let key_pair_handle = server + let key_pair_handle = api .key_client .load_key_pair(id) .map_err(|err| Error::Internal(InternalError::CreateCert(Box::new(err))))?; // Recurse with explicit issuer. - let x509 = create_cert(server, id, csr, Some((id, &key_pair_handle))).await?; + let x509 = create_cert(api, id, csr, Some((id, &key_pair_handle))).await?; Ok(x509) } } } } - Box::pin(create_cert_inner(server, id, csr, issuer)) + Box::pin(create_cert_inner(api, id, csr, issuer)) } fn get_cert_inner( diff --git a/cert/aziot-certd/src/main.rs b/cert/aziot-certd/src/main.rs deleted file mode 100644 index 98f399691..000000000 --- a/cert/aziot-certd/src/main.rs +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -#![deny(rust_2018_idioms, warnings)] -#![deny(clippy::all, clippy::pedantic)] -#![allow( - clippy::default_trait_access, - clippy::let_and_return, - clippy::let_unit_value, - clippy::similar_names, - clippy::type_complexity -)] - -mod http; - -#[tokio::main] -async fn main() -> Result<(), Error> { - let config_path: std::path::PathBuf = std::env::var_os("AZIOT_CERTD_CONFIG") - .map_or_else(|| "/etc/aziot/certd/config.toml".into(), Into::into); - - let config = std::fs::read(config_path).map_err(|err| { - aziot_certd::Error::Internal(aziot_certd::InternalError::ReadConfig(Box::new(err))) - })?; - let aziot_certd::Config { - homedir_path, - cert_issuance, - preloaded_certs, - endpoints: - aziot_certd::Endpoints { - aziot_certd: connector, - aziot_keyd: key_connector, - }, - } = toml::from_slice(&config).map_err(|err| { - aziot_certd::Error::Internal(aziot_certd::InternalError::ReadConfig(Box::new(err))) - })?; - - let key_client = { - let key_client = aziot_key_client::Client::new( - aziot_key_common_http::ApiVersion::V2020_09_01, - key_connector, - ); - let key_client = std::sync::Arc::new(key_client); - key_client - }; - - let server = - aziot_certd::Server::new(homedir_path, cert_issuance, preloaded_certs, key_client)?; - let server = std::sync::Arc::new(futures_util::lock::Mutex::new(server)); - - eprintln!("Starting server..."); - - let incoming = connector.incoming().await?; - let server = hyper::Server::builder(incoming).serve(hyper::service::make_service_fn(|_| { - let server = http::Server { - inner: server.clone(), - }; - futures_util::future::ok::<_, std::convert::Infallible>(server) - })); - let () = server.await?; - - eprintln!("Server stopped."); - - Ok(()) -} - -struct Error(Box, backtrace::Backtrace); - -impl std::fmt::Debug for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "{}", self.0)?; - - let mut source = self.0.source(); - while let Some(err) = source { - writeln!(f, "caused by: {}", err)?; - source = err.source(); - } - - writeln!(f, "{:?}", self.1)?; - - Ok(()) - } -} - -impl From for Error -where - E: Into>, -{ - fn from(err: E) -> Self { - Error(err.into(), Default::default()) - } -} diff --git a/contrib/centos/aziot-identity-service.spec b/contrib/centos/aziot-identity-service.spec index db6383e85..6cea462a9 100644 --- a/contrib/centos/aziot-identity-service.spec +++ b/contrib/centos/aziot-identity-service.spec @@ -109,10 +109,13 @@ fi %files + # Binaries +%{_libexecdir}/%{name}/aziotd %{_libexecdir}/%{name}/aziot-certd %{_libexecdir}/%{name}/aziot-identityd %{_libexecdir}/%{name}/aziot-keyd + %{_bindir}/aziot # libaziot-keys @@ -121,10 +124,15 @@ fi # libaziot-key-openssl-engine-shared %{_libdir}/openssl/engines/libaziot_keys.so -# Default configs +# Default configs and config directories %attr(400, aziotcs, aziotcs) %{_sysconfdir}/aziot/certd/config.toml.default +%attr(700, aziotcs, aziotcs) %dir %{_sysconfdir}/aziot/certd/config.d + %attr(400, aziotid, aziotid) %{_sysconfdir}/aziot/identityd/config.toml.default +%attr(700, aziotid, aziotid) %dir %{_sysconfdir}/aziot/identityd/config.d + %attr(400, aziotks, aziotks) %{_sysconfdir}/aziot/keyd/config.toml.default +%attr(700, aziotks, aziotks) %dir %{_sysconfdir}/aziot/keyd/config.d # Home directories %attr(-, aziotcs, aziotcs) %dir /var/lib/aziot/certd diff --git a/docs/dev/running/aziot-certd.md b/docs/dev/running/aziot-certd.md index d68b9e04c..ab0b8f2c2 100644 --- a/docs/dev/running/aziot-certd.md +++ b/docs/dev/running/aziot-certd.md @@ -205,9 +205,13 @@ With this basic file, fill it out depending on what workflow you want to test: As mentioned at the beginning, if your config file is not saved at `/etc/aziot/certd/config.toml`, set the `AZIOT_CERTD_CONFIG` env var to its actual path. ```sh + export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$PWD/target/x86_64-unknown-linux-gnu/debug" + + export AZIOT_LOG=aziot=debug + export AZIOT_CERTD_CONFIG='...' - cargo run -p aziot-certd + (exec -a aziot-certd target/x86_64-unknown-linux-gnu/debug/aziotd) ``` The service will remain running. diff --git a/docs/dev/running/aziot-identityd.md b/docs/dev/running/aziot-identityd.md index 4c02af5b7..c145e9072 100644 --- a/docs/dev/running/aziot-identityd.md +++ b/docs/dev/running/aziot-identityd.md @@ -147,9 +147,14 @@ The table of principal accounts in Identity Service configuration represents eac ## Running `aziot-identityd` -Run the following command to launch the Identity Service, passing the `config.toml` file path as an argument: +Run the following command to launch the Identity Service, setting the path of the `config.toml` file as the `AZIOT_IDENTITYD_CONFIG` env var: ```sh -cargo run -p aziot-identityd -- -c /path/to/default.toml -``` +export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$PWD/target/x86_64-unknown-linux-gnu/debug" + +export AZIOT_LOG=aziot=debug +export AZIOT_IDENTITYD_CONFIG='...' + +(exec -a aziot-identityd target/x86_64-unknown-linux-gnu/debug/aziotd) +``` diff --git a/docs/dev/running/aziot-keyd.md b/docs/dev/running/aziot-keyd.md index fd7fa0bed..a92ff5989 100644 --- a/docs/dev/running/aziot-keyd.md +++ b/docs/dev/running/aziot-keyd.md @@ -121,9 +121,13 @@ Assuming you're using Microsoft's implementation of `libaziot_keys.so`, start wi 1. Run the service, setting the `AZIOT_KEYD_CONFIG` env var if necessary. ```sh + export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$PWD/target/x86_64-unknown-linux-gnu/debug" + + export AZIOT_LOG=aziot=debug + export AZIOT_KEYD_CONFIG='...' - cargo run -p aziot-keyd + (exec -a aziot-keyd target/x86_64-unknown-linux-gnu/debug/aziotd) ``` The service will remain running. diff --git a/http-common/Cargo.toml b/http-common/Cargo.toml index df94e9ac2..83e2a87e5 100644 --- a/http-common/Cargo.toml +++ b/http-common/Cargo.toml @@ -11,6 +11,7 @@ futures-util = "0.3" http = "0.2" hyper = { version = "0.13", optional = true } lazy_static = "1" +log = "0.4" nix = "0.18" percent-encoding = "2" serde = { version = "1", features = ["derive"] } diff --git a/http-common/src/server.rs b/http-common/src/server.rs index 85b94996f..d79145c73 100644 --- a/http-common/src/server.rs +++ b/http-common/src/server.rs @@ -3,212 +3,212 @@ use crate::DynRangeBounds; #[macro_export] -macro_rules! make_server { - ( - server: $server_ty:ty, - api_version: $api_version_ty:ty, - routes: [ - $($route:path ,)* - ], - ) => { - impl hyper::service::Service> for $server_ty { - type Response = hyper::Response; - type Error = std::convert::Infallible; - type Future = std::pin::Pin> + Send>>; - - fn poll_ready(&mut self, _cx: &mut std::task::Context<'_>) -> std::task::Poll> { - std::task::Poll::Ready(Ok(())) - } - - fn call(&mut self, req: hyper::Request) -> Self::Future { - fn call_inner( - this: &mut $server_ty, - req: hyper::Request, - ) -> std::pin::Pin, std::convert::Infallible>> + Send>> { - let (http::request::Parts { method, uri, headers, .. }, body) = req.into_parts(); - - let path = uri.path(); - - let (api_version, query_params) = { - let mut api_version = None; - let mut query_params = vec![]; - - if let Some(query) = uri.query() { - let mut params = url::form_urlencoded::parse(query.as_bytes()); - while let Some((name, value)) = params.next() { - if name == "api-version" { - api_version = Some(value); - } - else { - query_params.push((name, value)); - } - } - } - - let api_version = match api_version { - Some(api_version) => api_version, - None => return Box::pin(futures_util::future::ok((http_common::server::Error { - status_code: http::StatusCode::BAD_REQUEST, - message: "api-version not specified".into(), - }).to_http_response())), - }; - let api_version: $api_version_ty = match api_version.parse() { - Ok(api_version) => api_version, - Err(()) => return Box::pin(futures_util::future::ok((http_common::server::Error { - status_code: http::StatusCode::BAD_REQUEST, - message: format!("invalid api-version {:?}", api_version).into(), - }).to_http_response())), - }; - (api_version, query_params) - }; - - $( - let route_api_version_matches = <$route as http_common::server::Route>::api_version().contains(&api_version); - if route_api_version_matches { - let route: Option<$route> = http_common::server::Route::from_uri(&*this, path, &query_params); - if let Some(route) = route { - return Box::pin(async move { - let response = match method { - http::Method::DELETE => { - let body = { - let content_type = headers.get(hyper::header::CONTENT_TYPE).and_then(|value| value.to_str().ok()); - if content_type.as_deref() == Some("application/json") { - let body = match hyper::body::to_bytes(body).await { - Ok(body) => body, - Err(err) => return Ok((http_common::server::Error { - status_code: http::StatusCode::BAD_REQUEST, - message: http_common::server::error_to_message(&err).into(), - }).to_http_response()), - }; - - let body: <$route as http_common::server::Route>::DeleteBody = match serde_json::from_slice(&body) { - Ok(body) => body, - Err(err) => return Ok((http_common::server::Error { - status_code: http::StatusCode::UNPROCESSABLE_ENTITY, - message: http_common::server::error_to_message(&err).into(), - }).to_http_response()), - }; - - Some(body) - } - else { - None - } - }; - - let (status_code, response) = match <$route as http_common::server::Route>::delete(route, body).await { - Ok(result) => result, - Err(err) => return Ok(err.to_http_response()), - }; - http_common::server::json_response(status_code, response.as_ref()) - }, - - http::Method::GET => { - let (status_code, response) = match <$route as http_common::server::Route>::get(route).await { - Ok(result) => result, - Err(err) => return Ok(err.to_http_response()), - }; - http_common::server::json_response(status_code, Some(&response)) - }, - - http::Method::POST => { - let body = { - let content_type = headers.get(hyper::header::CONTENT_TYPE).and_then(|value| value.to_str().ok()); - if content_type.as_deref() == Some("application/json") { - let body = match hyper::body::to_bytes(body).await { - Ok(body) => body, - Err(err) => return Ok((http_common::server::Error { - status_code: http::StatusCode::BAD_REQUEST, - message: http_common::server::error_to_message(&err).into(), - }).to_http_response()), - }; - - let body: <$route as http_common::server::Route>::PostBody = match serde_json::from_slice(&body) { - Ok(body) => body, - Err(err) => return Ok((http_common::server::Error { - status_code: http::StatusCode::UNPROCESSABLE_ENTITY, - message: http_common::server::error_to_message(&err).into(), - }).to_http_response()), - }; - - Some(body) - } - else { - None - } - }; - - let (status_code, response) = match <$route as http_common::server::Route>::post(route, body).await { - Ok(result) => result, - Err(err) => return Ok(err.to_http_response()), - }; - http_common::server::json_response(status_code, response.as_ref()) - }, - - http::Method::PUT => { - let content_type = headers.get(hyper::header::CONTENT_TYPE).and_then(|value| value.to_str().ok()); - if content_type.as_deref() != Some("application/json") { - return Ok((http_common::server::Error { - status_code: http::StatusCode::UNSUPPORTED_MEDIA_TYPE, - message: "request body must be application/json".into(), - }).to_http_response()); - } - - let body = match hyper::body::to_bytes(body).await { - Ok(body) => body, - Err(err) => return Ok((http_common::server::Error { - status_code: http::StatusCode::BAD_REQUEST, - message: http_common::server::error_to_message(&err).into(), - }).to_http_response()), - }; - - let body: <$route as http_common::server::Route>::PutBody = match serde_json::from_slice(&body) { - Ok(body) => body, - Err(err) => return Ok((http_common::server::Error { - status_code: http::StatusCode::UNPROCESSABLE_ENTITY, - message: http_common::server::error_to_message(&err).into(), - }).to_http_response()), - }; - - let (status_code, response) = match <$route as http_common::server::Route>::put(route, body).await { - Ok(result) => result, - Err(err) => return Ok(err.to_http_response()), - }; - http_common::server::json_response(status_code, Some(&response)) - }, - - _ => return Ok((http_common::server::Error { - status_code: http::StatusCode::BAD_REQUEST, - message: "method not allowed".into(), - }).to_http_response()), - }; - Ok(response) - }) - } - } - )* - - let res = (http_common::server::Error { - status_code: http::StatusCode::NOT_FOUND, - message: "not found".into(), - }).to_http_response(); - Box::pin(futures_util::future::ok(res)) - } - - // TODO: When we get distributed tracing, associate these two logs with the tracing ID. - eprintln!("<-- {:?} {:?} {:?}", req.method(), req.uri(), req.headers()); - let res = call_inner(self, req); - Box::pin(async move { - let res = res.await; - match &res { - Ok(res) => eprintln!("--> {:?} {:?}", res.status(), res.headers()), - Err(err) => eprintln!("-!> {:?}", err), - } - res - }) - } - } - }; +macro_rules! make_service { + ( + service: $service_ty:ty, + api_version: $api_version_ty:ty, + routes: [ + $($route:path ,)* + ], + ) => { + impl hyper::service::Service> for $service_ty { + type Response = hyper::Response; + type Error = std::convert::Infallible; + type Future = std::pin::Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut std::task::Context<'_>) -> std::task::Poll> { + std::task::Poll::Ready(Ok(())) + } + + fn call(&mut self, req: hyper::Request) -> Self::Future { + fn call_inner( + this: &mut $service_ty, + req: hyper::Request, + ) -> std::pin::Pin, std::convert::Infallible>> + Send>> { + let (http::request::Parts { method, uri, headers, .. }, body) = req.into_parts(); + + let path = uri.path(); + + let (api_version, query_params) = { + let mut api_version = None; + let mut query_params = vec![]; + + if let Some(query) = uri.query() { + let mut params = url::form_urlencoded::parse(query.as_bytes()); + while let Some((name, value)) = params.next() { + if name == "api-version" { + api_version = Some(value); + } + else { + query_params.push((name, value)); + } + } + } + + let api_version = match api_version { + Some(api_version) => api_version, + None => return Box::pin(futures_util::future::ok((http_common::server::Error { + status_code: http::StatusCode::BAD_REQUEST, + message: "api-version not specified".into(), + }).to_http_response())), + }; + let api_version: $api_version_ty = match api_version.parse() { + Ok(api_version) => api_version, + Err(()) => return Box::pin(futures_util::future::ok((http_common::server::Error { + status_code: http::StatusCode::BAD_REQUEST, + message: format!("invalid api-version {:?}", api_version).into(), + }).to_http_response())), + }; + (api_version, query_params) + }; + + $( + let route_api_version_matches = <$route as http_common::server::Route>::api_version().contains(&api_version); + if route_api_version_matches { + let route: Option<$route> = http_common::server::Route::from_uri(&*this, path, &query_params); + if let Some(route) = route { + return Box::pin(async move { + let response = match method { + http::Method::DELETE => { + let body = { + let content_type = headers.get(hyper::header::CONTENT_TYPE).and_then(|value| value.to_str().ok()); + if content_type.as_deref() == Some("application/json") { + let body = match hyper::body::to_bytes(body).await { + Ok(body) => body, + Err(err) => return Ok((http_common::server::Error { + status_code: http::StatusCode::BAD_REQUEST, + message: http_common::server::error_to_message(&err).into(), + }).to_http_response()), + }; + + let body: <$route as http_common::server::Route>::DeleteBody = match serde_json::from_slice(&body) { + Ok(body) => body, + Err(err) => return Ok((http_common::server::Error { + status_code: http::StatusCode::UNPROCESSABLE_ENTITY, + message: http_common::server::error_to_message(&err).into(), + }).to_http_response()), + }; + + Some(body) + } + else { + None + } + }; + + let (status_code, response) = match <$route as http_common::server::Route>::delete(route, body).await { + Ok(result) => result, + Err(err) => return Ok(err.to_http_response()), + }; + http_common::server::json_response(status_code, response.as_ref()) + }, + + http::Method::GET => { + let (status_code, response) = match <$route as http_common::server::Route>::get(route).await { + Ok(result) => result, + Err(err) => return Ok(err.to_http_response()), + }; + http_common::server::json_response(status_code, Some(&response)) + }, + + http::Method::POST => { + let body = { + let content_type = headers.get(hyper::header::CONTENT_TYPE).and_then(|value| value.to_str().ok()); + if content_type.as_deref() == Some("application/json") { + let body = match hyper::body::to_bytes(body).await { + Ok(body) => body, + Err(err) => return Ok((http_common::server::Error { + status_code: http::StatusCode::BAD_REQUEST, + message: http_common::server::error_to_message(&err).into(), + }).to_http_response()), + }; + + let body: <$route as http_common::server::Route>::PostBody = match serde_json::from_slice(&body) { + Ok(body) => body, + Err(err) => return Ok((http_common::server::Error { + status_code: http::StatusCode::UNPROCESSABLE_ENTITY, + message: http_common::server::error_to_message(&err).into(), + }).to_http_response()), + }; + + Some(body) + } + else { + None + } + }; + + let (status_code, response) = match <$route as http_common::server::Route>::post(route, body).await { + Ok(result) => result, + Err(err) => return Ok(err.to_http_response()), + }; + http_common::server::json_response(status_code, response.as_ref()) + }, + + http::Method::PUT => { + let content_type = headers.get(hyper::header::CONTENT_TYPE).and_then(|value| value.to_str().ok()); + if content_type.as_deref() != Some("application/json") { + return Ok((http_common::server::Error { + status_code: http::StatusCode::UNSUPPORTED_MEDIA_TYPE, + message: "request body must be application/json".into(), + }).to_http_response()); + } + + let body = match hyper::body::to_bytes(body).await { + Ok(body) => body, + Err(err) => return Ok((http_common::server::Error { + status_code: http::StatusCode::BAD_REQUEST, + message: http_common::server::error_to_message(&err).into(), + }).to_http_response()), + }; + + let body: <$route as http_common::server::Route>::PutBody = match serde_json::from_slice(&body) { + Ok(body) => body, + Err(err) => return Ok((http_common::server::Error { + status_code: http::StatusCode::UNPROCESSABLE_ENTITY, + message: http_common::server::error_to_message(&err).into(), + }).to_http_response()), + }; + + let (status_code, response) = match <$route as http_common::server::Route>::put(route, body).await { + Ok(result) => result, + Err(err) => return Ok(err.to_http_response()), + }; + http_common::server::json_response(status_code, Some(&response)) + }, + + _ => return Ok((http_common::server::Error { + status_code: http::StatusCode::BAD_REQUEST, + message: "method not allowed".into(), + }).to_http_response()), + }; + Ok(response) + }) + } + } + )* + + let res = (http_common::server::Error { + status_code: http::StatusCode::NOT_FOUND, + message: "not found".into(), + }).to_http_response(); + Box::pin(futures_util::future::ok(res)) + } + + // TODO: When we get distributed tracing, associate these two logs with the tracing ID. + log::info!("<-- {:?} {:?} {:?}", req.method(), req.uri(), req.headers()); + let res = call_inner(self, req); + Box::pin(async move { + let res = res.await; + match &res { + Ok(res) => log::info!("--> {:?} {:?}", res.status(), res.headers()), + Err(err) => log::error!("-!> {:?}", err), + } + res + }) + } + } + }; } // DEVNOTE: Set *Body assoc type to `serde::de::IgnoredAny` if the corresponding method isn't overridden. @@ -217,9 +217,9 @@ pub trait Route: Sized { type ApiVersion: std::cmp::PartialOrd; fn api_version() -> &'static dyn DynRangeBounds; - type Server; + type Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option; @@ -351,15 +351,15 @@ mod test_server { } } - http_common::make_server! { - server: Server, + http_common::make_service! { + service: Service, api_version: ApiVersion, routes: [ test_route::Route, ], } - struct Server; + struct Service; mod test_route { use crate as http_common; @@ -375,9 +375,9 @@ mod test_server { &(..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - _server: &Self::Server, + _service: &Self::Service, _path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { diff --git a/identity/aziot-identityd/Cargo.toml b/identity/aziot-identityd/Cargo.toml index 9d2f10190..cc3362abc 100644 --- a/identity/aziot-identityd/Cargo.toml +++ b/identity/aziot-identityd/Cargo.toml @@ -8,12 +8,11 @@ The code used for Identity Service. """ edition = "2018" + [dependencies] async-trait = "0.1" base64 = "0.12" chrono = "0.4" -clap = "2.31" -env_logger = "0.5" futures-util = "0.3" http = "0.2" hyper = "0.13" @@ -25,11 +24,10 @@ percent-encoding = "2" regex = "1" serde = { version = "1", features = ["derive"] } serde_json = "1.0" -tokio = { version = "0.2", features = ["macros", "time"] } +tokio = { version = "0.2", features = ["time"] } toml = "0.5" url = { version = "2.0", features = ["serde"] } - aziot-cert-common-http = { path = "../../cert/aziot-cert-common-http" } aziot-cert-client-async = { path = "../../cert/aziot-cert-client-async" } aziot-dps-client-async = { path = "../aziot-dps-client-async" } @@ -44,5 +42,7 @@ aziot-key-openssl-engine = { path = "../../key/aziot-key-openssl-engine" } http-common = { path = "../../http-common" } openssl2 = { path = "../../openssl2" } + [dev-dependencies] test-case = "1.0.0" +tokio = { version = "0.2", features = ["macros"] } diff --git a/identity/aziot-identityd/src/app.rs b/identity/aziot-identityd/src/app.rs deleted file mode 100644 index 61cdae45a..000000000 --- a/identity/aziot-identityd/src/app.rs +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -use std::{ - ffi::{OsStr, OsString}, - path::PathBuf, -}; - -use clap::{crate_description, crate_name, crate_version, App, Arg}; -use log::info; - -use crate::error::InternalError; -use crate::logging; - -fn create_app(default_config_file: &OsStr) -> App<'_, '_> { - App::new(crate_name!()) - .version(crate_version!()) - .about(crate_description!()) - .arg( - Arg::with_name("config-file") - .short("c") - .long("config-file") - .value_name("FILE") - .help("Sets daemon configuration file") - .takes_value(true) - .default_value_os(default_config_file), - ) -} - -pub fn init() -> Result { - let default_config_file = OsString::from("/etc/aziot/identityd/config.toml"); - - let matches = create_app(&default_config_file).get_matches(); - - logging::init(); - - info!("Starting Azure IoT Identity Service Daemon"); - info!("Version - {}", "1.0"); - - let config_file: std::path::PathBuf = matches - .value_of_os("config-file") - .expect("arg has a default value") - .to_os_string() - .into(); - - info!("Using config file: {}", config_file.display()); - - Ok(config_file) -} diff --git a/identity/aziot-identityd/src/http/create_or_list_module_identity.rs b/identity/aziot-identityd/src/http/create_or_list_module_identity.rs index 1a6caaf29..acb755ceb 100644 --- a/identity/aziot-identityd/src/http/create_or_list_module_identity.rs +++ b/identity/aziot-identityd/src/http/create_or_list_module_identity.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_identity_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -31,17 +31,17 @@ impl http_common::server::Route for Route { type GetResponse = aziot_identity_common_http::get_module_identities::Response; async fn get(self) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - let identities = match inner.get_identities(auth_id, "aziot").await { + let identities = match api.get_identities(auth_id, "aziot").await { Ok(v) => v, Err(err) => return Err(super::to_http_error(&err)), }; @@ -60,17 +60,17 @@ impl http_common::server::Route for Route { message: "missing request body".into(), })?; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - let identity = match inner + let identity = match api .create_identity(auth_id, &body.id_type, &body.module_id) .await { diff --git a/identity/aziot-identityd/src/http/get_caller_identity.rs b/identity/aziot-identityd/src/http/get_caller_identity.rs index 1cc60a34f..e3ffe405d 100644 --- a/identity/aziot-identityd/src/http/get_caller_identity.rs +++ b/identity/aziot-identityd/src/http/get_caller_identity.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_identity_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -31,17 +31,17 @@ impl http_common::server::Route for Route { type GetResponse = aziot_identity_common_http::get_module_identity::Response; async fn get(self) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - let identity = match inner.get_caller_identity(auth_id).await { + let identity = match api.get_caller_identity(auth_id).await { Ok(v) => v, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/identity/aziot-identityd/src/http/get_device_identity.rs b/identity/aziot-identityd/src/http/get_device_identity.rs index 2960a422d..cb21e841e 100644 --- a/identity/aziot-identityd/src/http/get_device_identity.rs +++ b/identity/aziot-identityd/src/http/get_device_identity.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_identity_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -42,17 +42,17 @@ impl http_common::server::Route for Route { message: "missing request body".into(), })?; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - let identity = match inner.get_device_identity(auth_id, &body.id_type).await { + let identity = match api.get_device_identity(auth_id, &body.id_type).await { Ok(v) => v, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/identity/aziot-identityd/src/http/get_trust_bundle.rs b/identity/aziot-identityd/src/http/get_trust_bundle.rs index 95f40d796..40b96b37b 100644 --- a/identity/aziot-identityd/src/http/get_trust_bundle.rs +++ b/identity/aziot-identityd/src/http/get_trust_bundle.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_identity_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -31,17 +31,17 @@ impl http_common::server::Route for Route { type GetResponse = aziot_identity_common_http::get_trust_bundle::Response; async fn get(self) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - let certificate = match inner.get_trust_bundle(auth_id).await { + let certificate = match api.get_trust_bundle(auth_id).await { Ok(v) => v, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/identity/aziot-identityd/src/http/get_update_or_delete_module_identity.rs b/identity/aziot-identityd/src/http/get_update_or_delete_module_identity.rs index 497255b22..70eed68a1 100644 --- a/identity/aziot-identityd/src/http/get_update_or_delete_module_identity.rs +++ b/identity/aziot-identityd/src/http/get_update_or_delete_module_identity.rs @@ -7,7 +7,7 @@ lazy_static::lazy_static! { } pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, module_id: String, } @@ -18,9 +18,9 @@ impl http_common::server::Route for Route { &((aziot_identity_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -32,7 +32,7 @@ impl http_common::server::Route for Route { .ok()?; Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), module_id: module_id.into_owned(), }) } @@ -43,20 +43,17 @@ impl http_common::server::Route for Route { self, _body: Option, ) -> http_common::server::RouteResponse> { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - match inner - .delete_identity(auth_id, "aziot", &self.module_id) - .await - { + match api.delete_identity(auth_id, "aziot", &self.module_id).await { Ok(()) => (), Err(err) => return Err(super::to_http_error(&err)), } @@ -66,17 +63,17 @@ impl http_common::server::Route for Route { type GetResponse = aziot_identity_common_http::get_module_identity::Response; async fn get(self) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - let identity = match inner.get_identity(auth_id, "aziot", &self.module_id).await { + let identity = match api.get_identity(auth_id, "aziot", &self.module_id).await { Ok(v) => v, Err(err) => return Err(super::to_http_error(&err)), }; @@ -99,19 +96,16 @@ impl http_common::server::Route for Route { self, _body: Self::PutBody, ) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; - let identity = match inner - .update_identity(auth_id, "aziot", &self.module_id) - .await - { + let identity = match api.update_identity(auth_id, "aziot", &self.module_id).await { Ok(v) => v, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/identity/aziot-identityd/src/http/mod.rs b/identity/aziot-identityd/src/http/mod.rs index d0186a0fa..718103458 100644 --- a/identity/aziot-identityd/src/http/mod.rs +++ b/identity/aziot-identityd/src/http/mod.rs @@ -7,12 +7,13 @@ mod get_trust_bundle; mod get_update_or_delete_module_identity; mod reprovision_device; -pub(crate) struct Server { - pub(crate) inner: std::sync::Arc>, +#[derive(Clone)] +pub struct Service { + pub(crate) api: std::sync::Arc>, } -http_common::make_server! { - server: Server, +http_common::make_service! { + service: Service, api_version: aziot_identity_common_http::ApiVersion, routes: [ create_or_list_module_identity::Route, @@ -24,39 +25,49 @@ http_common::make_server! { ], } -fn to_http_error(err: &aziot_identityd::Error) -> http_common::server::Error { +fn to_http_error(err: &crate::Error) -> http_common::server::Error { let error_message = http_common::server::error_to_message(err); // TODO: When we get distributed tracing, associate these logs with the tracing ID. for line in error_message.split('\n') { - eprintln!("!!! {}", line); + log::log!( + if matches!(err, crate::Error::Internal(_)) { + log::Level::Info + } else { + log::Level::Error + }, + "!!! {}", + line, + ); } match err { // Do not use error_message because we don't want to leak internal errors to the client. // Just return the top-level error, ie "internal error" - aziot_identityd::Error::Internal(_) => http_common::server::Error { + crate::Error::Internal(_) => http_common::server::Error { status_code: hyper::StatusCode::INTERNAL_SERVER_ERROR, message: err.to_string().into(), }, - aziot_identityd::error::Error::InvalidParameter(_, _) - | aziot_identityd::error::Error::DeviceNotFound - | aziot_identityd::error::Error::ModuleNotFound => http_common::server::Error { + crate::error::Error::InvalidParameter(_, _) + | crate::error::Error::DeviceNotFound + | crate::error::Error::ModuleNotFound => http_common::server::Error { status_code: hyper::StatusCode::BAD_REQUEST, message: error_message.into(), }, - aziot_identityd::error::Error::DPSClient(_) - | aziot_identityd::error::Error::HubClient(_) => http_common::server::Error { - status_code: hyper::StatusCode::NOT_FOUND, - message: error_message.into(), - }, + crate::error::Error::DPSClient(_) | crate::error::Error::HubClient(_) => { + http_common::server::Error { + status_code: hyper::StatusCode::NOT_FOUND, + message: error_message.into(), + } + } - aziot_identityd::error::Error::Authentication - | aziot_identityd::error::Error::Authorization => http_common::server::Error { - status_code: hyper::StatusCode::UNAUTHORIZED, - message: error_message.into(), - }, + crate::error::Error::Authentication | crate::error::Error::Authorization => { + http_common::server::Error { + status_code: hyper::StatusCode::UNAUTHORIZED, + message: error_message.into(), + } + } } } diff --git a/identity/aziot-identityd/src/http/reprovision_device.rs b/identity/aziot-identityd/src/http/reprovision_device.rs index da1b241ac..9b0a08d0c 100644 --- a/identity/aziot-identityd/src/http/reprovision_device.rs +++ b/identity/aziot-identityd/src/http/reprovision_device.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_identity_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -37,17 +37,17 @@ impl http_common::server::Route for Route { self, _body: Option, ) -> http_common::server::RouteResponse> { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let user = aziot_identityd::auth::Uid(0); - let auth_id = match inner.authenticator.authenticate(user) { + let user = crate::auth::Uid(0); + let auth_id = match api.authenticator.authenticate(user) { Ok(auth_id) => auth_id, Err(err) => return Err(super::to_http_error(&err)), }; //TODO: get uid from UDS - match inner.reprovision_device(auth_id).await { + match api.reprovision_device(auth_id).await { Ok(()) => (), Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/identity/aziot-identityd/src/lib.rs b/identity/aziot-identityd/src/lib.rs index 52e71682c..cecd78458 100644 --- a/identity/aziot-identityd/src/lib.rs +++ b/identity/aziot-identityd/src/lib.rs @@ -4,20 +4,20 @@ #![deny(clippy::all, clippy::pedantic)] #![allow( clippy::default_trait_access, + clippy::let_and_return, + clippy::let_unit_value, clippy::missing_errors_doc, clippy::module_name_repetitions, clippy::must_use_candidate, clippy::too_many_lines, - clippy::let_and_return, clippy::type_complexity )] #![allow(dead_code)] -pub mod app; pub mod auth; pub mod error; +mod http; pub mod identity; -mod logging; pub mod settings; pub use error::{Error, InternalError}; @@ -28,7 +28,106 @@ const DPS_ASSIGNMENT_RETRY_INTERVAL_SECS: u64 = 10; /// This is the number of seconds to wait for DPS to complete assignment to a hub const DPS_ASSIGNMENT_TIMEOUT_SECS: u64 = 120; -pub struct Server { +pub async fn main( + settings: settings::Settings, +) -> Result<(http_common::Connector, http::Service), Box> { + // Written to prev_settings_path if provisioning is successful. + let settings_serialized = toml::to_vec(&settings).expect("serializing settings cannot fail"); + + let homedir_path = &settings.homedir; + let connector = settings.endpoints.aziot_identityd.clone(); + + let mut prev_settings_path = homedir_path.clone(); + prev_settings_path.push("prev_state"); + + let mut prev_device_info_path = homedir_path.clone(); + prev_device_info_path.push("device_info"); + + if !homedir_path.exists() { + let () = + std::fs::create_dir_all(&homedir_path).map_err(error::InternalError::CreateHomeDir)?; + } + + let (_pmap, hub_mset, local_mmap) = convert_to_map(&settings.principal); + + let authenticator = Box::new(|_| Ok(auth::AuthId::Unknown)); + + // TODO: Enable SettingsAuthorizer once Unix Domain Sockets is ported over. + // let authorizer = SettingsAuthorizer { pmap }; + // let authorizer = Box::new(authorizer); + let authorizer = Box::new(|_| Ok(true)); + + let api = Api::new(settings, authenticator, authorizer)?; + let api = std::sync::Arc::new(futures_util::lock::Mutex::new(api)); + + { + let (mut prev_hub_mset, prev_local_mmap) = if prev_settings_path.exists() { + let prev_settings = settings::Settings::new(&prev_settings_path)?; + let (_, h, l) = convert_to_map(&prev_settings.principal); + (h, l) + } else { + ( + std::collections::BTreeSet::default(), + std::collections::BTreeMap::default(), + ) + }; + + let mut api_ = api.lock().await; + + log::info!("Provisioning starting."); + let provisioning = api_.provision_device().await?; + log::info!("Provisioning complete."); + + let device_status = if let aziot_identity_common::ProvisioningStatus::Provisioned(device) = + provisioning + { + let curr_hub_device_info = settings::HubDeviceInfo { + hub_name: device.iothub_hostname, + device_id: device.device_id, + }; + + // Only consider the previous Hub modules if the current and previous Hub devices match. + let mut use_prev = false; + + if prev_device_info_path.exists() { + let prev_hub_device_info = settings::HubDeviceInfo::new(&prev_device_info_path)?; + + if let Some(prev_info) = prev_hub_device_info { + if prev_info == curr_hub_device_info { + use_prev = true; + } + } + } + + if !use_prev { + prev_hub_mset = std::collections::BTreeSet::default(); + } + + let () = api_.init_hub_identities(prev_hub_mset, hub_mset).await?; + log::info!("Identity reconciliation with IoT Hub complete."); + + toml::to_string(&curr_hub_device_info)? + } else { + settings::HubDeviceInfo::unprovisioned() + }; + + let () = api_ + .init_local_identities(prev_local_mmap, local_mmap) + .await?; + log::info!("Local identity reconciliation complete."); + + std::fs::write(prev_device_info_path, device_status) + .map_err(error::InternalError::SaveDeviceInfo)?; + let () = std::fs::write(prev_settings_path, &settings_serialized) + .map_err(error::InternalError::SaveSettings)?; + } + + let service = http::Service { api }; + + Ok((connector, service)) +} + +pub struct Api { pub settings: settings::Settings, pub authenticator: Box + Send + Sync>, pub authorizer: Box + Send + Sync>, @@ -39,7 +138,7 @@ pub struct Server { cert_client: std::sync::Arc, } -impl Server { +impl Api { pub fn new( settings: settings::Settings, authenticator: Box + Send + Sync>, @@ -85,7 +184,7 @@ impl Server { None, ); - Ok(Server { + Ok(Api { settings, authenticator, authorizer, @@ -791,15 +890,58 @@ impl auth::authorization::Authorizer for SettingsAuthorizer { } } +fn convert_to_map( + principal: &[settings::Principal], +) -> ( + std::collections::BTreeMap, + std::collections::BTreeSet, + std::collections::BTreeMap>, +) { + let mut local_mmap: std::collections::BTreeMap< + aziot_identity_common::ModuleId, + Option, + > = std::collections::BTreeMap::new(); + let mut module_mset: std::collections::BTreeSet = + std::collections::BTreeSet::new(); + let mut pmap: std::collections::BTreeMap = + std::collections::BTreeMap::new(); + + for p in principal { + if let Some(id_type) = &p.id_type { + for i in id_type { + match i { + aziot_identity_common::IdType::Module => module_mset.insert(p.name.clone()), + aziot_identity_common::IdType::Local => local_mmap + .insert(p.name.clone(), p.localid.clone()) + .is_none(), + _ => true, + }; + } + } + + pmap.insert(p.uid, p.clone()); + } + + (pmap, module_mset, local_mmap) +} + #[cfg(test)] mod tests { - use super::Server; - use crate::{ - auth::AuthId, - settings::{Endpoints, ManualAuthMethod, Provisioning, ProvisioningType, Settings}, - }; - use http_common::Connector; use std::collections::BTreeSet; + use std::path::Path; + + use aziot_identity_common::{IdType, LocalIdAttr, ModuleId}; + use http_common::Connector; + + use crate::auth::authorization::Authorizer; + use crate::auth::{AuthId, Operation, OperationType, Uid}; + use crate::settings::{ + Endpoints, LocalId, LocalIdOpts, ManualAuthMethod, Principal, Provisioning, + ProvisioningType, Settings, + }; + use crate::SettingsAuthorizer; + + use super::{convert_to_map, Api}; fn make_empty_settings() -> Settings { Settings { @@ -837,16 +979,149 @@ mod tests { #[tokio::test] async fn init_identities_with_empty_args_exits_early() { - let server = Server::new( + let api = Api::new( make_empty_settings(), Box::new(|_| Ok(AuthId::Unknown)), Box::new(|_| Ok(true)), ) .unwrap(); - let result = server + let result = api .init_hub_identities(BTreeSet::new(), BTreeSet::new()) .await; result.unwrap(); } + + #[test] + fn convert_to_map_creates_principal_lookup() { + let local_p: Principal = Principal { + uid: Uid(1000), + name: ModuleId(String::from("local1")), + id_type: Some(vec![IdType::Local]), + localid: None, + }; + let module_p: Principal = Principal { + uid: Uid(1001), + name: ModuleId(String::from("module1")), + id_type: Some(vec![IdType::Module]), + localid: None, + }; + let v = vec![module_p.clone(), local_p.clone()]; + let (map, _, _) = convert_to_map(&v); + + assert!(map.contains_key(&Uid(1000))); + assert_eq!(map.get(&Uid(1000)).unwrap(), &local_p); + assert!(map.contains_key(&Uid(1001))); + assert_eq!(map.get(&Uid(1001)).unwrap(), &module_p); + } + + #[test] + fn convert_to_map_module_sets() { + let v = vec![ + Principal { + uid: Uid(1000), + name: ModuleId("hubmodule".to_owned()), + id_type: Some(vec![IdType::Module]), + localid: None, + }, + Principal { + uid: Uid(1001), + name: ModuleId("localmodule".to_owned()), + id_type: Some(vec![IdType::Local]), + localid: None, + }, + Principal { + uid: Uid(1002), + name: ModuleId("globalmodule".to_owned()), + id_type: Some(vec![IdType::Module, IdType::Local]), + localid: None, + }, + ]; + + let (_, hub_modules, local_modules) = convert_to_map(&v); + + assert!(hub_modules.contains(&ModuleId("hubmodule".to_owned()))); + assert!(hub_modules.contains(&ModuleId("globalmodule".to_owned()))); + assert!(!hub_modules.contains(&ModuleId("localmodule".to_owned()))); + + assert!(local_modules.contains_key(&ModuleId("localmodule".to_owned()))); + assert!(local_modules.contains_key(&ModuleId("globalmodule".to_owned()))); + assert!(!local_modules.contains_key(&ModuleId("hubmodule".to_owned()))); + } + + #[test] + fn settings_test() { + let settings = Settings::new(Path::new("test/good_auth_settings.toml")).unwrap(); + + let localid = settings.localid.unwrap(); + assert_eq!( + localid, + LocalId { + domain: "example.com".to_owned(), + } + ); + + let (map, _, _) = convert_to_map(&settings.principal); + assert_eq!(map.len(), 3); + assert!(map.contains_key(&Uid(1003))); + assert_eq!(map.get(&Uid(1003)).unwrap().uid, Uid(1003)); + assert_eq!( + map.get(&Uid(1003)).unwrap().name, + ModuleId(String::from("hostprocess2")) + ); + assert_eq!( + map.get(&Uid(1003)).unwrap().id_type, + Some(vec![IdType::Module, IdType::Local]) + ); + } + + #[test] + fn local_id_opts() { + let s = Settings::new(std::path::Path::new("test/good_local_opts.toml")).unwrap(); + + assert_eq!( + &s.principal, + &[ + Principal { + uid: Uid(1000), + name: ModuleId("module1".to_owned()), + id_type: Some(vec![IdType::Local]), + localid: None, + }, + Principal { + uid: Uid(1001), + name: ModuleId("module2".to_owned()), + id_type: Some(vec![IdType::Local]), + localid: Some(LocalIdOpts::X509 { + attributes: LocalIdAttr::default() + }), + }, + Principal { + uid: Uid(1002), + name: ModuleId("module3".to_owned()), + id_type: Some(vec![IdType::Local]), + localid: Some(LocalIdOpts::X509 { + attributes: LocalIdAttr::Server + }), + }, + ] + ); + } + + #[test] + fn empty_auth_settings_deny_any_action() { + let (pmap, mset, _) = convert_to_map(&[]); + let auth = SettingsAuthorizer { pmap, mset }; + let operation = Operation { + auth_id: AuthId::Unknown, + op_type: OperationType::CreateModule(String::default()), + }; + + let res = auth.authorize(operation); + + match res { + Ok(false) => (), + _ => panic!("incorrect authorization returned"), + } + } } diff --git a/identity/aziot-identityd/src/logging.rs b/identity/aziot-identityd/src/logging.rs deleted file mode 100644 index 525616192..000000000 --- a/identity/aziot-identityd/src/logging.rs +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -use std::env; -use std::io::Write; - -use log::{Level, LevelFilter}; - -const ENV_LOG: &str = "AZIOT_LOG"; - -pub fn init() { - env_logger::Builder::new() - .format(|fmt, record| { - let level = match record.level() { - Level::Trace => "TRCE", - Level::Debug => "DBUG", - Level::Info => "INFO", - Level::Warn => "WARN", - Level::Error => "ERR!", - }; - let timestamp = fmt.timestamp(); - - if record.level() >= Level::Debug { - writeln!( - fmt, - "<{}>{} [{}] - [{}] {}", - syslog_level(record.level()), - timestamp, - level, - record.target(), - record.args() - ) - } else { - writeln!( - fmt, - "<{}>{} [{}] - {}", - syslog_level(record.level()), - timestamp, - level, - record.args() - ) - } - }) - .filter_level(LevelFilter::Info) - .parse(&env::var(ENV_LOG).unwrap_or_default()) - .init(); -} - -fn syslog_level(level: Level) -> i8 { - match level { - Level::Error => 3, - Level::Warn => 4, - Level::Info => 6, - Level::Debug | Level::Trace => 7, - } -} diff --git a/identity/aziot-identityd/src/main.rs b/identity/aziot-identityd/src/main.rs deleted file mode 100644 index 80b33c010..000000000 --- a/identity/aziot-identityd/src/main.rs +++ /dev/null @@ -1,305 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -#![deny(rust_2018_idioms, warnings)] -#![deny(clippy::all, clippy::pedantic)] -#![allow(clippy::let_unit_value, clippy::type_complexity)] - -mod http; - -#[tokio::main] -async fn main() -> Result<(), Box> { - run().await?; - - Ok(()) -} - -async fn run() -> Result<(), Box> { - let config_file = aziot_identityd::app::init()?; - - let settings = aziot_identityd::settings::Settings::new(&config_file)?; - let homedir_path = &settings.homedir.to_owned(); - let connector = settings.endpoints.aziot_identityd.clone(); - - let mut prev_settings_path = homedir_path.clone(); - prev_settings_path.push("prev_state"); - - let mut prev_device_info_path = homedir_path.clone(); - prev_device_info_path.push("device_info"); - - if !homedir_path.exists() { - let () = std::fs::create_dir_all(&homedir_path) - .map_err(aziot_identityd::error::InternalError::CreateHomeDir)?; - } - - let (_pmap, hub_mset, local_mmap) = convert_to_map(&settings.principal); - - let authenticator = Box::new(|_| Ok(aziot_identityd::auth::AuthId::Unknown)); - - // TODO: Enable SettingsAuthorizer once Unix Domain Sockets is ported over. - // let authorizer = aziot_identityd::SettingsAuthorizer { pmap }; - // let authorizer = Box::new(authorizer); - let authorizer = Box::new(|_| Ok(true)); - - let server = aziot_identityd::Server::new(settings, authenticator, authorizer)?; - let server = std::sync::Arc::new(futures_util::lock::Mutex::new(server)); - { - let (mut prev_hub_mset, prev_local_mmap) = if prev_settings_path.exists() { - let prev_settings = aziot_identityd::settings::Settings::new(&prev_settings_path)?; - let (_, h, l) = convert_to_map(&prev_settings.principal); - (h, l) - } else { - ( - std::collections::BTreeSet::default(), - std::collections::BTreeMap::default(), - ) - }; - - let mut server_ = server.lock().await; - - log::info!("Provisioning starting."); - let provisioning = server_.provision_device().await?; - log::info!("Provisioning complete."); - - let device_status = - if let aziot_identity_common::ProvisioningStatus::Provisioned(device) = provisioning { - let curr_hub_device_info = aziot_identityd::settings::HubDeviceInfo { - hub_name: device.iothub_hostname, - device_id: device.device_id, - }; - - // Only consider the previous Hub modules if the current and previous Hub devices match. - let mut use_prev = false; - - if prev_device_info_path.exists() { - let prev_hub_device_info = - aziot_identityd::settings::HubDeviceInfo::new(&prev_device_info_path)?; - - if let Some(prev_info) = prev_hub_device_info { - if prev_info == curr_hub_device_info { - use_prev = true; - } - } - } - - if !use_prev { - prev_hub_mset = std::collections::BTreeSet::default(); - } - - let () = server_.init_hub_identities(prev_hub_mset, hub_mset).await?; - log::info!("Identity reconciliation with IoT Hub complete."); - - toml::to_string(&curr_hub_device_info)? - } else { - aziot_identityd::settings::HubDeviceInfo::unprovisioned() - }; - - let () = server_ - .init_local_identities(prev_local_mmap, local_mmap) - .await?; - log::info!("Local identity reconciliation complete."); - - std::fs::write(prev_device_info_path, device_status) - .map_err(aziot_identityd::error::InternalError::SaveDeviceInfo)?; - std::fs::copy(config_file, prev_settings_path) - .map_err(aziot_identityd::error::InternalError::SaveSettings)?; - } - - let incoming = connector.incoming().await?; - log::info!("Identity Service started."); - - let server = hyper::Server::builder(incoming).serve(hyper::service::make_service_fn(|_| { - let server = http::Server { - inner: server.clone(), - }; - futures_util::future::ok::<_, std::convert::Infallible>(server) - })); - let () = server.await?; - - log::info!("Identity Service stopped."); - - Ok(()) -} - -fn convert_to_map( - principal: &[aziot_identityd::settings::Principal], -) -> ( - std::collections::BTreeMap, - std::collections::BTreeSet, - std::collections::BTreeMap< - aziot_identity_common::ModuleId, - Option, - >, -) { - let mut local_mmap: std::collections::BTreeMap< - aziot_identity_common::ModuleId, - Option, - > = std::collections::BTreeMap::new(); - let mut module_mset: std::collections::BTreeSet = - std::collections::BTreeSet::new(); - let mut pmap: std::collections::BTreeMap< - aziot_identityd::auth::Uid, - aziot_identityd::settings::Principal, - > = std::collections::BTreeMap::new(); - - for p in principal { - if let Some(id_type) = &p.id_type { - for i in id_type { - match i { - aziot_identity_common::IdType::Module => module_mset.insert(p.name.clone()), - aziot_identity_common::IdType::Local => local_mmap - .insert(p.name.clone(), p.localid.clone()) - .is_none(), - _ => true, - }; - } - } - - pmap.insert(p.uid, p.clone()); - } - - (pmap, module_mset, local_mmap) -} - -#[cfg(test)] -mod tests { - use crate::convert_to_map; - use aziot_identity_common::{IdType, LocalIdAttr, ModuleId}; - use aziot_identityd::auth::authorization::Authorizer; - use aziot_identityd::auth::{AuthId, Operation, OperationType, Uid}; - use aziot_identityd::settings::{LocalId, LocalIdOpts, Principal, Settings}; - use aziot_identityd::SettingsAuthorizer; - use std::path::Path; - - #[test] - fn convert_to_map_creates_principal_lookup() { - let local_p: Principal = Principal { - uid: Uid(1000), - name: ModuleId(String::from("local1")), - id_type: Some(vec![IdType::Local]), - localid: None, - }; - let module_p: Principal = Principal { - uid: Uid(1001), - name: ModuleId(String::from("module1")), - id_type: Some(vec![IdType::Module]), - localid: None, - }; - let v = vec![module_p.clone(), local_p.clone()]; - let (map, _, _) = convert_to_map(&v); - - assert!(map.contains_key(&Uid(1000))); - assert_eq!(map.get(&Uid(1000)).unwrap(), &local_p); - assert!(map.contains_key(&Uid(1001))); - assert_eq!(map.get(&Uid(1001)).unwrap(), &module_p); - } - - #[test] - fn convert_to_map_module_sets() { - let v = vec![ - Principal { - uid: Uid(1000), - name: ModuleId("hubmodule".to_owned()), - id_type: Some(vec![IdType::Module]), - localid: None, - }, - Principal { - uid: Uid(1001), - name: ModuleId("localmodule".to_owned()), - id_type: Some(vec![IdType::Local]), - localid: None, - }, - Principal { - uid: Uid(1002), - name: ModuleId("globalmodule".to_owned()), - id_type: Some(vec![IdType::Module, IdType::Local]), - localid: None, - }, - ]; - - let (_, hub_modules, local_modules) = convert_to_map(&v); - - assert!(hub_modules.contains(&ModuleId("hubmodule".to_owned()))); - assert!(hub_modules.contains(&ModuleId("globalmodule".to_owned()))); - assert!(!hub_modules.contains(&ModuleId("localmodule".to_owned()))); - - assert!(local_modules.contains_key(&ModuleId("localmodule".to_owned()))); - assert!(local_modules.contains_key(&ModuleId("globalmodule".to_owned()))); - assert!(!local_modules.contains_key(&ModuleId("hubmodule".to_owned()))); - } - - #[test] - fn settings_test() { - let settings = Settings::new(Path::new("test/good_auth_settings.toml")).unwrap(); - - let localid = settings.localid.unwrap(); - assert_eq!( - localid, - LocalId { - domain: "example.com".to_owned(), - } - ); - - let (map, _, _) = convert_to_map(&settings.principal); - assert_eq!(map.len(), 3); - assert!(map.contains_key(&Uid(1003))); - assert_eq!(map.get(&Uid(1003)).unwrap().uid, Uid(1003)); - assert_eq!( - map.get(&Uid(1003)).unwrap().name, - ModuleId(String::from("hostprocess2")) - ); - assert_eq!( - map.get(&Uid(1003)).unwrap().id_type, - Some(vec![IdType::Module, IdType::Local]) - ); - } - - #[test] - fn local_id_opts() { - let s = Settings::new(std::path::Path::new("test/good_local_opts.toml")).unwrap(); - - assert_eq!( - &s.principal, - &[ - Principal { - uid: Uid(1000), - name: ModuleId("module1".to_owned()), - id_type: Some(vec![IdType::Local]), - localid: None, - }, - Principal { - uid: Uid(1001), - name: ModuleId("module2".to_owned()), - id_type: Some(vec![IdType::Local]), - localid: Some(LocalIdOpts::X509 { - attributes: LocalIdAttr::default() - }), - }, - Principal { - uid: Uid(1002), - name: ModuleId("module3".to_owned()), - id_type: Some(vec![IdType::Local]), - localid: Some(LocalIdOpts::X509 { - attributes: LocalIdAttr::Server - }), - }, - ] - ); - } - - #[test] - fn empty_auth_settings_deny_any_action() { - let (pmap, mset, _) = convert_to_map(&[]); - let auth = SettingsAuthorizer { pmap, mset }; - let operation = Operation { - auth_id: AuthId::Unknown, - op_type: OperationType::CreateModule(String::default()), - }; - - let res = auth.authorize(operation); - - match res { - Ok(false) => (), - _ => panic!("incorrect authorization returned"), - } - } -} diff --git a/key/aziot-key-openssl-engine/Cargo.toml b/key/aziot-key-openssl-engine/Cargo.toml index 776d86812..37f6df092 100644 --- a/key/aziot-key-openssl-engine/Cargo.toml +++ b/key/aziot-key-openssl-engine/Cargo.toml @@ -10,6 +10,7 @@ build = "build/main.rs" [dependencies] base64 = "0.12" foreign-types-shared = "0.1" +log = "0.4" openssl = "0.10" openssl-errors = "0.1" openssl-sys = "0.9" diff --git a/key/aziot-key-openssl-engine/src/lib.rs b/key/aziot-key-openssl-engine/src/lib.rs index 87ee2a6bc..bd40876aa 100644 --- a/key/aziot-key-openssl-engine/src/lib.rs +++ b/key/aziot-key-openssl-engine/src/lib.rs @@ -102,7 +102,7 @@ fn r#catch( if let Some(function) = function { openssl_errors::put_error!(function(), Error::MESSAGE, "{}", err); } else { - eprintln!("[aziot-key-openssl-engine] error: {}", err); + log::error!("[aziot-key-openssl-engine] error: {}", err); } let mut source = err.source(); @@ -110,7 +110,7 @@ fn r#catch( if let Some(function) = function { openssl_errors::put_error!(function(), Error::MESSAGE, "{}", err); } else { - eprintln!("[aziot-key-openssl-engine] caused by: {}", err); + log::error!("[aziot-key-openssl-engine] caused by: {}", err); } source = err.source(); diff --git a/key/aziot-keyd/Cargo.toml b/key/aziot-keyd/Cargo.toml index c4d9d056d..361edc9d6 100644 --- a/key/aziot-keyd/Cargo.toml +++ b/key/aziot-keyd/Cargo.toml @@ -4,25 +4,29 @@ version = "0.1.0" authors = ["Azure IoT Edge Devs"] edition = "2018" + [dependencies] async-trait = "0.1" -backtrace = "0.3" base64 = "0.12" futures-util = "0.3" http = "0.2" hyper = "0.13" lazy_static = "1" +log = "0.4" openssl = "0.10" openssl-sys = "0.9" percent-encoding = "2" regex = "1" serde = { version = "1", features = ["derive"] } serde_json = "1" -tokio = { version = "0.2", features = ["macros"] } -toml = "0.5" +tokio = "0.2" url = "2" aziot-key-common = { path = "../aziot-key-common" } aziot-key-common-http = { path = "../aziot-key-common-http" } -aziot-keys = { path = "../aziot-keys" } +aziot-keys-common = { path = "../aziot-keys-common" } http-common = { path = "../../http-common" } + + +[dev-dependencies] +toml = "0.5" diff --git a/key/aziot-keyd/src/http/create_derived_key.rs b/key/aziot-keyd/src/http/create_derived_key.rs index 500be4535..49b2e8060 100644 --- a/key/aziot-keyd/src/http/create_derived_key.rs +++ b/key/aziot-keyd/src/http/create_derived_key.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -42,10 +42,10 @@ impl http_common::server::Route for Route { message: "missing request body".into(), })?; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let handle = match inner.create_derived_key(&body.base_handle, &body.derivation_data.0) { + let handle = match api.create_derived_key(&body.base_handle, &body.derivation_data.0) { Ok(handle) => handle, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/key/aziot-keyd/src/http/create_key_if_not_exists.rs b/key/aziot-keyd/src/http/create_key_if_not_exists.rs index 22c348c35..3d834db90 100644 --- a/key/aziot-keyd/src/http/create_key_if_not_exists.rs +++ b/key/aziot-keyd/src/http/create_key_if_not_exists.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -69,10 +69,10 @@ impl http_common::server::Route for Route { } }; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let handle = match inner.create_key_if_not_exists(&body.id, create_key_value) { + let handle = match api.create_key_if_not_exists(&body.id, create_key_value) { Ok(handle) => handle, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/key/aziot-keyd/src/http/create_key_pair_if_not_exists.rs b/key/aziot-keyd/src/http/create_key_pair_if_not_exists.rs index 0e7082fa6..d1e37ede2 100644 --- a/key/aziot-keyd/src/http/create_key_pair_if_not_exists.rs +++ b/key/aziot-keyd/src/http/create_key_pair_if_not_exists.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -42,10 +42,10 @@ impl http_common::server::Route for Route { message: "missing request body".into(), })?; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let handle = match inner + let handle = match api .create_key_pair_if_not_exists(&body.id, body.preferred_algorithms.as_deref()) { Ok(handle) => handle, diff --git a/key/aziot-keyd/src/http/decrypt.rs b/key/aziot-keyd/src/http/decrypt.rs index 6c27f7f5a..9de75e9d6 100644 --- a/key/aziot-keyd/src/http/decrypt.rs +++ b/key/aziot-keyd/src/http/decrypt.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -59,10 +59,10 @@ impl http_common::server::Route for Route { } }; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let plaintext = match inner.decrypt(&body.key_handle, mechanism, &body.ciphertext.0) { + let plaintext = match api.decrypt(&body.key_handle, mechanism, &body.ciphertext.0) { Ok(plaintext) => plaintext, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/key/aziot-keyd/src/http/encrypt.rs b/key/aziot-keyd/src/http/encrypt.rs index e93a327c9..6deaadca9 100644 --- a/key/aziot-keyd/src/http/encrypt.rs +++ b/key/aziot-keyd/src/http/encrypt.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -59,10 +59,10 @@ impl http_common::server::Route for Route { } }; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let ciphertext = match inner.encrypt(&body.key_handle, mechanism, &body.plaintext.0) { + let ciphertext = match api.encrypt(&body.key_handle, mechanism, &body.plaintext.0) { Ok(ciphertext) => ciphertext, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/key/aziot-keyd/src/http/export_derived_key.rs b/key/aziot-keyd/src/http/export_derived_key.rs index 0af079905..6c068651b 100644 --- a/key/aziot-keyd/src/http/export_derived_key.rs +++ b/key/aziot-keyd/src/http/export_derived_key.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -42,10 +42,10 @@ impl http_common::server::Route for Route { message: "missing request body".into(), })?; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let derived_key = match inner.export_derived_key(&body.handle) { + let derived_key = match api.export_derived_key(&body.handle) { Ok(derived_key) => derived_key, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/key/aziot-keyd/src/http/get_key_pair_public_parameter.rs b/key/aziot-keyd/src/http/get_key_pair_public_parameter.rs index 35d4904af..daf831548 100644 --- a/key/aziot-keyd/src/http/get_key_pair_public_parameter.rs +++ b/key/aziot-keyd/src/http/get_key_pair_public_parameter.rs @@ -7,7 +7,7 @@ lazy_static::lazy_static! { } pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, parameter_name: String, } @@ -18,9 +18,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -32,7 +32,7 @@ impl http_common::server::Route for Route { .ok()?; Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), parameter_name: parameter_name.into_owned(), }) } @@ -53,11 +53,11 @@ impl http_common::server::Route for Route { message: "missing request body".into(), })?; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; let parameter_value = - match inner.get_key_pair_public_parameter(&body.key_handle, &self.parameter_name) { + match api.get_key_pair_public_parameter(&body.key_handle, &self.parameter_name) { Ok(parameter_value) => parameter_value, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/key/aziot-keyd/src/http/load.rs b/key/aziot-keyd/src/http/load.rs index 07e02f1c6..b8994c80e 100644 --- a/key/aziot-keyd/src/http/load.rs +++ b/key/aziot-keyd/src/http/load.rs @@ -7,7 +7,7 @@ lazy_static::lazy_static! { } pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, type_: String, key_id: String, } @@ -19,9 +19,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -38,7 +38,7 @@ impl http_common::server::Route for Route { .ok()?; Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), type_: type_.into_owned(), key_id: key_id.into_owned(), }) @@ -49,15 +49,15 @@ impl http_common::server::Route for Route { type GetResponse = aziot_key_common_http::load::Response; async fn get(self) -> http_common::server::RouteResponse { - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; let handle = match &*self.type_ { - "keypair" => match inner.load_key_pair(&self.key_id) { + "keypair" => match api.load_key_pair(&self.key_id) { Ok(handle) => handle, Err(err) => return Err(super::to_http_error(&err)), }, - "key" => match inner.load_key(&self.key_id) { + "key" => match api.load_key(&self.key_id) { Ok(handle) => handle, Err(err) => return Err(super::to_http_error(&err)), }, diff --git a/key/aziot-keyd/src/http/mod.rs b/key/aziot-keyd/src/http/mod.rs index a69dce01e..28a42949d 100644 --- a/key/aziot-keyd/src/http/mod.rs +++ b/key/aziot-keyd/src/http/mod.rs @@ -10,12 +10,13 @@ mod get_key_pair_public_parameter; mod load; mod sign; -pub(crate) struct Server { - pub(crate) inner: std::sync::Arc>, +#[derive(Clone)] +pub struct Service { + pub(crate) api: std::sync::Arc>, } -http_common::make_server! { - server: Server, +http_common::make_service! { + service: Service, api_version: aziot_key_common_http::ApiVersion, routes: [ create_derived_key::Route, @@ -30,23 +31,31 @@ http_common::make_server! { ], } -fn to_http_error(err: &aziot_keyd::Error) -> http_common::server::Error { +fn to_http_error(err: &crate::Error) -> http_common::server::Error { let error_message = http_common::server::error_to_message(err); // TODO: When we get distributed tracing, associate these logs with the tracing ID. for line in error_message.split('\n') { - eprintln!("!!! {}", line); + log::log!( + if matches!(err, crate::Error::Internal(_)) { + log::Level::Info + } else { + log::Level::Error + }, + "!!! {}", + line, + ); } match err { // Do not use error_message because we don't want to leak internal errors to the client. // Just return the top-level error, ie "internal error" - aziot_keyd::Error::Internal(_) => http_common::server::Error { + crate::Error::Internal(_) => http_common::server::Error { status_code: hyper::StatusCode::INTERNAL_SERVER_ERROR, message: err.to_string().into(), }, - aziot_keyd::Error::InvalidParameter(_) => http_common::server::Error { + crate::Error::InvalidParameter(_) => http_common::server::Error { status_code: hyper::StatusCode::BAD_REQUEST, message: error_message.into(), }, diff --git a/key/aziot-keyd/src/http/sign.rs b/key/aziot-keyd/src/http/sign.rs index 62facb7b6..fdef0fa81 100644 --- a/key/aziot-keyd/src/http/sign.rs +++ b/key/aziot-keyd/src/http/sign.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. pub(super) struct Route { - inner: std::sync::Arc>, + api: std::sync::Arc>, } #[async_trait::async_trait] @@ -11,9 +11,9 @@ impl http_common::server::Route for Route { &((aziot_key_common_http::ApiVersion::V2020_09_01)..) } - type Server = super::Server; + type Service = super::Service; fn from_uri( - server: &Self::Server, + service: &Self::Service, path: &str, _query: &[(std::borrow::Cow<'_, str>, std::borrow::Cow<'_, str>)], ) -> Option { @@ -22,7 +22,7 @@ impl http_common::server::Route for Route { } Some(Route { - inner: server.inner.clone(), + api: service.api.clone(), }) } @@ -52,10 +52,10 @@ impl http_common::server::Route for Route { } }; - let mut inner = self.inner.lock().await; - let inner = &mut *inner; + let mut api = self.api.lock().await; + let api = &mut *api; - let signature = match inner.sign(&body.key_handle, mechanism, &digest.0) { + let signature = match api.sign(&body.key_handle, mechanism, &digest.0) { Ok(signature) => signature, Err(err) => return Err(super::to_http_error(&err)), }; diff --git a/key/aziot-keyd/src/keys.rs b/key/aziot-keyd/src/keys.rs index c12d86859..1004fd5f5 100644 --- a/key/aziot-keyd/src/keys.rs +++ b/key/aziot-keyd/src/keys.rs @@ -176,9 +176,10 @@ impl Keys { .ok_or(LoadLibraryError::MissingFunction("decrypt"))?, }; - println!( + log::info!( "Loaded libaziot-keys with version 0x{:08x}, {:?}", - api_version, result + api_version, + result, ); Ok(result) diff --git a/key/aziot-keyd/src/lib.rs b/key/aziot-keyd/src/lib.rs index 388c472f8..c6ab1332e 100644 --- a/key/aziot-keyd/src/lib.rs +++ b/key/aziot-keyd/src/lib.rs @@ -5,6 +5,7 @@ #![allow( clippy::default_trait_access, clippy::let_and_return, + clippy::let_unit_value, clippy::missing_errors_doc )] @@ -16,11 +17,73 @@ pub use error::{Error, InternalError}; mod keys; -pub struct Server { +mod http; + +pub async fn main( + config: Config, +) -> Result<(http_common::Connector, http::Service), Box> { + let Config { + aziot_keys, + preloaded_keys, + endpoints: Endpoints { + aziot_keyd: connector, + }, + } = config; + + let api = { + let mut keys = keys::Keys::new()?; + + for (name, value) in aziot_keys { + let name = std::ffi::CString::new(name.clone()).map_err(|err| { + Error::Internal(InternalError::ReadConfig( + format!( + "key {:?} in [aziot_keys] section of the configuration could not be converted to a C string: {}", + name, err, + ) + .into(), + )) + })?; + + let value = + std::ffi::CString::new(value).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( + "value of key {:?} in [aziot_keys] section of the configuration could not be converted to a C string: {}", + name, err, + ).into())))?; + + keys.set_parameter(&name, &value)?; + } + + for (key_id, value) in preloaded_keys { + let name = format!("preloaded_key:{}", key_id); + let name = + std::ffi::CString::new(name).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( + "key ID {:?} in [preloaded_keys] section of the configuration could not be converted to a C string: {}", + key_id, err, + ).into())))?; + + let value = + std::ffi::CString::new(value).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( + "location of key ID {:?} in [preloaded_keys] section of the configuration could not be converted to a C string: {}", + key_id, err, + ).into())))?; + + keys.set_parameter(&name, &value)?; + } + + Api { keys } + }; + let api = std::sync::Arc::new(futures_util::lock::Mutex::new(api)); + + let service = http::Service { api }; + + Ok((connector, service)) +} + +pub struct Api { keys: keys::Keys, } -impl Server { +impl Api { pub fn new( aziot_keys: std::collections::BTreeMap, preloaded_keys: std::collections::BTreeMap, @@ -31,18 +94,18 @@ impl Server { let name = std::ffi::CString::new(name.clone()).map_err(|err| { Error::Internal(InternalError::ReadConfig( format!( - "key {:?} in [aziot_keys] section of the configuration could not be converted to a C string: {}", - name, err, - ) + "key {:?} in [aziot_keys] section of the configuration could not be converted to a C string: {}", + name, err, + ) .into(), )) })?; let value = - std::ffi::CString::new(value).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( - "value of key {:?} in [aziot_keys] section of the configuration could not be converted to a C string: {}", - name, err, - ).into())))?; + std::ffi::CString::new(value).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( + "value of key {:?} in [aziot_keys] section of the configuration could not be converted to a C string: {}", + name, err, + ).into())))?; keys.set_parameter(&name, &value)?; } @@ -50,21 +113,21 @@ impl Server { for (key_id, value) in preloaded_keys { let name = format!("preloaded_key:{}", key_id); let name = - std::ffi::CString::new(name).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( - "key ID {:?} in [preloaded_keys] section of the configuration could not be converted to a C string: {}", - key_id, err, - ).into())))?; + std::ffi::CString::new(name).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( + "key ID {:?} in [preloaded_keys] section of the configuration could not be converted to a C string: {}", + key_id, err, + ).into())))?; let value = - std::ffi::CString::new(value).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( - "location of key ID {:?} in [preloaded_keys] section of the configuration could not be converted to a C string: {}", - key_id, err, - ).into())))?; + std::ffi::CString::new(value).map_err(|err| Error::Internal(InternalError::ReadConfig(format!( + "location of key ID {:?} in [preloaded_keys] section of the configuration could not be converted to a C string: {}", + key_id, err, + ).into())))?; keys.set_parameter(&name, &value)?; } - Ok(Server { keys }) + Ok(Api { keys }) } pub fn create_key_pair_if_not_exists( diff --git a/key/aziot-keyd/src/main.rs b/key/aziot-keyd/src/main.rs deleted file mode 100644 index 3d075783f..000000000 --- a/key/aziot-keyd/src/main.rs +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -#![deny(rust_2018_idioms, warnings)] -#![deny(clippy::all, clippy::pedantic)] -#![allow( - clippy::default_trait_access, - clippy::let_and_return, - clippy::let_unit_value, - clippy::similar_names, - clippy::type_complexity -)] - -mod http; - -#[tokio::main] -async fn main() -> Result<(), Error> { - let config_path: std::path::PathBuf = std::env::var_os("AZIOT_KEYD_CONFIG") - .map_or_else(|| "/etc/aziot/keyd/config.toml".into(), Into::into); - - let config = std::fs::read(config_path).map_err(|err| { - aziot_keyd::Error::Internal(aziot_keyd::InternalError::ReadConfig(Box::new(err))) - })?; - let aziot_keyd::Config { - aziot_keys, - preloaded_keys, - endpoints: aziot_keyd::Endpoints { - aziot_keyd: connector, - }, - } = toml::from_slice(&config).map_err(|err| { - aziot_keyd::Error::Internal(aziot_keyd::InternalError::ReadConfig(Box::new(err))) - })?; - - let server = aziot_keyd::Server::new(aziot_keys, preloaded_keys)?; - let server = std::sync::Arc::new(futures_util::lock::Mutex::new(server)); - - eprintln!("Starting server..."); - - let incoming = connector.incoming().await?; - let server = hyper::Server::builder(incoming).serve(hyper::service::make_service_fn(|_| { - let server = http::Server { - inner: server.clone(), - }; - futures_util::future::ok::<_, std::convert::Infallible>(server) - })); - let () = server.await?; - - eprintln!("Server stopped."); - - Ok(()) -} - -struct Error(Box, backtrace::Backtrace); - -impl std::fmt::Debug for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "{}", self.0)?; - - let mut source = self.0.source(); - while let Some(err) = source { - writeln!(f, "caused by: {}", err)?; - source = err.source(); - } - - writeln!(f, "{:?}", self.1)?; - - Ok(()) - } -} - -impl From for Error -where - E: Into>, -{ - fn from(err: E) -> Self { - Error(err.into(), Default::default()) - } -} diff --git a/key/aziot-keys-common/Cargo.toml b/key/aziot-keys-common/Cargo.toml new file mode 100644 index 000000000..2e19f03b9 --- /dev/null +++ b/key/aziot-keys-common/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "aziot-keys-common" +version = "0.1.0" +authors = ["Azure IoT Edge Devs"] +edition = "2018" + +[dependencies] +url = "2" + +pkcs11 = { path = "../../pkcs11/pkcs11" } diff --git a/key/aziot-keys-common/src/lib.rs b/key/aziot-keys-common/src/lib.rs new file mode 100644 index 000000000..25923cf2a --- /dev/null +++ b/key/aziot-keys-common/src/lib.rs @@ -0,0 +1,63 @@ +// Copyright (c) Microsoft. All rights reserved. + +#![deny(rust_2018_idioms, warnings)] +#![deny(clippy::all, clippy::pedantic)] + +/// This crate exists to hold any types that need to be shared between aziot-keys and the other crates, +/// so that aziot-keys does not have to be compiled as an rlib. +/// +/// This is because anything that triggers aziot-keys to be compiled as an rlib triggers +/// , specifically: +/// +/// >`panic="abort"` and cdylibs and tests: Create a project with a lib (cdylib crate type), binary, and an integration test, +/// >with panic="abort" in the profile. When `cargo test` runs, the cdylib is built twice (once with panic=abort for the binary, +/// >and once without for the test), with the same filename. Building the lib for the test should probably skip the `cdylib` crate type +/// >(assuming rlib is also available), but implementing this is very difficult. +/// +/// What this bug means for us is that `make test` recompiles aziot-keys every time even if it hasn't changed, +/// because `cargo build` and `cargo test` keep marking the build dirty for each other. + +#[derive(Debug)] +pub enum PreloadedKeyLocation { + Filesystem { path: std::path::PathBuf }, + Pkcs11 { uri: pkcs11::Uri }, +} + +impl std::fmt::Display for PreloadedKeyLocation { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + PreloadedKeyLocation::Filesystem { path } => write!( + f, + "{}", + url::Url::from_file_path(path).map_err(|_| std::fmt::Error)? + ), + PreloadedKeyLocation::Pkcs11 { uri } => uri.fmt(f), + } + } +} + +impl std::str::FromStr for PreloadedKeyLocation { + type Err = Box; + + fn from_str(s: &str) -> Result { + let scheme_end_index = s.find(':').ok_or("missing scheme")?; + let scheme = &s[..scheme_end_index]; + + match scheme { + "file" => { + let uri: url::Url = s.parse()?; + let path = uri + .to_file_path() + .map_err(|()| "cannot convert to file path")?; + Ok(PreloadedKeyLocation::Filesystem { path }) + } + + "pkcs11" => { + let uri = s.parse()?; + Ok(PreloadedKeyLocation::Pkcs11 { uri }) + } + + _ => Err("unrecognized scheme".into()), + } + } +} diff --git a/key/aziot-keys/Cargo.toml b/key/aziot-keys/Cargo.toml index 99d107cd1..3881a6224 100644 --- a/key/aziot-keys/Cargo.toml +++ b/key/aziot-keys/Cargo.toml @@ -5,18 +5,20 @@ authors = ["Azure IoT Edge Devs"] edition = "2018" [lib] -crate-type = ["cdylib", "rlib"] +crate-type = ["cdylib"] [dependencies] foreign-types-shared = "0.1" hex = "0.4" hmac = "0.8" lazy_static = "1" +log = "0.4" openssl = "0.10" openssl-sys = "0.9" sha2 = "0.9" url = "2" +aziot-keys-common = { path = "../aziot-keys-common" } openssl2 = { path = "../../openssl2" } openssl-sys2 = { path = "../../openssl-sys2" } pkcs11 = { path = "../../pkcs11/pkcs11" } diff --git a/key/aziot-keys/src/implementation.rs b/key/aziot-keys/src/implementation.rs index 716c34c0d..8b9160a1d 100644 --- a/key/aziot-keys/src/implementation.rs +++ b/key/aziot-keys/src/implementation.rs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +use aziot_keys_common::PreloadedKeyLocation; + lazy_static::lazy_static! { static ref HOMEDIR_PATH: std::sync::RwLock> = Default::default(); @@ -11,51 +13,6 @@ lazy_static::lazy_static! { static ref PKCS11_BASE_SLOT_SESSION: std::sync::Mutex>> = Default::default(); } -#[derive(Debug)] -pub enum PreloadedKeyLocation { - Filesystem { path: std::path::PathBuf }, - Pkcs11 { uri: pkcs11::Uri }, -} - -impl std::fmt::Display for PreloadedKeyLocation { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - PreloadedKeyLocation::Filesystem { path } => write!( - f, - "{}", - url::Url::from_file_path(path).map_err(|_| std::fmt::Error)? - ), - PreloadedKeyLocation::Pkcs11 { uri } => uri.fmt(f), - } - } -} - -impl std::str::FromStr for PreloadedKeyLocation { - type Err = Box; - - fn from_str(s: &str) -> Result { - let scheme_end_index = s.find(':').ok_or("missing scheme")?; - let scheme = &s[..scheme_end_index]; - - match scheme { - "file" => { - let uri: url::Url = s.parse()?; - let path = uri - .to_file_path() - .map_err(|()| "cannot convert to file path")?; - Ok(PreloadedKeyLocation::Filesystem { path }) - } - - "pkcs11" => { - let uri = s.parse()?; - Ok(PreloadedKeyLocation::Pkcs11 { uri }) - } - - _ => Err("unrecognized scheme".into()), - } - } -} - pub(crate) unsafe fn get_function_list( version: crate::AZIOT_KEYS_VERSION, pfunction_list: *mut *const crate::AZIOT_KEYS_FUNCTION_LIST, @@ -573,7 +530,7 @@ pub(crate) fn err_external(err: E) -> crate::AZIOT_KEYS_STATUS where E: std::fmt::Display, { - eprintln!("{}", err); + log::error!("{}", err); crate::AZIOT_KEYS_ERROR_EXTERNAL } @@ -581,7 +538,7 @@ pub(crate) fn err_fatal(err: E) -> crate::AZIOT_KEYS_STATUS where E: std::fmt::Display, { - eprintln!("{}", err); + log::error!("{}", err); crate::AZIOT_KEYS_ERROR_EXTERNAL } @@ -589,6 +546,6 @@ pub(crate) fn err_invalid_parameter(name: &str, err: E) -> crate::AZIOT_KEYS_ where E: std::fmt::Display, { - eprintln!("invalid parameter {:?}: {}", name, err); + log::error!("invalid parameter {:?}: {}", name, err); crate::AZIOT_KEYS_ERROR_INVALID_PARAMETER } diff --git a/key/aziot-keys/src/lib.rs b/key/aziot-keys/src/lib.rs index 270ccfa86..128cb24f8 100644 --- a/key/aziot-keys/src/lib.rs +++ b/key/aziot-keys/src/lib.rs @@ -447,11 +447,6 @@ pub extern "C" fn cbindgen_unused_AZIOT_KEYS_ENCRYPT_DERIVED_PARAMETERS( unimplemented!(); } -// rlib-only exports: - -// Re-exported so that `aziot init` can use it for parsing user input. -pub use implementation::PreloadedKeyLocation; - /// Catches the error, if any, and returns it. Otherwise returns [`AZIOT_KEYS_SUCCESS`]. fn r#catch(f: impl FnOnce() -> Result<(), AZIOT_KEYS_STATUS>) -> AZIOT_KEYS_STATUS { match f() { diff --git a/pkcs11/pkcs11-openssl-engine/Cargo.toml b/pkcs11/pkcs11-openssl-engine/Cargo.toml index a9e1cdc05..ec3e41d9b 100644 --- a/pkcs11/pkcs11-openssl-engine/Cargo.toml +++ b/pkcs11/pkcs11-openssl-engine/Cargo.toml @@ -9,6 +9,7 @@ build = "build/main.rs" [dependencies] foreign-types-shared = "0.1" +log = "0.4" openssl = "0.10" openssl-errors = "0.1" openssl-sys = "0.9" diff --git a/pkcs11/pkcs11-openssl-engine/src/lib.rs b/pkcs11/pkcs11-openssl-engine/src/lib.rs index 4a4fd11bb..9d3377048 100644 --- a/pkcs11/pkcs11-openssl-engine/src/lib.rs +++ b/pkcs11/pkcs11-openssl-engine/src/lib.rs @@ -78,7 +78,7 @@ fn r#catch( if let Some(function) = function { openssl_errors::put_error!(function(), Error::MESSAGE, "{}", err); } else { - eprintln!("[pkcs11-openssl-engine] error: {}", err); + log::error!("[pkcs11-openssl-engine] error: {}", err); } let mut source = err.source(); @@ -86,7 +86,7 @@ fn r#catch( if let Some(function) = function { openssl_errors::put_error!(function(), Error::MESSAGE, "{}", err); } else { - eprintln!("[pkcs11-openssl-engine] caused by: {}", err); + log::error!("[pkcs11-openssl-engine] caused by: {}", err); } source = err.source(); From 716719d7ab2a46855c3c96294b1267b868e6fcf8 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Mon, 26 Oct 2020 12:52:29 -0700 Subject: [PATCH 02/11] PR comments, and add `merge_toml` test. --- aziotd/src/error.rs | 41 +++++++++ aziotd/src/main.rs | 103 ++++++++++++++--------- cert/aziot-certd/src/http/mod.rs | 4 +- identity/aziot-identityd/src/http/mod.rs | 4 +- key/aziot-keyd/src/http/mod.rs | 4 +- 5 files changed, 111 insertions(+), 45 deletions(-) create mode 100644 aziotd/src/error.rs diff --git a/aziotd/src/error.rs b/aziotd/src/error.rs new file mode 100644 index 000000000..c36fe2256 --- /dev/null +++ b/aziotd/src/error.rs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft. All rights reserved. + +#[derive(Debug)] +pub(crate) struct Error(pub(crate) ErrorKind, pub(crate) backtrace::Backtrace); + +#[derive(Debug)] +pub(crate) enum ErrorKind { + GetProcessName(std::borrow::Cow<'static, str>), + ReadConfig(Option, Box), + Service(Box), +} + +impl std::fmt::Display for ErrorKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ErrorKind::GetProcessName(message) => write!(f, "could not read argv[0]: {}", message), + ErrorKind::ReadConfig(Some(path), _) => { + write!(f, "could not read config from {}", path.display()) + } + ErrorKind::ReadConfig(None, _) => f.write_str("could not read config"), + ErrorKind::Service(_) => f.write_str("service encountered an error"), + } + } +} + +impl std::error::Error for ErrorKind { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + #[allow(clippy::match_same_arms)] + match self { + ErrorKind::GetProcessName(_) => None, + ErrorKind::ReadConfig(_, err) => Some(&**err), + ErrorKind::Service(err) => Some(&**err), + } + } +} + +impl From for Error { + fn from(err: ErrorKind) -> Self { + Error(err, Default::default()) + } +} diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index ec0ce3922..85ff73804 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -4,8 +4,11 @@ #![deny(clippy::all, clippy::pedantic)] #![allow(clippy::default_trait_access, clippy::let_unit_value)] +mod error; mod logging; +use error::{Error, ErrorKind}; + #[tokio::main] async fn main() { logging::init(); @@ -191,8 +194,10 @@ fn merge_toml(base: &mut toml::Value, patch: toml::Value) { if let toml::Value::Table(base) = base { if let toml::Value::Table(patch) = patch { - for (k, v) in patch { - merge_toml(base.entry(k).or_insert(toml::Value::Boolean(false)), v); + for (key, value) in patch { + // Insert a dummy `false` if the original key didn't exist at all. It'll be overwritten by `value` in that case. + let original_value = base.entry(key).or_insert(toml::Value::Boolean(false)); + merge_toml(original_value, value); } return; @@ -202,42 +207,62 @@ fn merge_toml(base: &mut toml::Value, patch: toml::Value) { *base = patch; } -#[derive(Debug)] -struct Error(ErrorKind, backtrace::Backtrace); - -#[derive(Debug)] -enum ErrorKind { - GetProcessName(std::borrow::Cow<'static, str>), - ReadConfig(Option, Box), - Service(Box), -} - -impl std::fmt::Display for ErrorKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - ErrorKind::GetProcessName(message) => write!(f, "could not read argv[0]: {}", message), - ErrorKind::ReadConfig(Some(path), _) => { - write!(f, "could not read config from {}", path.display()) - } - ErrorKind::ReadConfig(None, _) => f.write_str("could not read config"), - ErrorKind::Service(_) => f.write_str("service encountered an error"), - } - } -} - -impl std::error::Error for ErrorKind { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - #[allow(clippy::match_same_arms)] - match self { - ErrorKind::GetProcessName(_) => None, - ErrorKind::ReadConfig(_, err) => Some(&**err), - ErrorKind::Service(err) => Some(&**err), - } - } -} - -impl From for Error { - fn from(err: ErrorKind) -> Self { - Error(err, Default::default()) +#[cfg(test)] +mod tests { + #[test] + fn merge_toml() { + let base = r#" +foo_key = "A" +foo_parent_key = { foo_sub_key = "B" } + +[bar_table] +bar_table_key = "C" +bar_table_parent_key = { bar_table_sub_key = "D" } + +[[baz_table_array]] +baz_table_array_key = "E" +baz_table_array_parent_key = { baz_table_sub_key = "F" } +"#; + let mut base: toml::Value = toml::from_str(base).unwrap(); + + let patch = r#" +foo_key = "A2" +foo_key_new = "A3" +foo_parent_key = { foo_sub_key = "B2", foo_sub_key2 = "B3" } +foo_parent_key_new = { foo_sub_key = "B4", foo_sub_key2 = "B5" } + +[bar_table] +bar_table_key = "C2" +bar_table_key_new = "C3" +bar_table_parent_key = { bar_table_sub_key = "D2", bar_table_sub_key2 = "D3" } +bar_table_parent_key_new = { bar_table_sub_key = "D4", bar_table_sub_key2 = "D5" } + +[[baz_table_array]] +baz_table_array_key = "G" +baz_table_array_parent_key = { baz_table_sub_key = "H" } +"#; + let patch: toml::Value = toml::from_str(patch).unwrap(); + + super::merge_toml(&mut base, patch); + + let expected = r#" +foo_key = "A2" +foo_key_new = "A3" +foo_parent_key = { foo_sub_key = "B2", foo_sub_key2 = "B3" } +foo_parent_key_new = { foo_sub_key = "B4", foo_sub_key2 = "B5" } + + +[bar_table] +bar_table_key = "C2" +bar_table_key_new = "C3" +bar_table_parent_key = { bar_table_sub_key = "D2", bar_table_sub_key2 = "D3" } +bar_table_parent_key_new = { bar_table_sub_key = "D4", bar_table_sub_key2 = "D5" } + +[[baz_table_array]] +baz_table_array_key = "G" +baz_table_array_parent_key = { baz_table_sub_key = "H" } +"#; + let expected: toml::Value = toml::from_str(expected).unwrap(); + assert_eq!(expected, base); } } diff --git a/cert/aziot-certd/src/http/mod.rs b/cert/aziot-certd/src/http/mod.rs index 3846a09d4..52072fecd 100644 --- a/cert/aziot-certd/src/http/mod.rs +++ b/cert/aziot-certd/src/http/mod.rs @@ -24,9 +24,9 @@ fn to_http_error(err: &crate::Error) -> http_common::server::Error { for line in error_message.split('\n') { log::log!( if matches!(err, crate::Error::Internal(_)) { - log::Level::Info - } else { log::Level::Error + } else { + log::Level::Info }, "!!! {}", line, diff --git a/identity/aziot-identityd/src/http/mod.rs b/identity/aziot-identityd/src/http/mod.rs index 718103458..96f459956 100644 --- a/identity/aziot-identityd/src/http/mod.rs +++ b/identity/aziot-identityd/src/http/mod.rs @@ -32,9 +32,9 @@ fn to_http_error(err: &crate::Error) -> http_common::server::Error { for line in error_message.split('\n') { log::log!( if matches!(err, crate::Error::Internal(_)) { - log::Level::Info - } else { log::Level::Error + } else { + log::Level::Info }, "!!! {}", line, diff --git a/key/aziot-keyd/src/http/mod.rs b/key/aziot-keyd/src/http/mod.rs index 28a42949d..6c6cd063a 100644 --- a/key/aziot-keyd/src/http/mod.rs +++ b/key/aziot-keyd/src/http/mod.rs @@ -38,9 +38,9 @@ fn to_http_error(err: &crate::Error) -> http_common::server::Error { for line in error_message.split('\n') { log::log!( if matches!(err, crate::Error::Internal(_)) { - log::Level::Info - } else { log::Level::Error + } else { + log::Level::Info }, "!!! {}", line, From 5b6ef8611f93d2dd1712455f932a1d58661776a3 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Mon, 26 Oct 2020 12:57:45 -0700 Subject: [PATCH 03/11] Fix Cargo.toml formatting to be more consistent. --- aziot/Cargo.toml | 2 ++ key/aziot-keys-common/Cargo.toml | 1 + key/aziot-keys/Cargo.toml | 2 ++ 3 files changed, 5 insertions(+) diff --git a/aziot/Cargo.toml b/aziot/Cargo.toml index 6898dd3f5..41058acce 100644 --- a/aziot/Cargo.toml +++ b/aziot/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" authors = ["Azure IoT Edge Devs"] edition = "2018" + [dependencies] backtrace = "0.3" base64 = "0.12" @@ -19,5 +20,6 @@ aziot-identityd = { path = "../identity/aziot-identityd" } aziot-keyd = { path = "../key/aziot-keyd" } aziot-keys-common = { path = "../key/aziot-keys-common" } + [dev-dependencies] bytes = "0.5" diff --git a/key/aziot-keys-common/Cargo.toml b/key/aziot-keys-common/Cargo.toml index 2e19f03b9..4395854b7 100644 --- a/key/aziot-keys-common/Cargo.toml +++ b/key/aziot-keys-common/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" authors = ["Azure IoT Edge Devs"] edition = "2018" + [dependencies] url = "2" diff --git a/key/aziot-keys/Cargo.toml b/key/aziot-keys/Cargo.toml index 3881a6224..be75334d1 100644 --- a/key/aziot-keys/Cargo.toml +++ b/key/aziot-keys/Cargo.toml @@ -4,9 +4,11 @@ version = "0.1.0" authors = ["Azure IoT Edge Devs"] edition = "2018" + [lib] crate-type = ["cdylib"] + [dependencies] foreign-types-shared = "0.1" hex = "0.4" From f3399efb023b408928e77d6686fd07c38522a39b Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Mon, 26 Oct 2020 14:16:22 -0700 Subject: [PATCH 04/11] matches! -> match --- cert/aziot-certd/src/http/mod.rs | 7 +++---- identity/aziot-identityd/src/http/mod.rs | 7 +++---- key/aziot-keyd/src/http/mod.rs | 7 +++---- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/cert/aziot-certd/src/http/mod.rs b/cert/aziot-certd/src/http/mod.rs index 52072fecd..9caac825b 100644 --- a/cert/aziot-certd/src/http/mod.rs +++ b/cert/aziot-certd/src/http/mod.rs @@ -23,10 +23,9 @@ fn to_http_error(err: &crate::Error) -> http_common::server::Error { // TODO: When we get distributed tracing, associate these logs with the tracing ID. for line in error_message.split('\n') { log::log!( - if matches!(err, crate::Error::Internal(_)) { - log::Level::Error - } else { - log::Level::Info + match err { + crate::Error::Internal(_) => log::Level::Error, + _ => log::Level::Info, }, "!!! {}", line, diff --git a/identity/aziot-identityd/src/http/mod.rs b/identity/aziot-identityd/src/http/mod.rs index 96f459956..d4c2099a4 100644 --- a/identity/aziot-identityd/src/http/mod.rs +++ b/identity/aziot-identityd/src/http/mod.rs @@ -31,10 +31,9 @@ fn to_http_error(err: &crate::Error) -> http_common::server::Error { // TODO: When we get distributed tracing, associate these logs with the tracing ID. for line in error_message.split('\n') { log::log!( - if matches!(err, crate::Error::Internal(_)) { - log::Level::Error - } else { - log::Level::Info + match err { + crate::Error::Internal(_) => log::Level::Error, + _ => log::Level::Info, }, "!!! {}", line, diff --git a/key/aziot-keyd/src/http/mod.rs b/key/aziot-keyd/src/http/mod.rs index 6c6cd063a..0578ab411 100644 --- a/key/aziot-keyd/src/http/mod.rs +++ b/key/aziot-keyd/src/http/mod.rs @@ -37,10 +37,9 @@ fn to_http_error(err: &crate::Error) -> http_common::server::Error { // TODO: When we get distributed tracing, associate these logs with the tracing ID. for line in error_message.split('\n') { log::log!( - if matches!(err, crate::Error::Internal(_)) { - log::Level::Error - } else { - log::Level::Info + match err { + crate::Error::Internal(_) => log::Level::Error, + _ => log::Level::Info, }, "!!! {}", line, From 1faadc7bb1dd3a619b881c902cde7fb4e54ad371 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Mon, 26 Oct 2020 16:21:25 -0700 Subject: [PATCH 05/11] Support falling back to argv[1] for service name, and add tests. --- aziotd/src/main.rs | 133 ++++++++++++++++++++++------ docs/dev/running/aziot-certd.md | 2 +- docs/dev/running/aziot-identityd.md | 2 +- docs/dev/running/aziot-keyd.md | 2 +- 4 files changed, 110 insertions(+), 29 deletions(-) diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index 85ff73804..8e19fa5b1 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -29,25 +29,11 @@ async fn main() { } async fn main_inner() -> Result<(), Error> { - let argv0 = std::env::args_os() - .next() - .ok_or_else(|| ErrorKind::GetProcessName("argv[0] not set".into()))?; + let mut args = std::env::args_os(); + let process_name = process_name_from_args(&mut args)?; - // argv[0] could be a single component like "aziot-certd", or a path that ends with "aziot-certd", - // so parse it as a Path and get the last component. This does the right thing in either case. - let argv0 = std::path::Path::new(&argv0); - let process_name = argv0.file_name().ok_or_else(|| { - ErrorKind::GetProcessName( - format!( - "could not extract process name from argv[0] {:?}", - argv0.display(), - ) - .into(), - ) - })?; - - match process_name.to_str() { - Some("aziot-certd") => { + match process_name { + ProcessName::Certd => { run( aziot_certd::main, "AZIOT_CERTD_CONFIG", @@ -58,7 +44,7 @@ async fn main_inner() -> Result<(), Error> { .await? } - Some("aziot-identityd") => { + ProcessName::Identityd => { run( aziot_identityd::main, "AZIOT_IDENTITYD_CONFIG", @@ -69,7 +55,7 @@ async fn main_inner() -> Result<(), Error> { .await? } - Some("aziot-keyd") => { + ProcessName::Keyd => { run( aziot_keyd::main, "AZIOT_CERTD_CONFIG", @@ -79,17 +65,57 @@ async fn main_inner() -> Result<(), Error> { ) .await? } - _ => { - return Err(ErrorKind::GetProcessName( - format!("unrecognized process name {:?}", process_name).into(), - ) - .into()) - } } Ok(()) } +#[derive(Clone, Copy, Debug, PartialEq)] +enum ProcessName { + Certd, + Identityd, + Keyd, +} + +fn process_name_from_args(args: &mut I) -> Result +where + I: Iterator, + ::Item: AsRef, +{ + let arg = args.next().ok_or_else(|| { + ErrorKind::GetProcessName("could not extract process name from args".into()) + })?; + + // arg could be a single component like "aziot-certd", or a path that ends with "aziot-certd", + // so parse it as a Path and get the last component. This does the right thing in either case. + let arg = std::path::Path::new(&arg); + let process_name = arg.file_name().ok_or_else(|| { + ErrorKind::GetProcessName( + format!( + "could not extract process name from arg {:?}", + arg.display(), + ) + .into(), + ) + })?; + + match process_name.to_str() { + Some("aziot-certd") => Ok(ProcessName::Certd), + + Some("aziot-identityd") => Ok(ProcessName::Identityd), + + Some("aziot-keyd") => Ok(ProcessName::Keyd), + + // The next is the process name + Some("aziotd") => process_name_from_args(args), + + _ => Err(ErrorKind::GetProcessName( + format!("unrecognized process name {:?}", process_name).into(), + ) + .into()), + } +} + async fn run( main: TMain, config_env_var: &str, @@ -209,6 +235,61 @@ fn merge_toml(base: &mut toml::Value, patch: toml::Value) { #[cfg(test)] mod tests { + #[test] + fn process_name_from_args() { + for &(input, expected) in &[ + (&["aziot-certd"][..], super::ProcessName::Certd), + (&["aziot-identityd"][..], super::ProcessName::Identityd), + (&["aziot-keyd"][..], super::ProcessName::Keyd), + ( + &["/usr/libexec/aziot/aziot-certd"][..], + super::ProcessName::Certd, + ), + ( + &["/usr/libexec/aziot/aziot-identityd"][..], + super::ProcessName::Identityd, + ), + ( + &["/usr/libexec/aziot/aziot-keyd"][..], + super::ProcessName::Keyd, + ), + (&["aziotd", "aziot-certd"][..], super::ProcessName::Certd), + ( + &["aziotd", "aziot-identityd"][..], + super::ProcessName::Identityd, + ), + (&["aziotd", "aziot-keyd"][..], super::ProcessName::Keyd), + ( + &["/usr/libexec/aziot/aziotd", "aziot-certd"][..], + super::ProcessName::Certd, + ), + ( + &["/usr/libexec/aziot/aziotd", "aziot-identityd"][..], + super::ProcessName::Identityd, + ), + ( + &["/usr/libexec/aziot/aziotd", "aziot-keyd"][..], + super::ProcessName::Keyd, + ), + ] { + let mut input = input.iter().copied().map(std::ffi::OsStr::new); + let actual = super::process_name_from_args(&mut input).unwrap(); + assert_eq!(None, input.next()); + assert_eq!(expected, actual); + } + + for &input in &[ + &["foo"][..], + &["/usr/libexec/aziot/foo"][..], + &["aziotd", "foo"][..], + &["/usr/libexec/aziot/aziotd", "foo"][..], + &["/usr/libexec/aziot/foo", "aziot-certd"][..], + ] { + let mut input = input.iter().copied().map(std::ffi::OsStr::new); + let _ = super::process_name_from_args(&mut input).unwrap_err(); + } + } + #[test] fn merge_toml() { let base = r#" diff --git a/docs/dev/running/aziot-certd.md b/docs/dev/running/aziot-certd.md index ab0b8f2c2..4b3ea0d9d 100644 --- a/docs/dev/running/aziot-certd.md +++ b/docs/dev/running/aziot-certd.md @@ -211,7 +211,7 @@ With this basic file, fill it out depending on what workflow you want to test: export AZIOT_CERTD_CONFIG='...' - (exec -a aziot-certd target/x86_64-unknown-linux-gnu/debug/aziotd) + cargo run --target x86_64-unknown-linux-gnu -p aziotd -- aziot-certd ``` The service will remain running. diff --git a/docs/dev/running/aziot-identityd.md b/docs/dev/running/aziot-identityd.md index c145e9072..ae98fe6ff 100644 --- a/docs/dev/running/aziot-identityd.md +++ b/docs/dev/running/aziot-identityd.md @@ -156,5 +156,5 @@ export AZIOT_LOG=aziot=debug export AZIOT_IDENTITYD_CONFIG='...' -(exec -a aziot-identityd target/x86_64-unknown-linux-gnu/debug/aziotd) +cargo run --target x86_64-unknown-linux-gnu -p aziotd -- aziot-identityd ``` diff --git a/docs/dev/running/aziot-keyd.md b/docs/dev/running/aziot-keyd.md index a92ff5989..29d1bfdbf 100644 --- a/docs/dev/running/aziot-keyd.md +++ b/docs/dev/running/aziot-keyd.md @@ -127,7 +127,7 @@ Assuming you're using Microsoft's implementation of `libaziot_keys.so`, start wi export AZIOT_KEYD_CONFIG='...' - (exec -a aziot-keyd target/x86_64-unknown-linux-gnu/debug/aziotd) + cargo run --target x86_64-unknown-linux-gnu -p aziotd -- aziot-keyd ``` The service will remain running. From e36e1334cd8c4a37f4506e5dcb3c1845e3c3dd24 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Mon, 26 Oct 2020 16:30:58 -0700 Subject: [PATCH 06/11] Add some docs, and restrict argv[1] fallback to debug builds. --- aziotd/src/main.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index 8e19fa5b1..9ecea753b 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -1,5 +1,10 @@ // Copyright (c) Microsoft. All rights reserved. +//! This binary is the process entrypoint for aziot-certd, -identityd and -keyd. +//! Rather than be three separate binaries, all three services are symlinks to +//! this one aziotd binary. The aziotd binary looks at its command-line args to figure out +//! which service it's being invoked as, and runs the code of that service accordingly. + #![deny(rust_2018_idioms, warnings)] #![deny(clippy::all, clippy::pedantic)] #![allow(clippy::default_trait_access, clippy::let_unit_value)] @@ -77,6 +82,12 @@ enum ProcessName { Keyd, } +/// If the symlink is being used to invoke this binary, the process name can be determined +/// from the first arg, ie `argv[0]` in C terms. +/// +/// An alternative is supported where the binary is invoked as aziotd itself, +/// and the process name is instead the next arg, ie `argv[1]` in C terms. +/// This is primary useful for local development, so it's only allowed in debug builds. fn process_name_from_args(args: &mut I) -> Result where I: Iterator, @@ -106,7 +117,8 @@ where Some("aziot-keyd") => Ok(ProcessName::Keyd), - // The next is the process name + // The next arg is the process name + #[cfg(debug_assertions)] Some("aziotd") => process_name_from_args(args), _ => Err(ErrorKind::GetProcessName( From c83455a606efc9885c5fb846a0434c42cdb01bd2 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Mon, 26 Oct 2020 16:39:02 -0700 Subject: [PATCH 07/11] Merge arrays by concatenating the patch to the base. --- aziotd/src/main.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index 9ecea753b..3fba94a63 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -227,10 +227,13 @@ where } fn merge_toml(base: &mut toml::Value, patch: toml::Value) { - // Similar to JSON patch, except that maps are called tables, and - // there is no equivalent of null that can be used to remove keys from an object. + // Similar to JSON patch, except that: + // + // - Maps are called tables + // - There is no equivalent of null that can be used to remove keys from an object. + // - Arrays are merged via concatenating the patch to the base, rather than replacing the base with the patch. - if let toml::Value::Table(base) = base { + if let toml::Value::Table(base) = &mut *base { if let toml::Value::Table(patch) = patch { for (key, value) in patch { // Insert a dummy `false` if the original key didn't exist at all. It'll be overwritten by `value` in that case. @@ -242,6 +245,13 @@ fn merge_toml(base: &mut toml::Value, patch: toml::Value) { } } + if let toml::Value::Array(base) = base { + if let toml::Value::Array(patch) = patch { + base.extend(patch); + return; + } + } + *base = patch; } @@ -344,13 +354,16 @@ foo_key_new = "A3" foo_parent_key = { foo_sub_key = "B2", foo_sub_key2 = "B3" } foo_parent_key_new = { foo_sub_key = "B4", foo_sub_key2 = "B5" } - [bar_table] bar_table_key = "C2" bar_table_key_new = "C3" bar_table_parent_key = { bar_table_sub_key = "D2", bar_table_sub_key2 = "D3" } bar_table_parent_key_new = { bar_table_sub_key = "D4", bar_table_sub_key2 = "D5" } +[[baz_table_array]] +baz_table_array_key = "E" +baz_table_array_parent_key = { baz_table_sub_key = "F" } + [[baz_table_array]] baz_table_array_key = "G" baz_table_array_parent_key = { baz_table_sub_key = "H" } From becac6833628aece78cddffe9052f5c81d624d92 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Mon, 26 Oct 2020 17:51:18 -0700 Subject: [PATCH 08/11] Fix tests in release builds. --- aziot/src/init.rs | 1 + aziotd/src/main.rs | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/aziot/src/init.rs b/aziot/src/init.rs index f580dc68e..37d09231a 100644 --- a/aziot/src/init.rs +++ b/aziot/src/init.rs @@ -1065,6 +1065,7 @@ fn write_file( Ok(()) } +#[cfg(debug_assertions)] #[cfg(test)] mod tests { struct Stdin(std::io::BufReader); diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index 3fba94a63..78fa9af9d 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -229,7 +229,7 @@ where fn merge_toml(base: &mut toml::Value, patch: toml::Value) { // Similar to JSON patch, except that: // - // - Maps are called tables + // - Maps are called tables. // - There is no equivalent of null that can be used to remove keys from an object. // - Arrays are merged via concatenating the patch to the base, rather than replacing the base with the patch. @@ -275,20 +275,26 @@ mod tests { &["/usr/libexec/aziot/aziot-keyd"][..], super::ProcessName::Keyd, ), + #[cfg(debug_assertions)] (&["aziotd", "aziot-certd"][..], super::ProcessName::Certd), + #[cfg(debug_assertions)] ( &["aziotd", "aziot-identityd"][..], super::ProcessName::Identityd, ), + #[cfg(debug_assertions)] (&["aziotd", "aziot-keyd"][..], super::ProcessName::Keyd), + #[cfg(debug_assertions)] ( &["/usr/libexec/aziot/aziotd", "aziot-certd"][..], super::ProcessName::Certd, ), + #[cfg(debug_assertions)] ( &["/usr/libexec/aziot/aziotd", "aziot-identityd"][..], super::ProcessName::Identityd, ), + #[cfg(debug_assertions)] ( &["/usr/libexec/aziot/aziotd", "aziot-keyd"][..], super::ProcessName::Keyd, From d9db9c16d4212ae0cd971542561b17f6d2a88596 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Tue, 27 Oct 2020 11:11:28 -0700 Subject: [PATCH 09/11] PR comments. --- aziotd/src/main.rs | 62 ++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index 78fa9af9d..decb05f99 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -259,7 +259,8 @@ fn merge_toml(base: &mut toml::Value, patch: toml::Value) { mod tests { #[test] fn process_name_from_args() { - for &(input, expected) in &[ + // Success test cases + let mut test_cases = vec![ (&["aziot-certd"][..], super::ProcessName::Certd), (&["aziot-identityd"][..], super::ProcessName::Identityd), (&["aziot-keyd"][..], super::ProcessName::Keyd), @@ -275,43 +276,50 @@ mod tests { &["/usr/libexec/aziot/aziot-keyd"][..], super::ProcessName::Keyd, ), - #[cfg(debug_assertions)] - (&["aziotd", "aziot-certd"][..], super::ProcessName::Certd), - #[cfg(debug_assertions)] - ( - &["aziotd", "aziot-identityd"][..], - super::ProcessName::Identityd, - ), - #[cfg(debug_assertions)] - (&["aziotd", "aziot-keyd"][..], super::ProcessName::Keyd), - #[cfg(debug_assertions)] - ( - &["/usr/libexec/aziot/aziotd", "aziot-certd"][..], - super::ProcessName::Certd, - ), - #[cfg(debug_assertions)] - ( - &["/usr/libexec/aziot/aziotd", "aziot-identityd"][..], - super::ProcessName::Identityd, - ), - #[cfg(debug_assertions)] - ( - &["/usr/libexec/aziot/aziotd", "aziot-keyd"][..], - super::ProcessName::Keyd, - ), - ] { + ]; + + // argv[1] fallback is only in release builds. + if cfg!(debug_assertions) { + test_cases.extend_from_slice(&[ + (&["aziotd", "aziot-certd"][..], super::ProcessName::Certd), + ( + &["aziotd", "aziot-identityd"][..], + super::ProcessName::Identityd, + ), + (&["aziotd", "aziot-keyd"][..], super::ProcessName::Keyd), + ( + &["/usr/libexec/aziot/aziotd", "aziot-certd"][..], + super::ProcessName::Certd, + ), + ( + &["/usr/libexec/aziot/aziotd", "aziot-identityd"][..], + super::ProcessName::Identityd, + ), + ( + &["/usr/libexec/aziot/aziotd", "aziot-keyd"][..], + super::ProcessName::Keyd, + ), + ]); + } + + for (input, expected) in test_cases { let mut input = input.iter().copied().map(std::ffi::OsStr::new); let actual = super::process_name_from_args(&mut input).unwrap(); assert_eq!(None, input.next()); assert_eq!(expected, actual); } + // Failure test cases for &input in &[ + // Unrecognized process name in argv[0] &["foo"][..], &["/usr/libexec/aziot/foo"][..], + &["/usr/libexec/aziot/foo", "aziot-certd"][..], + // Either fails because it's a release build so argv[1] fallback is disabled, + // or fails because it's a debug build where argv[1] fallback is enabled + // but the process name in argv[1] is unrecognized anyway. &["aziotd", "foo"][..], &["/usr/libexec/aziot/aziotd", "foo"][..], - &["/usr/libexec/aziot/foo", "aziot-certd"][..], ] { let mut input = input.iter().copied().map(std::ffi::OsStr::new); let _ = super::process_name_from_args(&mut input).unwrap_err(); From e49cf4b6e5b54b273f634a9d4b05c8b60004a4d1 Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Tue, 27 Oct 2020 11:38:12 -0700 Subject: [PATCH 10/11] PR comments. --- aziot/src/init.rs | 87 ++++++++++++++++++++++------------------------ aziotd/src/main.rs | 2 +- 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/aziot/src/init.rs b/aziot/src/init.rs index 37d09231a..06c8071fc 100644 --- a/aziot/src/init.rs +++ b/aziot/src/init.rs @@ -36,33 +36,35 @@ const EST_BOOTSTRAP_ID: &str = "est-bootstrap-id"; const EST_SERVER_CA_ID: &str = "est-server-ca"; pub(crate) fn run() -> Result<(), crate::Error> { - // Get the three users. + // In production, running as root is the easiest way to guarantee the tool has write access to every service's config file. + // But it's convenient to not do this for the sake of development because the the development machine doesn't necessarily + // have the package installed and the users created, and it's easier to have the config files owned by the current user anyway. // - // In debug builds, we allow `aziot init` to be run without root. In that case, use the current user. - - let aziotks_user = (if cfg!(debug_assertions) && !nix::unistd::Uid::current().is_root() { - nix::unistd::User::from_uid(nix::unistd::Uid::current()) - } else { - nix::unistd::User::from_name("aziotks") - }) - .map_err(|err| format!("could not query aziotks user information: {}", err))? - .ok_or_else(|| "could not query aziotks user information")?; - - let aziotcs_user = (if cfg!(debug_assertions) && !nix::unistd::Uid::current().is_root() { - nix::unistd::User::from_uid(nix::unistd::Uid::current()) - } else { - nix::unistd::User::from_name("aziotcs") - }) - .map_err(|err| format!("could not query aziotcs user information: {}", err))? - .ok_or_else(|| "could not query aziotcs user information")?; - - let aziotid_user = (if cfg!(debug_assertions) && !nix::unistd::Uid::current().is_root() { - nix::unistd::User::from_uid(nix::unistd::Uid::current()) + // So when running as root, get the three users appropriately. + // Otherwise, if this is a debug build, fall back to using the current user. + // Otherwise, tell the user to re-run as root. + let (aziotks_user, aziotcs_user, aziotid_user) = if nix::unistd::Uid::current().is_root() { + let aziotks_user = nix::unistd::User::from_name("aziotks") + .map_err(|err| format!("could not query aziotks user information: {}", err))? + .ok_or_else(|| "could not query aziotks user information")?; + + let aziotcs_user = nix::unistd::User::from_name("aziotcs") + .map_err(|err| format!("could not query aziotcs user information: {}", err))? + .ok_or_else(|| "could not query aziotcs user information")?; + + let aziotid_user = nix::unistd::User::from_name("aziotid") + .map_err(|err| format!("could not query aziotid user information: {}", err))? + .ok_or_else(|| "could not query aziotid user information")?; + + (aziotks_user, aziotcs_user, aziotid_user) + } else if cfg!(debug_assertions) { + let current_user = nix::unistd::User::from_uid(nix::unistd::Uid::current()) + .map_err(|err| format!("could not query current user information: {}", err))? + .ok_or_else(|| "could not query current user information")?; + (current_user.clone(), current_user.clone(), current_user) } else { - nix::unistd::User::from_name("aziotid") - }) - .map_err(|err| format!("could not query aziotid user information: {}", err))? - .ok_or_else(|| "could not query aziotid user information")?; + return Err("this command must be run as root".into()); + }; for &f in &[ "/etc/aziot/certd/config.toml", @@ -76,9 +78,9 @@ pub(crate) fn run() -> Result<(), crate::Error> { if std::path::Path::new(f).exists() { return Err(format!( "\ - Cannot run because file {} already exists. \ - Delete this file (after taking a backup if necessary) before running this command.\ - ", + Cannot run because file {} already exists. \ + Delete this file (after taking a backup if necessary) before running this command.\ + ", f ) .into()); @@ -141,15 +143,15 @@ pub(crate) fn run() -> Result<(), crate::Error> { /// This macro expands to a relatively straightforward expression. The advantage of using this macro instead of calling /// [`choose`] directly is that the macro forces you to use each one of your choice enum variants exactly once, no more and no less. macro_rules! choose { - ( - $stdin:ident , - $question:expr , - $($choice:path => $value:expr ,)* - ) => {{ - match choose($stdin, $question, &[$($choice ,)*])? { - $($choice => $value ,)* - } - }}; + ( + $stdin:ident , + $question:expr , + $($choice:path => $value:expr ,)* + ) => {{ + match choose($stdin, $question, &[$($choice ,)*])? { + $($choice => $value ,)* + } + }}; } #[derive(Debug)] @@ -167,12 +169,6 @@ fn run_inner(stdin: &mut impl Reader) -> Result { println!("This command will set up the configurations for aziot-keyd, aziot-certd and aziot-identityd."); println!(); - // In production, running as root is the easiest way to guarantee the tool has write access to every service's config file. - // But allow running as a regular use in debug builds for the sake of development. - if !cfg!(debug_assertions) && !nix::unistd::Uid::current().is_root() { - return Err("this command must be run as root".into()); - } - let hostname = get_hostname()?; let (provisioning_type, preloaded_device_id_pk_bytes) = choose! { @@ -925,8 +921,8 @@ fn parse_manual_connection_string( let symmetric_key = symmetric_key.ok_or(r#"required parameter "SharedAccessKey" is missing"#)?; let symmetric_key = - base64::decode(symmetric_key) - .map_err(|err| format!(r#"connection string's "SharedAccessKey" parameter could not be decoded from base64: {}"#, err))?; + base64::decode(symmetric_key) + .map_err(|err| format!(r#"connection string's "SharedAccessKey" parameter could not be decoded from base64: {}"#, err))?; Ok(( iothub_hostname.to_owned(), @@ -1065,7 +1061,6 @@ fn write_file( Ok(()) } -#[cfg(debug_assertions)] #[cfg(test)] mod tests { struct Stdin(std::io::BufReader); diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index decb05f99..eb6c95915 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -233,7 +233,7 @@ fn merge_toml(base: &mut toml::Value, patch: toml::Value) { // - There is no equivalent of null that can be used to remove keys from an object. // - Arrays are merged via concatenating the patch to the base, rather than replacing the base with the patch. - if let toml::Value::Table(base) = &mut *base { + if let toml::Value::Table(base) = base { if let toml::Value::Table(patch) = patch { for (key, value) in patch { // Insert a dummy `false` if the original key didn't exist at all. It'll be overwritten by `value` in that case. From fdb5f2213541b2f7dddf0cdfaf3d5c275ac8206a Mon Sep 17 00:00:00 2001 From: Arnav Singh Date: Tue, 27 Oct 2020 12:09:22 -0700 Subject: [PATCH 11/11] Sort patch paths to ensure they're applied deterministically. --- aziotd/src/main.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/aziotd/src/main.rs b/aziotd/src/main.rs index eb6c95915..3f5f8a463 100644 --- a/aziotd/src/main.rs +++ b/aziotd/src/main.rs @@ -169,6 +169,7 @@ where match std::fs::read_dir(&config_directory_path) { Ok(entries) => { + let mut patch_paths = vec![]; for entry in entries { let entry = entry.map_err(|err| { ErrorKind::ReadConfig(Some(config_directory_path.clone()), Box::new(err)) @@ -186,6 +187,11 @@ where continue; } + patch_paths.push(patch_path); + } + patch_paths.sort(); + + for patch_path in patch_paths { let patch = std::fs::read(&patch_path).map_err(|err| { ErrorKind::ReadConfig(Some(patch_path.clone()), Box::new(err)) })?;