From 6af974548e7042ddefac5afa641a95879732be63 Mon Sep 17 00:00:00 2001 From: Anastasia Lubennikova Date: Wed, 19 Mar 2025 00:13:36 +0000 Subject: [PATCH] feat(compute_ctl): Add basic audit logging for computes. (#11170) if `audit_log_level` is set to Log, preload pgaudit extension and log DDL with masked parameters into standard postgresql log --- compute_tools/src/config.rs | 104 +++++++++++++++++++++----------- compute_tools/src/spec_apply.rs | 5 +- 2 files changed, 74 insertions(+), 35 deletions(-) diff --git a/compute_tools/src/config.rs b/compute_tools/src/config.rs index 290632e4cd..eb9dd00da5 100644 --- a/compute_tools/src/config.rs +++ b/compute_tools/src/config.rs @@ -158,53 +158,89 @@ pub fn write_postgres_conf( writeln!(file, "# Managed by compute_ctl: end")?; } - // If audit logging is enabled, configure pgaudit. + // If base audit logging is enabled, configure it. + // In this setup, the audit log will be written to the standard postgresql log. + // + // If compliance audit logging is enabled, configure pgaudit. // // Note, that this is called after the settings from spec are written. // This way we always override the settings from the spec // and don't allow the user or the control plane admin to change them. - if let ComputeAudit::Hipaa = spec.audit_log_level { - writeln!(file, "# Managed by compute_ctl audit settings: begin")?; - // This log level is very verbose - // but this is necessary for HIPAA compliance. - // Exclude 'misc' category, because it doesn't contain anythig relevant. - writeln!(file, "pgaudit.log='all, -misc'")?; - writeln!(file, "pgaudit.log_parameter=on")?; - // Disable logging of catalog queries - // The catalog doesn't contain sensitive data, so we don't need to audit it. - writeln!(file, "pgaudit.log_catalog=off")?; - // Set log rotation to 5 minutes - // TODO: tune this after performance testing - writeln!(file, "pgaudit.log_rotation_age=5")?; + match spec.audit_log_level { + ComputeAudit::Disabled => {} + ComputeAudit::Log => { + writeln!(file, "# Managed by compute_ctl base audit settings: start")?; + writeln!(file, "pgaudit.log='ddl,role'")?; + // Disable logging of catalog queries to reduce the noise + writeln!(file, "pgaudit.log_catalog=off")?; - // Add audit shared_preload_libraries, if they are not present. - // - // The caller who sets the flag is responsible for ensuring that the necessary - // shared_preload_libraries are present in the compute image, - // otherwise the compute start will fail. - if let Some(libs) = spec.cluster.settings.find("shared_preload_libraries") { - let mut extra_shared_preload_libraries = String::new(); - if !libs.contains("pgaudit") { - extra_shared_preload_libraries.push_str(",pgaudit"); - } - if !libs.contains("pgauditlogtofile") { - extra_shared_preload_libraries.push_str(",pgauditlogtofile"); + if let Some(libs) = spec.cluster.settings.find("shared_preload_libraries") { + let mut extra_shared_preload_libraries = String::new(); + if !libs.contains("pgaudit") { + extra_shared_preload_libraries.push_str(",pgaudit"); + } + writeln!( + file, + "shared_preload_libraries='{}{}'", + libs, extra_shared_preload_libraries + )?; + } else { + // Typically, this should be unreacheable, + // because we always set at least some shared_preload_libraries in the spec + // but let's handle it explicitly anyway. + writeln!(file, "shared_preload_libraries='neon,pgaudit'")?; } + writeln!(file, "# Managed by compute_ctl base audit settings: end")?; + } + ComputeAudit::Hipaa => { writeln!( file, - "shared_preload_libraries='{}{}'", - libs, extra_shared_preload_libraries + "# Managed by compute_ctl compliance audit settings: begin" )?; - } else { - // Typically, this should be unreacheable, - // because we always set at least some shared_preload_libraries in the spec - // but let's handle it explicitly anyway. + // This log level is very verbose + // but this is necessary for HIPAA compliance. + // Exclude 'misc' category, because it doesn't contain anythig relevant. + writeln!(file, "pgaudit.log='all, -misc'")?; + writeln!(file, "pgaudit.log_parameter=on")?; + // Disable logging of catalog queries + // The catalog doesn't contain sensitive data, so we don't need to audit it. + writeln!(file, "pgaudit.log_catalog=off")?; + // Set log rotation to 5 minutes + // TODO: tune this after performance testing + writeln!(file, "pgaudit.log_rotation_age=5")?; + + // Add audit shared_preload_libraries, if they are not present. + // + // The caller who sets the flag is responsible for ensuring that the necessary + // shared_preload_libraries are present in the compute image, + // otherwise the compute start will fail. + if let Some(libs) = spec.cluster.settings.find("shared_preload_libraries") { + let mut extra_shared_preload_libraries = String::new(); + if !libs.contains("pgaudit") { + extra_shared_preload_libraries.push_str(",pgaudit"); + } + if !libs.contains("pgauditlogtofile") { + extra_shared_preload_libraries.push_str(",pgauditlogtofile"); + } + writeln!( + file, + "shared_preload_libraries='{}{}'", + libs, extra_shared_preload_libraries + )?; + } else { + // Typically, this should be unreacheable, + // because we always set at least some shared_preload_libraries in the spec + // but let's handle it explicitly anyway. + writeln!( + file, + "shared_preload_libraries='neon,pgaudit,pgauditlogtofile'" + )?; + } writeln!( file, - "shared_preload_libraries='neon,pgaudit,pgauditlogtofile'" + "# Managed by compute_ctl compliance audit settings: end" )?; } - writeln!(file, "# Managed by compute_ctl audit settings: end")?; } writeln!(file, "neon.extension_server_port={}", extension_server_port)?; diff --git a/compute_tools/src/spec_apply.rs b/compute_tools/src/spec_apply.rs index 80506b13cb..27af0e80f6 100644 --- a/compute_tools/src/spec_apply.rs +++ b/compute_tools/src/spec_apply.rs @@ -286,7 +286,10 @@ impl ComputeNode { phases.push(CreatePgauditlogtofileExtension); phases.push(DisablePostgresDBPgAudit); } - ComputeAudit::Log => { /* not implemented yet */ } + ComputeAudit::Log => { + phases.push(CreatePgauditExtension); + phases.push(DisablePostgresDBPgAudit); + } ComputeAudit::Disabled => {} }