Fixed PR comments and Test case after signature verification fix

tough-kms/src/error.rs

@@ -87,4 +87,14 @@ pub enum Error {
     /// Empty signature was returned by AWS KMS
     #[snafu(display("Empty signature returned by AWS KMS"))]
     SignatureNotFound,
+
+    /// Provided signing algorithm is not valid
+    #[snafu(display("Please provide valid signing algorithm"))]
+    ValidSignAlgorithm,
+
+    /// Supported signing algorithm list is missing for CMK in AWS KMS
+    #[snafu(display(
+        "Found public key from AWS KMS, but list of supported signing algorithm is missing"
+    ))]
+    MissingSignAlgorithm,
 }

tough-kms/src/lib.rs

@@ -34,20 +34,42 @@ 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),
+#[non_exhaustive]
+#[derive(Debug, Clone, PartialEq, Copy)]
+pub enum KmsSigningAlgorithm {
+    /// Signing Algorithm `RSASSA_PSS_SHA_256`
+    RsassaPssSha256,
+}
+
+impl Default for KmsSigningAlgorithm {
+    fn default() -> Self {
+        KmsSigningAlgorithm::RsassaPssSha256
+    }
+}
+
+impl KmsSigningAlgorithm {
+    fn value(self) -> String {
+        // Currently we are supporting only single algorithm, but code stub is added to support
+        // multiple algorithms in future.
+        match self {
+            #![allow(clippy::wildcard_in_or_patterns)]
+            #[allow(unreachable_patterns)]
+            KmsSigningAlgorithm::RsassaPssSha256 | _ => String::from("RSASSA_PSS_SHA_256"),
+        }
+    }
 }
 
 /// Implements the `KeySource` trait for keys that live in AWS KMS
+#[derive(Default)]
 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>,
+    /// Signing Algorithm to be used for the message digest, only `KmsSigningAlgorithm::RsassaPssSha256` is supported at present.
+    pub signing_algorithm: KmsSigningAlgorithm,
 }
 
 impl fmt::Debug for KmsKeySource {
@@ -90,12 +112,19 @@ impl KeySource for KmsKeySource {
                 line_ending: pem::LineEnding::LF,
             },
         );
+        if !response
+            .signing_algorithms
+            .context(error::MissingSignAlgorithm)?
+            .contains(&self.signing_algorithm.value())
+        {
+            error::ValidSignAlgorithm.fail()?;
+        }
         Ok(Box::new(KmsRsaKey {
             profile: self.profile.clone(),
             client: Some(kms_client.clone()),
             key_id: self.key_id.clone(),
             public_key: key.parse().context(error::PublicKeyParse)?,
-            signing_algorithm: KmsSigningAlgorithms::Rsa(String::from("RSASSA_PSS_SHA_256")),
+            signing_algorithm: self.signing_algorithm.value(),
         }))
     }
 
@@ -119,7 +148,7 @@ pub struct KmsRsaKey {
     /// Public Key corresponding to Customer Managed Key
     public_key: Decoded<RsaPem>,
     /// Signing Algorithm to be used for the Customer Managed Key
-    signing_algorithm: KmsSigningAlgorithms,
+    signing_algorithm: String,
 }
 
 impl fmt::Debug for KmsRsaKey {
@@ -158,9 +187,7 @@ impl Sign for KmsRsaKey {
             key_id: self.key_id.clone(),
             message: digest(&SHA256, msg).as_ref().to_vec().into(),
             message_type: Some(String::from("DIGEST")),
-            signing_algorithm: match self.signing_algorithm.clone() {
-                KmsSigningAlgorithms::Rsa(algorithm) => algorithm,
-            },
+            signing_algorithm: self.signing_algorithm.clone(),
             ..rusoto_kms::SignRequest::default()
         });
         let response = tokio::runtime::Runtime::new()

tough-kms/tests/all_test.rs

@@ -8,11 +8,12 @@ use ring::rand::SystemRandom;
 use rusoto_core::signature::SignedRequest;
 use rusoto_core::{HttpDispatchError, Region};
 use serde::Deserialize;
-use std::collections::HashMap;
 use std::fs::File;
 use std::io::BufReader;
 use tough::key_source::KeySource;
-use tough::schema::key::{Key, RsaKey, RsaScheme};
+use tough::schema::decoded::{Decoded, RsaPem};
+use tough::schema::key::Key;
+use tough_kms::KmsKeySource;
 
 #[derive(Default, Debug, Clone, PartialEq, Deserialize)]
 struct PublicKeyResp {
@@ -26,6 +27,11 @@ struct PublicKeyResp {
     public_key: bytes::Bytes,
 }
 
+#[derive(Deserialize, Debug)]
+struct ExpectedPublicKey {
+    public_key: Decoded<RsaPem>,
+}
+
 #[derive(Default, Debug, Clone, PartialEq, Deserialize)]
 struct SignResp {
     #[serde(rename = "Signature")]
@@ -50,17 +56,15 @@ struct CreateKeyResp {
 fn check_tuf_key_success() {
     let input = "response_public_key.json";
     let key_id = String::from("alias/some_alias");
-    let file = File::open(test_utils::test_data().join(input).to_str().unwrap()).unwrap();
+    let file = File::open(
+        test_utils::test_data()
+            .join("expected_public_key.json")
+            .to_str()
+            .unwrap(),
+    )
+    .unwrap();
     let reader = BufReader::new(file);
-    let expected_json: PublicKeyResp = serde_json::from_reader(reader).unwrap();
-    let expected_key = Key::Rsa {
-        keyval: RsaKey {
-            public: expected_json.public_key.to_vec().into(),
-            _extra: HashMap::new(),
-        },
-        scheme: RsaScheme::RsassaPssSha256,
-        _extra: HashMap::new(),
-    };
+    let expected_key: Key = serde_json::from_reader(reader).unwrap();
     let mock = MockRequestDispatcher::default()
         .with_request_checker(|request: &SignedRequest| {
             assert!(request
@@ -74,10 +78,11 @@ fn check_tuf_key_success() {
                 .as_ref(),
         );
     let mock_client = KmsClient::new_with(mock, MockCredentialsProvider, Region::UsEast1);
-    let kms_key = tough_kms::KmsKeySource {
+    let kms_key = KmsKeySource {
         profile: None,
         key_id: key_id.clone(),
         client: Some(mock_client),
+        ..KmsKeySource::default()
     };
     let sign = kms_key.as_sign().unwrap();
     let key = sign.tuf_key();
@@ -133,10 +138,11 @@ fn check_sign_success() {
             ),
     ]);
     let mock_client = KmsClient::new_with(mock, MockCredentialsProvider, Region::UsEast1);
-    let kms_key = tough_kms::KmsKeySource {
+    let kms_key = KmsKeySource {
         profile: None,
         key_id: String::from("alias/some_alias"),
         client: Some(mock_client),
+        ..KmsKeySource::default()
     };
     let rng = SystemRandom::new();
     let kms_sign = kms_key.as_sign().unwrap();
@@ -154,10 +160,11 @@ fn check_public_key_failure() {
         MockRequestDispatcher::with_dispatch_error(HttpDispatchError::new(error_msg.clone()));
     let client = KmsClient::new_with(mock, MockCredentialsProvider, Region::UsEast1);
     let key_id = String::from("alias/some_alias");
-    let kms_key = tough_kms::KmsKeySource {
+    let kms_key = KmsKeySource {
         profile: None,
         key_id: key_id.clone(),
         client: Some(client),
+        ..KmsKeySource::default()
     };
     let result = kms_key.as_sign();
     assert!(result.is_err());
@@ -172,6 +179,70 @@ fn check_public_key_failure() {
     );
 }
 
+#[test]
+// Ensure call to as_sign fails when signing algorithms are missing in get_public_key response
+fn check_public_key_missing_algo() {
+    let input = "response_public_key_no_algo.json";
+    let key_id = String::from("alias/some_alias");
+    let mock = MockRequestDispatcher::default()
+        .with_request_checker(|request: &SignedRequest| {
+            assert!(request
+                .headers
+                .get("x-amz-target")
+                .unwrap()
+                .contains(&Vec::from("TrentService.GetPublicKey")));
+        })
+        .with_body(
+            MockResponseReader::read_response(test_utils::test_data().to_str().unwrap(), input)
+                .as_ref(),
+        );
+    let mock_client = KmsClient::new_with(mock, MockCredentialsProvider, Region::UsEast1);
+    let kms_key = KmsKeySource {
+        profile: None,
+        key_id: key_id.clone(),
+        client: Some(mock_client),
+        ..KmsKeySource::default()
+    };
+    let err = kms_key.as_sign().err().unwrap();
+    assert_eq!(
+        String::from(
+            "Found public key from AWS KMS, but list of supported signing algorithm is missing"
+        ),
+        err.to_string()
+    );
+}
+
+#[test]
+// Ensure call to as_sign fails when provided signing algorithm does not match
+fn check_public_key_unmatch_algo() {
+    let input = "response_public_key_unmatch_algo.json";
+    let key_id = String::from("alias/some_alias");
+    let mock = MockRequestDispatcher::default()
+        .with_request_checker(|request: &SignedRequest| {
+            assert!(request
+                .headers
+                .get("x-amz-target")
+                .unwrap()
+                .contains(&Vec::from("TrentService.GetPublicKey")));
+        })
+        .with_body(
+            MockResponseReader::read_response(test_utils::test_data().to_str().unwrap(), input)
+                .as_ref(),
+        );
+    let mock_client = KmsClient::new_with(mock, MockCredentialsProvider, Region::UsEast1);
+    let kms_key = KmsKeySource {
+        profile: None,
+        key_id: key_id.clone(),
+        client: Some(mock_client),
+        ..KmsKeySource::default()
+    };
+    let err = kms_key.as_sign().err().unwrap();
+    assert_eq!(
+        String::from("Please provide valid signing algorithm"),
+        err.to_string()
+    );
+}
+
 #[test]
 // Ensure sign error when Kms fails to sign message.
 fn check_sign_request_failure() {
@@ -204,10 +275,11 @@ fn check_sign_request_failure() {
             }),
     ]);
     let mock_client = KmsClient::new_with(mock, MockCredentialsProvider, Region::UsEast1);
-    let kms_key = tough_kms::KmsKeySource {
+    let kms_key = KmsKeySource {
         profile: None,
         key_id: key_id.clone(),
         client: Some(mock_client),
+        ..KmsKeySource::default()
     };
     let rng = SystemRandom::new();
     let kms_sign = kms_key.as_sign().unwrap();
@@ -263,10 +335,11 @@ fn check_signature_failure() {
             ),
     ]);
     let mock_client = KmsClient::new_with(mock, MockCredentialsProvider, Region::UsEast1);
-    let kms_key = tough_kms::KmsKeySource {
+    let kms_key = KmsKeySource {
         profile: None,
         key_id: key_id.clone(),
         client: Some(mock_client),
+        ..KmsKeySource::default()
     };
     let rng = SystemRandom::new();
     let kms_sign = kms_key.as_sign().unwrap();
@@ -278,3 +351,15 @@ fn check_signature_failure() {
         err.to_string()
     );
 }
+
+#[test]
+fn check_write_ok() {
+    let key_id = String::from("alias/some_alias");
+    let kms_key = KmsKeySource {
+        profile: None,
+        key_id: key_id.clone(),
+        client: None,
+        ..KmsKeySource::default()
+    };
+    assert_eq!((), kms_key.write("", "").unwrap())
+}

tough-kms/tests/data/expected_public_key.json

@@ -0,0 +1,7 @@
+{
+  "keytype": "rsa",
+  "keyval": {
+    "public": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwFWHzkki38xzt2d/jECK\nNVdHe/y1o6dPcdwUz07bnbtVKyT1Cw5iemChnIGwUNpsqmrOov2+wetWEW/zrCeH\npQtqBZFeSo689N99WVFSA48xWEYLHutYY0eOSQ+RZzrLXaAPnXlu93F8icLNHBU3\n7fSd8t5mpoj6tIn9M50mRp1qk1qJpgajB6a2W+T1yMur2kQkqzJLZQg4kxqXDCdg\ngxb8DWdRnT/m2K9b/RziwqGJbNPnxGe3A0n8HCNOF1IuTJeKn8vbQMoP3cXE6+TI\nNmZzU28IdiNDvN5+zlHFoxsNRL/yHJPX/8PVGNu+Gg2xBfmnYYzSR9hm9O1GBLix\nkwIDAQAB\n-----END PUBLIC KEY-----\n"
+  },
+  "scheme": "rsassa-pss-sha256"
+}

tough-kms/tests/data/response_public_key.json

@@ -1,3 +1,14 @@
 {
-  "PublicKey" : "Ii0tLS0tQkVHSU4gUFVCTElDIEtFWS0tLS0tXG5NSUlCb2pBTkJna3Foa2lHOXcwQkFRRUZBQU9DQVk4QU1JSUJpZ0tDQVlFQW5MNnU2UTlRNnBnMUc1MDIwYTgzXG5HbEgvYUZVTzBQUTVsZUlwd1dMOGtXZ3BhV3VVRzdvUmxPVUcyLzRjd041RkN2SkpHWHFVNUF0U0txMmZaNDJKXG41WFI5UU1pcDRQZzBRNm1FOFhDdkFYQW9NbmtXU2NoZHpnVDJHb0VudGFPZVJSVENVR2IvRHNWb3hzVlhqVjZtXG5GYVJNeDduaDhnZ3NoTVdnVFlnVFVESytDU0lCQ2NCV2FwQ0ZxMUJyTTYwWFptR1RxZUF1SFNIYVVVdUY5RzNiXG5nT2ZsSDVMOUlwUWthSFdiSnRHdnlLTHI1M21oV08ycjhCUFIzK0N0TlpvakFua3dtdTRsQTk0azhDN1RMTWRjXG51dHpVNE96T0RlOVVQRVJjMzNsUnY4REJnc0gzRjA3N1pRd3YvaWtaWFdTbEFDVERXWndlbm5jQ0V3cWRlRGQ0XG4rcTJBSHlxeFJON2JVQWg1N21VTitrRmQzU1MvNFQ0NHNmQnJKdzZONEpWL21FKy9ZZlJMV3RwSUtJc1huQkNiXG5yQytkdDk2VnF6Nmc2ZVZWdnFQd2hPQ1NLY1lzbXAvaVM2cXdWbjBEcTJTQ3JHRzFGVG1CamVBOVprY2paaFVHXG5RRU15TU5ob1MrVTJOeDVvSUVJcTJrUkVwdXUrS3NCU1RVYU9nUjA3V05VeEFnTUJBQUU9XG4tLS0tLUVORCBQVUJMSUMgS0VZLS0tLS1cbiI="
+  "KeyId": "arn:aws:kms:us-west-2:062205370538:key/3bbf2655-2dff-4040-ad37-2ec8f60d651b",
+  "PublicKey": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwFWHzkki38xzt2d/jECKNVdHe/y1o6dPcdwUz07bnbtVKyT1Cw5iemChnIGwUNpsqmrOov2+wetWEW/zrCeHpQtqBZFeSo689N99WVFSA48xWEYLHutYY0eOSQ+RZzrLXaAPnXlu93F8icLNHBU37fSd8t5mpoj6tIn9M50mRp1qk1qJpgajB6a2W+T1yMur2kQkqzJLZQg4kxqXDCdggxb8DWdRnT/m2K9b/RziwqGJbNPnxGe3A0n8HCNOF1IuTJeKn8vbQMoP3cXE6+TINmZzU28IdiNDvN5+zlHFoxsNRL/yHJPX/8PVGNu+Gg2xBfmnYYzSR9hm9O1GBLixkwIDAQAB",
+  "CustomerMasterKeySpec": "RSA_2048",
+  "KeyUsage": "SIGN_VERIFY",
+  "SigningAlgorithms": [
+    "RSASSA_PKCS1_V1_5_SHA_256",
+    "RSASSA_PKCS1_V1_5_SHA_384",
+    "RSASSA_PKCS1_V1_5_SHA_512",
+    "RSASSA_PSS_SHA_256",
+    "RSASSA_PSS_SHA_384",
+    "RSASSA_PSS_SHA_512"
+  ]
 }

tough-kms/tests/data/response_public_key_no_algo.json

@@ -0,0 +1,6 @@
+{
+  "KeyId": "arn:aws:kms:us-west-2:062205370538:key/3bbf2655-2dff-4040-ad37-2ec8f60d651b",
+  "PublicKey": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwFWHzkki38xzt2d/jECKNVdHe/y1o6dPcdwUz07bnbtVKyT1Cw5iemChnIGwUNpsqmrOov2+wetWEW/zrCeHpQtqBZFeSo689N99WVFSA48xWEYLHutYY0eOSQ+RZzrLXaAPnXlu93F8icLNHBU37fSd8t5mpoj6tIn9M50mRp1qk1qJpgajB6a2W+T1yMur2kQkqzJLZQg4kxqXDCdggxb8DWdRnT/m2K9b/RziwqGJbNPnxGe3A0n8HCNOF1IuTJeKn8vbQMoP3cXE6+TINmZzU28IdiNDvN5+zlHFoxsNRL/yHJPX/8PVGNu+Gg2xBfmnYYzSR9hm9O1GBLixkwIDAQAB",
+  "CustomerMasterKeySpec": "RSA_2048",
+  "KeyUsage": "SIGN_VERIFY"
+}

tough-kms/tests/data/response_public_key_unmatch_algo.json

@@ -0,0 +1,13 @@
+{
+  "KeyId": "arn:aws:kms:us-west-2:062205370538:key/3bbf2655-2dff-4040-ad37-2ec8f60d651b",
+  "PublicKey": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwFWHzkki38xzt2d/jECKNVdHe/y1o6dPcdwUz07bnbtVKyT1Cw5iemChnIGwUNpsqmrOov2+wetWEW/zrCeHpQtqBZFeSo689N99WVFSA48xWEYLHutYY0eOSQ+RZzrLXaAPnXlu93F8icLNHBU37fSd8t5mpoj6tIn9M50mRp1qk1qJpgajB6a2W+T1yMur2kQkqzJLZQg4kxqXDCdggxb8DWdRnT/m2K9b/RziwqGJbNPnxGe3A0n8HCNOF1IuTJeKn8vbQMoP3cXE6+TINmZzU28IdiNDvN5+zlHFoxsNRL/yHJPX/8PVGNu+Gg2xBfmnYYzSR9hm9O1GBLixkwIDAQAB",
+  "CustomerMasterKeySpec": "RSA_2048",
+  "KeyUsage": "SIGN_VERIFY",
+  "SigningAlgorithms": [
+    "RSASSA_PKCS1_V1_5_SHA_256",
+    "RSASSA_PKCS1_V1_5_SHA_384",
+    "RSASSA_PKCS1_V1_5_SHA_512",
+    "RSASSA_PSS_SHA_384",
+    "RSASSA_PSS_SHA_512"
+  ]
+}

tuftool/Cargo.toml

@@ -9,7 +9,7 @@ keywords = ["tuf", "update", "repository"]
 edition = "2018"
 
 [features]
-integration-tests = []
+integ = []
 default = ["rusoto"]
 rusoto = ["rusoto-rustls"]
 rusoto-native-tls = ["rusoto_core/native-tls", "rusoto_credential", "rusoto_ssm/native-tls", "rusoto_kms/native-tls"]

tuftool/README.md

@@ -1 +1,8 @@
 **tuftool** is a Rust command-line utility for generating and signing TUF repositories.
+
+
+## Testing
+
+Unit tests are run in the usual manner: `cargo test`.
+Integration tests require working AWS credentials and are disabled by default behind a feature named `integ`.
+To run all tests, including integration tests: `cargo test --features 'integ'` or `AWS_PROFILE=test-profile cargo test --features 'integ'` with specific profile.

tuftool/src/source.rs

@@ -91,6 +91,7 @@ pub(crate) fn parse_key_source(input: &str) -> Result<Box<dyn KeySource>> {
             // remove first '/' from the path to get the key_id
             key_id: url.path()[1..].to_string(),
             client: None,
+            ..KmsKeySource::default()
         })),
         _ => error::UnrecognizedScheme {
             scheme: url.scheme(),