Fixed code review comments

* Removed toug-kms create and created issue #211 to handle it.
* KMS CMK needs to be manually created separately
* Moved Sign implementation for KmsRsaKey to tough_kms crate

.github/workflows/rust.yml

@@ -13,6 +13,7 @@ jobs:
     - run: cargo build --locked -p olpc-cjson
     - run: cargo build --locked -p tough
     - run: cargo build --locked -p tough-ssm
+    - run: cargo build --locked -p tough-kms
     - run: cargo build --locked -p tuftool
     - run: cargo test --locked
     - run: cd tough && cargo test --all-features --locked

tough-kms/Cargo.toml

@@ -8,7 +8,6 @@ repository = "https://github.com/awslabs/tough"
 keywords = ["TUF", "KMS"]
 edition = "2018"
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 [features]
 default = ["rusoto"]
 rusoto = ["rusoto-rustls"]

tough-kms/src/client.rs

@@ -8,7 +8,7 @@ use rusoto_kms::KmsClient;
 use snafu::ResultExt;
 use std::str::FromStr;
 
-/// Builds an KMS client for a given profile name.
+/// Builds a KMS client for a given profile name.
 pub(crate) fn build_client_kms(profile: Option<&str>) -> Result<KmsClient> {
     Ok(if let Some(profile) = profile {
         let mut provider = ProfileProvider::new().context(error::RusotoCreds)?;

tough-kms/src/error.rs

@@ -1,76 +1,59 @@
 // Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
+//! Contains the error type for this library.
+
+#![allow(clippy::default_trait_access)]
+
 use snafu::{Backtrace, Snafu};
 
+/// Alias for `Result<T, Error>`.
 pub type Result<T> = std::result::Result<T, Error>;
 
 /// The error type for this library.
 #[derive(Debug, Snafu)]
 #[snafu(visibility = "pub(crate)")]
+#[non_exhaustive]
+#[allow(missing_docs)]
 pub enum Error {
-    #[snafu(display(
-    "Failed to create customer managed key for aws-kms://{}/{}: {}",
-    profile.as_deref().unwrap_or(""),
-    key_id,
-    source,
-    ))]
-    KmsCreateKey {
-        profile: Option<String>,
-        key_id: String,
-        source: rusoto_core::RusotoError<rusoto_kms::CreateKeyError>,
-        backtrace: Backtrace,
-    },
-
-    #[snafu(display(
-    "Failed to assign alias {} for created customer managed key_id {} for aws-kms://{}/{}: {}",
-    alias,
-    target_key_id,
-    profile.as_deref().unwrap_or(""),
-    key_id,
-    source,
-    ))]
-    KmsCreateAlias {
-        alias: String,
-        target_key_id: String,
-        profile: Option<String>,
-        key_id: String,
-        source: rusoto_core::RusotoError<rusoto_kms::CreateAliasError>,
-        backtrace: Backtrace,
-    },
-
+    /// The library failed to authenticate Aws account.
     #[snafu(display("Error creating AWS credentials provider: {}", source))]
     RusotoCreds {
         source: rusoto_credential::CredentialsError,
         backtrace: Backtrace,
     },
 
+    /// The library failed to get the region for the given profile.
     #[snafu(display("Unable to determine region from profile '{}': {}", profile, source))]
     RusotoRegionFromProfile {
         profile: String,
         source: rusoto_credential::CredentialsError,
         backtrace: Backtrace,
     },
 
-    #[snafu(display("Unknown AWS region \"{}\": {}", region, source))]
+    /// The library failed to identify the region obtained from the given profile.
+    #[snafu(display("Unknown AWS region '{}': {}", region, source))]
     RusotoRegion {
         region: String,
         source: rusoto_core::region::ParseRegionError,
         backtrace: Backtrace,
     },
 
+    /// The library failed to instantiate 'HttpClient'.
     #[snafu(display("Error creating AWS request dispatcher: {}", source))]
     RusotoTls {
         source: rusoto_core::request::TlsError,
         backtrace: Backtrace,
     },
 
+    /// The library failed to instantiate 'tokio Runtime'.
     #[snafu(display("Unable to create tokio runtime: {}", source))]
     RuntimeCreation {
         source: std::io::Error,
         backtrace: Backtrace,
     },
 
+    /// The library failed to get public key from Awm kms
     #[snafu(display(
     "Failed to get public key for aws-kms://{}/{} : {}",
     profile.as_deref().unwrap_or(""),
@@ -84,38 +67,20 @@ pub enum Error {
         backtrace: Backtrace,
     },
 
-    #[snafu(display(
-    "Alias {} exist for aws-kms:://{}/{}",
-    key_id,
-    profile.as_deref().unwrap_or(""),
-    key_id
-    ))]
-    KmsAliasCheck {
-        profile: Option<String>,
-        key_id: String,
-    },
-
-    #[snafu(display("Public key is none"))]
+    /// Empty public key was returned by AWS KMS
+    #[snafu(display("Public key does not exist"))]
     PublicKeyNone,
 
-    #[snafu(display(
-    "Write in Not defined for aws-kms://{}/{}",
-    profile.as_deref().unwrap_or(""),
-    key_id
-    ))]
-    WriteNotDefined {
-        profile: Option<String>,
+    /// The library failed to get the message signature from AWS KMS
+    #[snafu(display("Error while signing message for aws-kms://{}/{} : {}", profile.as_deref().unwrap_or(""), key_id, source))]
+    KmsSignMessage {
         key_id: String,
-    },
-
-    #[snafu(display(
-    "Key metatada is None while creating key for aws-kms://{}/{}",
-    profile.as_deref().unwrap_or(""),
-    key_id
-    ))]
-    KeyMatadata {
         profile: Option<String>,
-        key_id: String,
+        source: rusoto_core::RusotoError<rusoto_kms::SignError>,
         backtrace: Backtrace,
     },
+
+    /// Empty signature was returned by AWS KMS
+    #[snafu(display("Empty signature returned by AWS KMS"))]
+    SignatureNotFound,
 }

tough-kms/src/lib.rs

@@ -1,20 +1,51 @@
 // Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
+//! tough-kms implements the `KeySource` trait found in [tough, a Rust TUF client](https://github.com/awslabs/tough).
+//!
+//! By implementing this trait, AWS KMS can become a source of keys used to sign a [TUF repository](https://theupdateframework.github.io/).
+//!
+//! # Testing
+//!
+//! Unit tests are run in the usual manner: `cargo test`.
+
+#![forbid(missing_debug_implementations, missing_copy_implementations)]
+#![deny(rust_2018_idioms)]
+// missing_docs is on its own line to make it easy to comment out when making changes.
+#![deny(missing_docs)]
+#![warn(clippy::pedantic)]
+#![allow(
+    clippy::module_name_repetitions,
+    clippy::must_use_candidate,
+    clippy::missing_errors_doc
+)]
+
 mod client;
 pub mod error;
-use rusoto_kms::{Kms, KmsClient};
+use ring::rand::SecureRandom;
+use rusoto_kms::{Kms, KmsClient, SignRequest};
 use snafu::{OptionExt, ResultExt};
+use std::collections::HashMap;
 use std::fmt;
 use tough::key_source::KeySource;
 use tough::schema::decoded::{Decoded, RsaPem};
-use tough::schema::key::KmsSigningAlgorithms;
-use tough::sign::{KmsRsaKey, Sign};
+use tough::schema::key::{Key, RsaKey, RsaScheme};
+use tough::sign::Sign;
+
+/// Represents a Signing Algorithms for AWS KMS.
+#[derive(Debug, Clone, PartialEq)]
+pub enum KmsSigningAlgorithms {
+    /// The key type
+    Rsa(String),
+}
 
-/// Implements the KeySource trait for keys that live in AWS KMS
+/// Implements the `KeySource` trait for keys that live in AWS KMS
 pub struct KmsKeySource {
+    /// Identifies AWS account named profile, if not provided default AWS profile is used.
     pub profile: Option<String>,
+    /// Identifies an asymmetric CMK in AWS KMS.
     pub key_id: String,
+    /// KmsClient Object to query AWS KMS
     pub client: Option<KmsClient>,
 }
 
@@ -27,13 +58,13 @@ impl fmt::Debug for KmsKeySource {
     }
 }
 
-/// Implements the KeySource trait.
+/// Implement the `KeySource` trait.
 impl KeySource for KmsKeySource {
     fn as_sign(
         &self,
     ) -> std::result::Result<Box<dyn Sign>, Box<dyn std::error::Error + Send + Sync + 'static>>
     {
-        let kms_client = match self.client.clone() {
+        let kms_client = match self.client.to_owned() {
             Some(value) => value,
             None => client::build_client_kms(self.profile.as_deref())?,
         };
@@ -55,7 +86,8 @@ impl KeySource for KmsKeySource {
             .to_vec()
             .into();
         Ok(Box::new(KmsRsaKey {
-            kms_client: kms_client.clone(),
+            profile: self.profile.clone(),
+            client: Some(kms_client.clone()),
             key_id: self.key_id.clone(),
             public_key: pb_key,
             signing_algorithm: KmsSigningAlgorithms::Rsa(String::from("RSASSA_PSS_SHA_256")),
@@ -64,76 +96,76 @@ impl KeySource for KmsKeySource {
 
     fn write(
         &self,
-        value: &str,
-    ) -> std::result::Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
-        let kms_client = match self.client.clone() {
-            Some(value) => value,
-            None => client::build_client_kms(self.profile.as_deref())?,
-        };
-        // Assign an alias to the Key
-        let fut = kms_client.create_alias(rusoto_kms::CreateAliasRequest {
-            alias_name: self.key_id.clone(),
-            target_key_id: value.to_string(),
-        });
-        let _response = tokio::runtime::Runtime::new()
-            .context(error::RuntimeCreation)?
-            .block_on(fut)
-            .context(error::KmsCreateAlias {
-                alias: self.key_id.clone(),
-                target_key_id: value.to_string(),
-                profile: self.profile.clone(),
-                key_id: self.key_id.clone(),
-            })?;
+        _value: &str,
+        _key_id_hex: &str,
+    ) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
         Ok(())
     }
+}
+
+/// Implements the Sign trait for KMS rsa Key
+pub struct KmsRsaKey {
+    /// Key Id of Customer Managed Key in KMS used to sign the message
+    pub key_id: String,
+    /// Aws account profile
+    pub profile: Option<String>,
+    /// KmsClient Object to query AWS KMS
+    pub client: Option<KmsClient>,
+    /// Public Key corresponding to Customer Managed Key
+    pub public_key: Decoded<RsaPem>,
+    /// Signing Algorithm to be used for the Customer Managed Key
+    pub signing_algorithm: KmsSigningAlgorithms,
+}
 
-    fn create(
+impl fmt::Debug for KmsRsaKey {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("KmsRsaKey")
+            .field("key_id", &self.key_id)
+            .field("signing_algorithm", &self.signing_algorithm)
+            .field("public_key", &self.public_key)
+            .finish()
+    }
+}
+
+impl Sign for KmsRsaKey {
+    fn tuf_key(&self) -> Key {
+        // Create a Key struct for the public key
+        Key::Rsa {
+            keyval: RsaKey {
+                public: self.public_key.to_owned(),
+                _extra: HashMap::new(),
+            },
+            scheme: RsaScheme::RsassaPssSha256,
+            _extra: HashMap::new(),
+        }
+    }
+
+    fn sign(
         &self,
-        _bits: u16,
-        _exponent: u32,
-    ) -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
-        let kms_client = match self.client.clone() {
+        msg: &[u8],
+        _rng: &dyn SecureRandom,
+    ) -> Result<Vec<u8>, Box<dyn std::error::Error + Send + Sync + 'static>> {
+        let kms_client = match self.client.to_owned() {
             Some(value) => value,
             None => client::build_client_kms(self.profile.as_deref())?,
         };
-        // Kms currently does not have an API to validate if the alias exist. As a Hack,
-        // or temporary resolution, we are using the get_public_key API, if it succeeds than
-        // Alias exist and alarm the user.
-        // TODO: replace with correct API when available
-        let fut = kms_client.get_public_key(rusoto_kms::GetPublicKeyRequest {
+        let sign_fut = kms_client.sign(SignRequest {
             key_id: self.key_id.clone(),
-            ..rusoto_kms::GetPublicKeyRequest::default()
+            message: bytes::Bytes::from(msg.to_owned()),
+            message_type: Some(String::from("RAW")),
+            signing_algorithm: match self.signing_algorithm.clone() {
+                KmsSigningAlgorithms::Rsa(algorithm) => algorithm,
+            },
+            ..rusoto_kms::SignRequest::default()
         });
         let response = tokio::runtime::Runtime::new()
             .context(error::RuntimeCreation)?
-            .block_on(fut);
-        if response.is_ok() {
-            error::KmsAliasCheck {
-                profile: self.profile.clone(),
-                key_id: self.key_id.clone(),
-            }
-            .fail()?;
-        }
-        // Create a new Customer managed Key in KMS
-        let fut = kms_client.create_key(rusoto_kms::CreateKeyRequest {
-            customer_master_key_spec: Some(String::from("RSA_2048")),
-            description: Some(String::from("Tuf repository signing key")),
-            key_usage: Some(String::from("SIGN_VERIFY")),
-            ..rusoto_kms::CreateKeyRequest::default()
-        });
-        let key_id = tokio::runtime::Runtime::new()
-            .context(error::RuntimeCreation)?
-            .block_on(fut)
-            .context(error::KmsCreateKey {
+            .block_on(sign_fut)
+            .context(error::KmsSignMessage {
                 profile: self.profile.clone(),
                 key_id: self.key_id.clone(),
-            })?
-            .key_metadata
-            .context(error::KeyMatadata {
-                profile: self.profile.clone(),
-                key_id: self.key_id.clone(),
-            })?
-            .key_id;
-        self.write(&key_id)
+            })?;
+        let signature = response.signature.context(error::SignatureNotFound)?;
+        Ok(signature.to_vec())
     }
 }