Skip to content

Commit

Permalink
auditloggingccl: migrate role-based audit logging as a CCL feature
Browse files Browse the repository at this point in the history
This change moves the existing role-based audit logging logic to be
consumed as a CCL (enterprise) feature.

Release note (sql change): role-based audit logging not a CCL
(enterprise) feature

https://cockroachlabs.atlassian.net/browse/DOC-1355 #Informs:
cockroachdb#33316 #Epic: CRDB-8035
  • Loading branch information
Thomas Hardy committed Jun 6, 2023
1 parent 530e2ef commit 11e5249
Show file tree
Hide file tree
Showing 15 changed files with 169 additions and 47 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@
/pkg/workload/ @cockroachdb/test-eng #! @cockroachdb/sql-foundations-noreview
/pkg/obs/ @cockroachdb/obs-inf-prs
/pkg/obsservice/ @cockroachdb/obs-inf-prs
/pkg/ccl/auditloggingccl @cockroachdb/cluster-observability

# Own all bazel files to dev-inf, but don't request reviews for them
# as they are mostly - but not only - generated code that changes with
Expand Down
2 changes: 0 additions & 2 deletions docs/generated/settings/settings-for-tenants.txt
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,6 @@ sql.insights.latency_threshold duration 100ms amount of time after which an exec
sql.log.slow_query.experimental_full_table_scans.enabled boolean false when set to true, statements that perform a full table/index scan will be logged to the slow query log even if they do not meet the latency threshold. Must have the slow query log enabled for this setting to have any effect. tenant-rw
sql.log.slow_query.internal_queries.enabled boolean false when set to true, internal queries which exceed the slow query log threshold are logged to a separate log. Must have the slow query log enabled for this setting to have any effect. tenant-rw
sql.log.slow_query.latency_threshold duration 0s when set to non-zero, log statements whose service latency exceeds the threshold to a secondary logger on each node tenant-rw
sql.log.user_audit string user/role-based audit logging configuration tenant-rw
sql.log.user_audit.reduced_config.enabled boolean false enables logic to compute a reduced audit configuration, computing the audit configuration only once at session start instead of at each SQL event. The tradeoff with the increase in performance (~5%), is that changes to the audit configuration (user role memberships/cluster setting) are not reflected within session. Users will need to start a new session to see these changes in their auditing behaviour. tenant-rw
sql.metrics.index_usage_stats.enabled boolean true collect per index usage statistics tenant-rw
sql.metrics.max_mem_reported_stmt_fingerprints integer 100000 the maximum number of reported statement fingerprints stored in memory tenant-rw
sql.metrics.max_mem_reported_txn_fingerprints integer 100000 the maximum number of reported transaction fingerprints stored in memory tenant-rw
Expand Down
2 changes: 0 additions & 2 deletions docs/generated/settings/settings.html
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,6 @@
<tr><td><div id="setting-sql-log-slow-query-experimental-full-table-scans-enabled" class="anchored"><code>sql.log.slow_query.experimental_full_table_scans.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>when set to true, statements that perform a full table/index scan will be logged to the slow query log even if they do not meet the latency threshold. Must have the slow query log enabled for this setting to have any effect.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-slow-query-internal-queries-enabled" class="anchored"><code>sql.log.slow_query.internal_queries.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>when set to true, internal queries which exceed the slow query log threshold are logged to a separate log. Must have the slow query log enabled for this setting to have any effect.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-slow-query-latency-threshold" class="anchored"><code>sql.log.slow_query.latency_threshold</code></div></td><td>duration</td><td><code>0s</code></td><td>when set to non-zero, log statements whose service latency exceeds the threshold to a secondary logger on each node</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-user-audit" class="anchored"><code>sql.log.user_audit</code></div></td><td>string</td><td><code></code></td><td>user/role-based audit logging configuration</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-log-user-audit-reduced-config-enabled" class="anchored"><code>sql.log.user_audit.reduced_config.enabled</code></div></td><td>boolean</td><td><code>false</code></td><td>enables logic to compute a reduced audit configuration, computing the audit configuration only once at session start instead of at each SQL event. The tradeoff with the increase in performance (~5%), is that changes to the audit configuration (user role memberships/cluster setting) are not reflected within session. Users will need to start a new session to see these changes in their auditing behaviour.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-metrics-index-usage-stats-enabled" class="anchored"><code>sql.metrics.index_usage_stats.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>collect per index usage statistics</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-metrics-max-mem-reported-stmt-fingerprints" class="anchored"><code>sql.metrics.max_mem_reported_stmt_fingerprints</code></div></td><td>integer</td><td><code>100000</code></td><td>the maximum number of reported statement fingerprints stored in memory</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
<tr><td><div id="setting-sql-metrics-max-mem-reported-txn-fingerprints" class="anchored"><code>sql.metrics.max_mem_reported_txn_fingerprints</code></div></td><td>integer</td><td><code>100000</code></td><td>the maximum number of reported transaction fingerprints stored in memory</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
Expand Down
4 changes: 4 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ ALL_TESTS = [
"//pkg/build/starlarkutil:starlarkutil_test",
"//pkg/build/util:util_test",
"//pkg/build:build_test",
"//pkg/ccl/auditloggingccl:auditloggingccl_test",
"//pkg/ccl/backupccl/backupdest:backupdest_test",
"//pkg/ccl/backupccl/backupinfo:backupinfo_disallowed_imports_test",
"//pkg/ccl/backupccl/backupinfo:backupinfo_test",
Expand Down Expand Up @@ -742,6 +743,8 @@ GO_TARGETS = [
"//pkg/build/util:util_test",
"//pkg/build:build",
"//pkg/build:build_test",
"//pkg/ccl/auditloggingccl:auditloggingccl",
"//pkg/ccl/auditloggingccl:auditloggingccl_test",
"//pkg/ccl/backupccl/backupbase:backupbase",
"//pkg/ccl/backupccl/backupdest:backupdest",
"//pkg/ccl/backupccl/backupdest:backupdest_test",
Expand Down Expand Up @@ -2447,6 +2450,7 @@ GET_X_DATA_TARGETS = [
"//pkg/build/starlarkutil:get_x_data",
"//pkg/build/util:get_x_data",
"//pkg/ccl:get_x_data",
"//pkg/ccl/auditloggingccl:get_x_data",
"//pkg/ccl/backupccl:get_x_data",
"//pkg/ccl/backupccl/backupbase:get_x_data",
"//pkg/ccl/backupccl/backupdest:get_x_data",
Expand Down
50 changes: 50 additions & 0 deletions pkg/ccl/auditloggingccl/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
load("//build/bazelutil/unused_checker:unused.bzl", "get_x_data")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "auditloggingccl",
srcs = ["audit_log_config.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/auditloggingccl",
visibility = ["//visibility:public"],
deps = [
"//pkg/ccl/utilccl",
"//pkg/settings",
"//pkg/settings/cluster",
"//pkg/sql/auditlogging",
"//pkg/util/log",
"//pkg/util/uuid",
"@com_github_cockroachdb_errors//:errors",
],
)

go_test(
name = "auditloggingccl_test",
srcs = [
"audit_logging_test.go",
"main_test.go",
],
args = ["-test.timeout=295s"],
embed = [":auditloggingccl"],
tags = ["ccl_test"],
deps = [
"//pkg/base",
"//pkg/ccl",
"//pkg/security/securityassets",
"//pkg/security/securitytest",
"//pkg/security/username",
"//pkg/server",
"//pkg/testutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/log/logpb",
"//pkg/util/log/logtestutils",
"//pkg/util/randutil",
"@com_github_cockroachdb_errors//:errors",
"@com_github_stretchr_testify//require",
],
)

get_x_data(name = "get_x_data")
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
// Licensed as a CockroachDB Enterprise file under the Cockroach Community
// License (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.
// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt

package auditlogging
package auditloggingccl

import (
"context"

"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/auditlogging"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/cockroachdb/errors"
)

Expand Down Expand Up @@ -51,11 +52,11 @@ func validateAuditLogConfig(_ *settings.Values, input string) error {
return nil
}
// Ensure it can be parsed.
conf, err := parse(input)
conf, err := auditlogging.Parse(input)
if err != nil {
return err
}
if len(conf.settings) == 0 {
if len(conf.Settings) == 0 {
// The string was not empty, but we were unable to parse any settings.
return errors.WithHint(errors.New("no entries"),
"To use the default configuration, assign the empty string ('').")
Expand All @@ -66,17 +67,46 @@ func validateAuditLogConfig(_ *settings.Values, input string) error {
// UpdateAuditConfigOnChange initializes the local
// node's audit configuration each time the cluster setting
// is updated.
func UpdateAuditConfigOnChange(ctx context.Context, acl *AuditConfigLock, st *cluster.Settings) {
func UpdateAuditConfigOnChange(
ctx context.Context, acl *auditlogging.AuditConfigLock, st *cluster.Settings, cluster uuid.UUID,
) {
enterpriseCheckErr := utilccl.CheckEnterpriseEnabled(st, cluster, "role-based audit logging")
if enterpriseCheckErr != nil {
log.Ops.Error(ctx, "error updating role-based audit log configuration: requires enterprise license")
return
}
val := UserAuditLogConfig.Get(&st.SV)
config, err := parse(val)
config, err := auditlogging.Parse(val)
if err != nil {
// We encounter an error parsing (i.e. invalid config), fallback
// to an empty config.
log.Ops.Warningf(ctx, "invalid audit log config (sql.log.user_audit): %v\n"+
"falling back to empty audit config", err)
config = EmptyAuditConfig()
config = auditlogging.EmptyAuditConfig()
}
acl.Lock()
acl.Config = config
acl.Unlock()
}

var ConfigureRoleBasedAuditClusterSettings = func(ctx context.Context, acl *auditlogging.AuditConfigLock, st *cluster.Settings, sv *settings.Values, cluster uuid.UUID) {
UserAuditLogConfig.SetOnChange(
sv, func(ctx context.Context) {
UpdateAuditConfigOnChange(ctx, acl, st, cluster)
})
UpdateAuditConfigOnChange(ctx, acl, st, cluster)
}

var UserAuditLogConfigEmpty = func(sv *settings.Values) bool {
return UserAuditLogConfig.Get(sv) == ""
}

var UserAuditReducedConfigEnabled = func(sv *settings.Values) bool {
return UserAuditEnableReducedConfig.Get(sv)
}

func init() {
auditlogging.ConfigureRoleBasedAuditClusterSettings = ConfigureRoleBasedAuditClusterSettings
auditlogging.UserAuditLogConfigEmpty = UserAuditLogConfigEmpty
auditlogging.UserAuditReducedConfigEnabled = UserAuditReducedConfigEnabled
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
// Licensed as a CockroachDB Enterprise file under the Cockroach Community
// License (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.
// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt

package sql
package auditloggingccl

import (
"context"
Expand Down
31 changes: 31 additions & 0 deletions pkg/ccl/auditloggingccl/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2022 The Cockroach Authors.
//
// Licensed as a CockroachDB Enterprise file under the Cockroach Community
// License (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt

package auditloggingccl

import (
"os"
"testing"

"github.com/cockroachdb/cockroach/pkg/ccl"
"github.com/cockroachdb/cockroach/pkg/security/securityassets"
"github.com/cockroachdb/cockroach/pkg/security/securitytest"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/util/randutil"
)

func TestMain(m *testing.M) {
defer ccl.TestingEnableEnterprise()()
securityassets.SetLoader(securitytest.EmbeddedAssets)
randutil.SeedForTests()
serverutils.InitTestServerFactory(server.TestServerFactory)
serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
os.Exit(m.Run())
}
6 changes: 1 addition & 5 deletions pkg/server/server_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -1374,11 +1374,7 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
vmoduleSetting.SetOnChange(&cfg.Settings.SV, fn)
fn(ctx)

auditlogging.UserAuditLogConfig.SetOnChange(
&execCfg.Settings.SV, func(ctx context.Context) {
auditlogging.UpdateAuditConfigOnChange(ctx, execCfg.SessionInitCache.AuditConfig, execCfg.Settings)
})
auditlogging.UpdateAuditConfigOnChange(ctx, execCfg.SessionInitCache.AuditConfig, execCfg.Settings)
auditlogging.ConfigureRoleBasedAuditClusterSettings(ctx, execCfg.SessionInitCache.AuditConfig, execCfg.Settings, &execCfg.Settings.SV, cfg.ClusterIDContainer.Get())

return &SQLServer{
ambientCtx: cfg.BaseConfig.AmbientCtx,
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,6 @@ go_test(
"alter_column_type_test.go",
"ambiguous_commit_test.go",
"as_of_test.go",
"audit_logging_test.go",
"authorization_test.go",
"backfill_num_ranges_in_span_test.go",
"backfill_protected_timestamp_test.go",
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/audit_logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (p *planner) maybeAuditRoleBasedAuditEvent(ctx context.Context) {
}

// Use reduced audit config is enabled.
if auditlogging.UserAuditEnableReducedConfig.Get(&p.execCfg.Settings.SV) {
if auditlogging.UserAuditReducedConfigEnabled(&p.execCfg.Settings.SV) {
p.logReducedAuditConfig(ctx)
return
}
Expand Down Expand Up @@ -119,5 +119,5 @@ func (p *planner) shouldNotRoleBasedAudit() bool {
// Do not do audit work if the cluster setting is empty.
// Do not emit audit events for reserved users/roles. This does not omit the root user.
// Do not emit audit events for internal planners.
return auditlogging.UserAuditLogConfig.Get(&p.execCfg.Settings.SV) == "" || p.User().IsReserved() || p.isInternalPlanner
return auditlogging.UserAuditLogConfigEmpty(&p.execCfg.Settings.SV) || p.User().IsReserved() || p.isInternalPlanner
}
3 changes: 1 addition & 2 deletions pkg/sql/auditlogging/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ go_library(
name = "auditlogging",
srcs = [
"audit_log.go",
"audit_log_config.go",
"parser.go",
],
importpath = "github.com/cockroachdb/cockroach/pkg/sql/auditlogging",
Expand All @@ -19,10 +18,10 @@ go_library(
"//pkg/sql/pgwire/pgcode",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/sem/tree",
"//pkg/util/log",
"//pkg/util/log/eventpb",
"//pkg/util/log/logpb",
"//pkg/util/syncutil",
"//pkg/util/uuid",
"@com_github_cockroachdb_errors//:errors",
"@com_github_olekukonko_tablewriter//:tablewriter",
],
Expand Down
36 changes: 27 additions & 9 deletions pkg/sql/auditlogging/audit_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,31 @@ import (

"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
"github.com/cockroachdb/cockroach/pkg/util/log/logpb"
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
"github.com/cockroachdb/cockroach/pkg/util/uuid"
"github.com/olekukonko/tablewriter"
)

// ConfigureRoleBasedAuditClusterSettings is a noop global var injected by CCL hook.
// See corresponding ConfigureRoleBasedAuditClusterSettings in auditloggingccl.
var ConfigureRoleBasedAuditClusterSettings = func(ctx context.Context, acl *AuditConfigLock, st *cluster.Settings, sv *settings.Values, cluster uuid.UUID) {
}

// UserAuditLogConfigEmpty is a noop global var injected by CCL hook.
var UserAuditLogConfigEmpty = func(sv *settings.Values) bool {
return true
}

// UserAuditReducedConfigEnabled is a noop global var injected by CCL hook.
var UserAuditReducedConfigEnabled = func(sv *settings.Values) bool {
return false
}

// Auditor is an interface used to check and build different audit events.
type Auditor interface {
GetQualifiedTableNameByID(ctx context.Context, id int64, requiredType tree.RequiredTableKind) (*tree.TableName, error)
Expand Down Expand Up @@ -74,16 +92,16 @@ func (cl *AuditConfigLock) GetMatchingAuditSetting(

// AuditConfig is a parsed configuration.
type AuditConfig struct {
// settings are the collection of AuditSettings that make up the AuditConfig.
settings []AuditSetting
// allRoleAuditSettingIdx is an index corresponding to an AuditSetting in settings that applies to all
// Settings are the collection of AuditSettings that make up the AuditConfig.
Settings []AuditSetting
// allRoleAuditSettingIdx is an index corresponding to an AuditSetting in Settings that applies to all
// users, if it exists. Default value -1 (defaultAllAuditSettingIdx).
allRoleAuditSettingIdx int
}

const defaultAllAuditSettingIdx = -1

// EmptyAuditConfig returns an audit configuration with no audit settings.
// EmptyAuditConfig returns an audit configuration with no audit Settings.
func EmptyAuditConfig() *AuditConfig {
return &AuditConfig{
allRoleAuditSettingIdx: defaultAllAuditSettingIdx,
Expand All @@ -96,7 +114,7 @@ func (c AuditConfig) getMatchingAuditSetting(
userRoles map[username.SQLUsername]bool, name username.SQLUsername,
) *AuditSetting {
// If the user matches any Setting, return the corresponding filter.
for idx, filter := range c.settings {
for idx, filter := range c.Settings {
// If we have matched an audit setting by role, return the audit setting.
_, exists := userRoles[filter.Role]
if exists {
Expand All @@ -116,13 +134,13 @@ func (c AuditConfig) getMatchingAuditSetting(
}

func (c AuditConfig) String() string {
if len(c.settings) == 0 {
if len(c.Settings) == 0 {
return "# (empty configuration)\n"
}

var sb strings.Builder
sb.WriteString("# Original configuration:\n")
for _, setting := range c.settings {
for _, setting := range c.Settings {
fmt.Fprintf(&sb, "# %s\n", setting.input)
}
sb.WriteString("#\n# Interpreted configuration:\n")
Expand All @@ -138,7 +156,7 @@ func (c AuditConfig) String() string {

row := []string{"# ROLE", "STATEMENT_FILTER"}
table.Append(row)
for _, setting := range c.settings {
for _, setting := range c.Settings {
row[0] = setting.Role.Normalized()
row[1] = writeStatementFilter(setting.IncludeStatements)
table.Append(row)
Expand Down Expand Up @@ -167,5 +185,5 @@ type AuditSetting struct {
}

func (s AuditSetting) String() string {
return AuditConfig{settings: []AuditSetting{s}}.String()
return AuditConfig{Settings: []AuditSetting{s}}.String()
}
Loading

0 comments on commit 11e5249

Please sign in to comment.