Merge pull request #59 from aws-samples/kyboxethdev

Escape user input and add additional security checks

Author: parimaldeshmukh

SecretsManagerMongoDBRotationMultiUser/lambda_function.py

@@ -154,16 +154,26 @@ def set_secret(service_client, arn, token):
 
     """
     # First try to login with the pending secret, if it succeeds, return
+    current_dict = get_secret_dict(service_client, arn, "AWSCURRENT")
     pending_dict = get_secret_dict(service_client, arn, "AWSPENDING", token)
     conn = get_connection(pending_dict)
     if conn:
         conn.logout()
         logger.info("setSecret: AWSPENDING secret is already set as password in MongoDB for secret arn %s." % arn)
         return
 
+    # Make sure the user from current and pending match
+    if get_alt_username(current_dict['username']) != pending_dict['username']:
+        logger.error("setSecret: Attempting to modify user %s other than current user or clone %s" % (pending_dict['username'], current_dict['username']))
+        raise ValueError("Attempting to modify user %s other than current user or clone %s" % (pending_dict['username'], current_dict['username']))
+
+    # Make sure the host from current and pending match
+    if current_dict['host'] != pending_dict['host']:
+        logger.error("setSecret: Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+        raise ValueError("Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+
     # Before we do anything with the secret, make sure the AWSCURRENT secret is valid by logging in to the db
     # This ensures that the credential we are rotating is valid to protect against a confused deputy attack
-    current_dict = get_secret_dict(service_client, arn, "AWSCURRENT")
     conn = get_connection(current_dict)
     if not conn:
         logger.error("setSecret: Unable to log into database using current credentials for secret %s" % arn)
@@ -174,7 +184,8 @@ def set_secret(service_client, arn, token):
     master_arn = current_dict['masterarn']
     master_dict = get_secret_dict(service_client, master_arn, "AWSCURRENT")
     if current_dict['host'] != master_dict['host']:
-        logger.warn("setSecret: Master database host %s is not the same host as current %s" % (master_dict['host'], current_dict['host']))
+        logger.error("setSecret: Current database host %s is not the same host as master %s" % (current_dict['host'], master_dict['host']))
+        raise ValueError("Current database host %s is not the same host as master %s" % (current_dict['host'], master_dict['host']))
 
     # Now log into the database with the master credentials
     conn = get_connection(master_dict)
@@ -191,6 +202,9 @@ def set_secret(service_client, arn, token):
             user_info = conn.command('usersInfo', current_dict['username'])
             conn.command("createUser", pending_dict['username'], pwd=pending_dict['password'], roles=user_info['users'][0]['roles'])
         logger.info("setSecret: Successfully set password for %s in MongoDB for secret arn %s." % (pending_dict['username'], arn))
+    except errors.PyMongoError:
+        logger.error("setSecret: Error encountered when attempting to set password in database for user %s", pending_dict['username'])
+        raise ValueError("Error encountered when attempting to set password in database for user %s", pending_dict['username'])
     finally:
         conn.logout()
 
@@ -288,9 +302,9 @@ def get_connection(secret_dict):
     ssl = False
     if 'ssl' in secret_dict:
         if type(secret_dict['ssl']) is bool:
-	        ssl = secret_dict['ssl']
-	    else:
-	        ssl = (secret_dict['ssl'].lower() == "true")
+            ssl = secret_dict['ssl']
+        else:
+            ssl = (secret_dict['ssl'].lower() == "true")
 
     # Try to obtain a connection to the db
     try:

SecretsManagerMongoDBRotationSingleUser/lambda_function.py

@@ -146,22 +146,43 @@ def set_secret(service_client, arn, token):
         KeyError: If the secret json does not contain the expected keys
 
     """
-    # First try to login with the pending secret, if it succeeds, return
+    try:
+        previous_dict = get_secret_dict(service_client, arn, "AWSPREVIOUS")
+    except (service_client.exceptions.ResourceNotFoundException, KeyError):
+        previous_dict = None
+    current_dict = get_secret_dict(service_client, arn, "AWSCURRENT")
     pending_dict = get_secret_dict(service_client, arn, "AWSPENDING", token)
+    
+    # First try to login with the pending secret, if it succeeds, return
     conn = get_connection(pending_dict)
     if conn:
         conn.logout()
         logger.info("setSecret: AWSPENDING secret is already set as password in MongoDB for secret arn %s." % arn)
         return
 
+    # Make sure the user from current and pending match
+    if current_dict['username'] != pending_dict['username']:
+        logger.error("setSecret: Attempting to modify user %s other than current user %s" % (pending_dict['username'], current_dict['username']))
+        raise ValueError("Attempting to modify user %s other than current user %s" % (pending_dict['username'], current_dict['username']))
+
+    # Make sure the host from current and pending match
+    if current_dict['host'] != pending_dict['host']:
+        logger.error("setSecret: Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+        raise ValueError("Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+
     # Now try the current password
-    conn = get_connection(get_secret_dict(service_client, arn, "AWSCURRENT"))
-    if not conn:
+    conn = get_connection(current_dict)
+    if not conn and previous_dict:
         # If both current and pending do not work, try previous
-        try:
-            conn = get_connection(get_secret_dict(service_client, arn, "AWSPREVIOUS"))
-        except service_client.exceptions.ResourceNotFoundException:
-            conn = None
+        conn = get_connection(previous_dict)
+
+        # Make sure the user/host from previous and pending match
+        if previous_dict['username'] != pending_dict['username']:
+            logger.error("setSecret: Attempting to modify user %s other than previous valid user %s" % (pending_dict['username'], previous_dict['username']))
+            raise ValueError("Attempting to modify user %s other than previous valid user %s" % (pending_dict['username'], previous_dict['username']))
+        if previous_dict['host'] != pending_dict['host']:
+            logger.error("setSecret: Attempting to modify user for host %s other than previous host %s" % (pending_dict['host'], previous_dict['host']))
+            raise ValueError("Attempting to modify user for host %s other than previous host %s" % (pending_dict['host'], previous_dict['host']))
 
     # If we still don't have a connection, raise a ValueError
     if not conn:
@@ -172,6 +193,9 @@ def set_secret(service_client, arn, token):
     try:
         conn.command("updateUser", pending_dict['username'], pwd=pending_dict['password'])
         logger.info("setSecret: Successfully set password for user %s in MongoDB for secret arn %s." % (pending_dict['username'], arn))
+    except errors.PyMongoError:
+        logger.error("setSecret: Error encountered when attempting to set password in database for user %s", pending_dict['username'])
+        raise ValueError("Error encountered when attempting to set password in database for user %s", pending_dict['username'])
     finally:
         conn.logout()
 
@@ -267,10 +291,10 @@ def get_connection(secret_dict):
     ssl = False
     if 'ssl' in secret_dict:
         if type(secret_dict['ssl']) is bool:
-	        ssl = secret_dict['ssl']
-	    else:
-	        ssl = (secret_dict['ssl'].lower() == "true")
-            
+            ssl = secret_dict['ssl']
+        else:
+            ssl = (secret_dict['ssl'].lower() == "true")
+        
     # Try to obtain a connection to the db
     try:
         client = MongoClient(host=secret_dict['host'], port=port, connectTimeoutMS=5000, serverSelectionTimeoutMS=5000, ssl=ssl)

SecretsManagerRDSMariaDBRotationMultiUser/lambda_function.py

@@ -152,17 +152,28 @@ def set_secret(service_client, arn, token):
         KeyError: If the secret json does not contain the expected keys
 
     """
-    # First try to login with the pending secret, if it succeeds, return
+    current_dict = get_secret_dict(service_client, arn, "AWSCURRENT")
     pending_dict = get_secret_dict(service_client, arn, "AWSPENDING", token)
+    
+    # First try to login with the pending secret, if it succeeds, return
     conn = get_connection(pending_dict)
     if conn:
         conn.close()
         logger.info("setSecret: AWSPENDING secret is already set as password in MariaDB DB for secret arn %s." % arn)
         return
 
+    # Make sure the user from current and pending match
+    if get_alt_username(current_dict['username']) != pending_dict['username']:
+        logger.error("setSecret: Attempting to modify user %s other than current user or clone %s" % (pending_dict['username'], current_dict['username']))
+        raise ValueError("Attempting to modify user %s other than current user or clone %s" % (pending_dict['username'], current_dict['username']))
+
+    # Make sure the host from current and pending match
+    if current_dict['host'] != pending_dict['host']:
+        logger.error("setSecret: Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+        raise ValueError("Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+
     # Before we do anything with the secret, make sure the AWSCURRENT secret is valid by logging in to the db
     # This ensures that the credential we are rotating is valid to protect against a confused deputy attack
-    current_dict = get_secret_dict(service_client, arn, "AWSCURRENT")
     conn = get_connection(current_dict)
     if not conn:
         logger.error("setSecret: Unable to log into database using current credentials for secret %s" % arn)
@@ -172,8 +183,10 @@ def set_secret(service_client, arn, token):
     # Now get the master arn from the current secret
     master_arn = current_dict['masterarn']
     master_dict = get_secret_dict(service_client, master_arn, "AWSCURRENT")
-    if current_dict['host'] != master_dict['host']:
-        logger.warn("setSecret: Master database host %s is not the same host as current %s" % (master_dict['host'], current_dict['host']))
+    if current_dict['host'] != master_dict['host'] and not is_rds_replica_database(current_dict, master_dict):
+        # If current dict is a replica of the master dict, can proceed
+        logger.error("setSecret: Current database host %s is not the same host as/rds replica of master %s" % (current_dict['host'], master_dict['host']))
+        raise ValueError("Current database host %s is not the same host as/rds replica of master %s" % (current_dict['host'], master_dict['host']))
 
     # Now log into the database with the master credentials
     conn = get_connection(master_dict)
@@ -189,9 +202,8 @@ def set_secret(service_client, arn, token):
             cur.execute("SHOW GRANTS FOR %s", current_dict['username'])
             for row in cur.fetchall():
                 grant = row[0].split(' TO ')
-                new_grant = "%s TO %s" % (grant[0], pending_dict['username'])
-                new_grant_escaped = new_grant.replace('%','%%') # % is a special character in Python format strings.
-                cur.execute(new_grant_escaped + " IDENTIFIED BY %s", pending_dict['password'])
+                new_grant_escaped = grant[0].replace('%','%%') # % is a special character in Python format strings.
+                cur.execute(new_grant_escaped + " TO %s IDENTIFIED BY %s", (pending_dict['username'], pending_dict['password']))
             conn.commit()
             logger.info("setSecret: Successfully set password for %s in MariaDB DB for secret arn %s." % (pending_dict['username'], arn))
     finally:
@@ -365,3 +377,42 @@ def get_alt_username(current_username):
         if len(new_username) > 80:
             raise ValueError("Unable to clone user, username length with _clone appended would exceed 80 characters")
         return new_username
+
+def is_rds_replica_database(replica_dict, master_dict):
+    """Validates that the database of a secret is a replica of the database of the master secret
+
+    This helper function validates that the database of a secret is a replica of the database of the master secret.
+
+    Args:
+        replica_dict (dictionary): The secret dictionary containing the replica database
+
+        primary_dict (dictionary): The secret dictionary containing the primary database
+
+    Returns:
+        isReplica : whether or not the database is a replica
+
+    Raises:
+        ValueError: If the new username length would exceed the maximum allowed
+    """
+    # Setup the client
+    rds_client = boto3.client('rds')
+
+    # Get instance identifiers from endpoints
+    replica_instance_id = replica_dict['host'].split(".")[0]
+    master_instance_id = master_dict['host'].split(".")[0]
+
+    try:
+        describe_response = rds_client.describe_db_instances(DBInstanceIdentifier=replica_instance_id)
+    except Exception as err:
+        logger.warn("Encountered error while verifying rds replica status: %s" % err)
+        return False
+    instances = describe_response['DBInstances']
+
+    # Host from current secret cannot be found
+    if not instances:
+        logger.info("Cannot verify replica status - no RDS instance found with identifier: %s" % replica_instance_id)
+        return False
+
+    # DB Instance identifiers are unique - can only be one result
+    current_instance = instances[0]
+    return master_instance_id == current_instance.get('ReadReplicaSourceDBInstanceIdentifier')

SecretsManagerRDSMariaDBRotationSingleUser/lambda_function.py

@@ -145,22 +145,43 @@ def set_secret(service_client, arn, token):
         KeyError: If the secret json does not contain the expected keys
 
     """
-    # First try to login with the pending secret, if it succeeds, return
+    try:
+        previous_dict = get_secret_dict(service_client, arn, "AWSPREVIOUS")
+    except (service_client.exceptions.ResourceNotFoundException, KeyError):
+        previous_dict = None
+    current_dict = get_secret_dict(service_client, arn, "AWSCURRENT")
     pending_dict = get_secret_dict(service_client, arn, "AWSPENDING", token)
+
+    # First try to login with the pending secret, if it succeeds, return
     conn = get_connection(pending_dict)
     if conn:
         conn.close()
         logger.info("setSecret: AWSPENDING secret is already set as password in MariaDB DB for secret arn %s." % arn)
         return
 
+    # Make sure the user from current and pending match
+    if current_dict['username'] != pending_dict['username']:
+        logger.error("setSecret: Attempting to modify user %s other than current user %s" % (pending_dict['username'], current_dict['username']))
+        raise ValueError("Attempting to modify user %s other than current user %s" % (pending_dict['username'], current_dict['username']))
+
+    # Make sure the host from current and pending match
+    if current_dict['host'] != pending_dict['host']:
+        logger.error("setSecret: Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+        raise ValueError("Attempting to modify user for host %s other than current host %s" % (pending_dict['host'], current_dict['host']))
+
     # Now try the current password
-    conn = get_connection(get_secret_dict(service_client, arn, "AWSCURRENT"))
-    if not conn:
+    conn = get_connection(current_dict)
+    if not conn and previous_dict:
         # If both current and pending do not work, try previous
-        try:
-            conn = get_connection(get_secret_dict(service_client, arn, "AWSPREVIOUS"))
-        except service_client.exceptions.ResourceNotFoundException:
-            conn = None
+        conn = get_connection(previous_dict)
+
+        # Make sure the user and host from previous and pending match
+        if previous_dict['username'] != pending_dict['username']:
+            logger.error("setSecret: Attempting to modify user %s other than last valid user %s" % (pending_dict['username'], previous_dict['username']))
+            raise ValueError("Attempting to modify user %s other than last valid user %s" % (pending_dict['username'], previous_dict['username']))
+        if previous_dict['host'] != pending_dict['host']:
+            logger.error("setSecret: Attempting to modify user for host %s other than previous host %s" % (pending_dict['host'], previous_dict['host']))
+            raise ValueError("Attempting to modify user for host %s other than previous host %s" % (pending_dict['host'], previous_dict['host']))
 
     # If we still don't have a connection, raise a ValueError
     if not conn: