docs(aws-kms-tls-auth): clarify security impact of failure modes (#5424)

Author: jmayclin

bindings/rust/aws-kms-tls-auth/DEVELOPMENT.md

@@ -1,7 +1,7 @@
 # Managing Algorithm & Format Changes.
 
 ## Backwards Compatibility: Required
-Changes must always be backwards compatible. More specifically, server's must always be able to deserialize earlier versions of PskIdentities, otherwise all of the in-flight communications would fail when an upgrade happens.
+Changes must always be backwards compatible. More specifically, servers must always be able to deserialize earlier versions of PskIdentities, otherwise all of the in-flight communications would fail when an upgrade happens.
 
 ## Forward Compatibility: Customer Responsibility
 We generally do not promise forwards compatibility: A `0.0.1` V1 enabled server might not be able to handshake with a `0.0.2` V2 enabled client. We will strive to maintain forward compatibility, but if there was ever an upgrade from `AES_256_GCM_SIV` to `AES_512_GCM_SIV`, that would not be forward compatible. 

bindings/rust/aws-kms-tls-auth/src/identity.rs

@@ -84,19 +84,17 @@ impl ObfuscationKey {
     }
 
     fn aes_256_key(&self) -> anyhow::Result<LessSafeKey> {
+        // The "LessSafe" key refers to the fact that we have to create our own
+        // random nonces. A feature request is open to aws-lc-rs to support
+        // AES-256-GCM-SIV for the safer RandomizedNonceKey.
+        // https://github.com/aws/aws-lc-rs/issues/842
         Ok(LessSafeKey::new(UnboundKey::new(
             &AES_256_GCM_SIV,
             &self.material,
         )?))
     }
 }
 
-#[derive(Debug, Clone)]
-pub(crate) struct KmsDataKey {
-    pub ciphertext: Vec<u8>,
-    pub plaintext: Vec<u8>,
-}
-
 #[derive(Debug, Clone, Hash, PartialEq, Eq)]
 pub(crate) struct PskIdentity {
     version: PskVersion,

bindings/rust/aws-kms-tls-auth/src/provider.rs

@@ -3,7 +3,7 @@
 
 use crate::{
     codec::EncodeValue,
-    identity::{KmsDataKey, ObfuscationKey, PskIdentity, PskVersion},
+    identity::{ObfuscationKey, PskIdentity, PskVersion},
     psk_from_material, KeyArn, KEY_ROTATION_PERIOD, PSK_SIZE,
 };
 use aws_sdk_kms::Client;
@@ -15,6 +15,12 @@ use std::{
 };
 use tokio::time::Instant;
 
+#[derive(Debug, Clone)]
+struct KmsDataKey {
+    pub ciphertext: Vec<u8>,
+    pub plaintext: Vec<u8>,
+}
+
 /// The `PskProvider` is used along with the [`PskReceiver`] to perform TLS
 /// 1.3 out-of-band PSK authentication, using PSK's generated from KMS.
 ///
@@ -25,7 +31,15 @@ use tokio::time::Instant;
 ///
 /// Note that the "rotation check" only happens when a new connection is created.
 /// So if a new connection is only created every 2 hours, rotation might not be
-/// attempted until 26 hours have elapsed.
+/// attempted until 26 hours have elapsed. This results in a 26 hour old PSK being
+/// used for the connection.
+///
+/// ### ⚠️ WARNING ⚠️
+/// Because of the above behavior, this solution is not a good fit for
+/// extremely low tps scenarios. When performing ~ 1 connection per week or less,
+/// the low tps significantly slows key rotation. This does not cause any specific
+/// system failure, but long lived secrets do not align with cryptographic best
+/// practices.
 #[derive(Clone)]
 pub struct PskProvider {
     /// The version of PSK identities sent on the wire. Currently this is unused
@@ -79,6 +93,14 @@ impl PskProvider {
     ///     tracing::error!("failed to rotate key: {error}");
     /// });
     /// ```
+    ///
+    /// ### ⚠️ WARNING ⚠️
+    /// Failing to take action on the `failure_notification` will result in the
+    /// Provider continuing to use the same data key indefinitely. While this doesn't
+    /// cause any specific system failure, long lived secrets do not align with
+    /// cryptographic best practices. Longer lived secrets have a higher change
+    /// of exposure, so customers should ensure that they alarm and troubleshoot
+    /// rotation failures.
     pub async fn initialize(
         psk_version: PskVersion,
         kms_client: Client,