Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audit log prototype #7339

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 50 additions & 0 deletions common/src/api/external/http_pagination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ use crate::api::external::Name;
use crate::api::external::NameOrId;
use crate::api::external::ObjectIdentity;
use crate::api::external::PaginationOrder;
use chrono::DateTime;
use chrono::Utc;
use dropshot::HttpError;
use dropshot::PaginationParams;
use dropshot::RequestContext;
Expand Down Expand Up @@ -410,6 +412,54 @@ impl<
}
}

/// Query parameters for pagination by timestamp and ID
pub type PaginatedByTimeAndId<Selector = ()> = PaginationParams<
ScanByTimeAndId<Selector>,
PageSelectorByTimeAndId<Selector>,
>;
/// Page selector for pagination by timestamp and ID
pub type PageSelectorByTimeAndId<Selector = ()> =
PageSelector<ScanByTimeAndId<Selector>, (DateTime<Utc>, Uuid)>;

/// Scan parameters for resources that support scanning by (timestamp, id)
#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
pub struct ScanByTimeAndId<Selector = ()> {
#[serde(default = "default_ts_id_sort_mode")]
sort_by: TimeAndIdSortMode,

#[serde(flatten)]
pub selector: Selector,
}
/// Supported set of sort modes for scanning by timestamp and ID
///
/// Currently, we only support scanning in ascending order.
#[derive(Copy, Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum TimeAndIdSortMode {
/// sort in increasing order of timestamp and ID
Ascending,
}

fn default_ts_id_sort_mode() -> TimeAndIdSortMode {
TimeAndIdSortMode::Ascending
}

impl<
T: Clone + Debug + DeserializeOwned + JsonSchema + PartialEq + Serialize,
> ScanParams for ScanByTimeAndId<T>
{
type MarkerValue = (DateTime<Utc>, Uuid);
fn direction(&self) -> PaginationOrder {
PaginationOrder::Ascending
}
fn from_query(p: &PaginatedByTimeAndId<T>) -> Result<&Self, HttpError> {
Ok(match p.page {
WhichPage::First(ref scan_params) => scan_params,
WhichPage::Next(PageSelector { ref scan, .. }) => scan,
})
}
}

#[cfg(test)]
mod test {
use super::data_page_params_with_limit;
Expand Down
1 change: 1 addition & 0 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,7 @@ pub enum ResourceType {
AddressLot,
AddressLotBlock,
AllowList,
AuditLogEntry,
BackgroundTask,
BgpConfig,
BgpAnnounceSet,
Expand Down
9 changes: 9 additions & 0 deletions nexus/auth/src/authn/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,15 @@ impl Context {
&self.schemes_tried
}

/// If the user is authenticated, return the last scheme in the list of
/// schemes tried, which is the one that worked.
pub fn scheme_used(&self) -> Option<&SchemeName> {
match &self.kind {
Kind::Authenticated(..) => self.schemes_tried().last(),
Kind::Unauthenticated => None,
}
}

/// Returns an unauthenticated context for use internally
pub fn internal_unauthenticated() -> Context {
Context { kind: Kind::Unauthenticated, schemes_tried: vec![] }
Expand Down
62 changes: 60 additions & 2 deletions nexus/auth/src/authz/api_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,66 @@ impl AuthorizedResource for IpPoolList {
roleset: &'fut mut RoleSet,
) -> futures::future::BoxFuture<'fut, Result<(), Error>> {
// There are no roles on the IpPoolList, only permissions. But we still
// need to load the Fleet-related roles to verify that the actor has the
// "admin" role on the Fleet (possibly conferred from a Silo role).
// need to load the Fleet-related roles to verify that the actor's role
// on the Fleet (possibly conferred from a Silo role).
load_roles_for_resource_tree(&FLEET, opctx, authn, roleset).boxed()
}

fn on_unauthorized(
&self,
_: &Authz,
error: Error,
_: AnyActor,
_: Action,
) -> Error {
error
}

fn polar_class(&self) -> oso::Class {
Self::get_polar_class()
}
}

// Similar to IpPoolList, the audit log is a collection that doesn't exist in
// the database as an entity distinct from its children (IP pools, or in this
// case, audit log entries). We need a dummy resource here because we need
// something to hang permissions off of. We need to be able to create audit log
// children (entries) for login attempts, when there is no authenticated user,
// as well as for normal requests with an authenticated user. For retrieval, we
// want (to start out) to allow only fleet viewers to list children.

#[derive(Clone, Copy, Debug)]
pub struct AuditLog;

/// Singleton representing the [`AuditLog`] for authz purposes
pub const AUDIT_LOG: AuditLog = AuditLog;

impl Eq for AuditLog {}

impl PartialEq for AuditLog {
fn eq(&self, _: &Self) -> bool {
true
}
}

impl oso::PolarClass for AuditLog {
fn get_polar_class_builder() -> oso::ClassBuilder<Self> {
oso::Class::builder()
.with_equality_check()
.add_attribute_getter("fleet", |_: &AuditLog| FLEET)
}
}

impl AuthorizedResource for AuditLog {
fn load_roles<'fut>(
&'fut self,
opctx: &'fut OpContext,
authn: &'fut authn::Context,
roleset: &'fut mut RoleSet,
) -> futures::future::BoxFuture<'fut, Result<(), Error>> {
// There are no roles on the AuditLog, only permissions. But we still
// need to load the Fleet-related roles to verify that the actor's role
// on the Fleet (possibly conferred from a Silo role).
load_roles_for_resource_tree(&FLEET, opctx, authn, roleset).boxed()
}

Expand Down
19 changes: 19 additions & 0 deletions nexus/auth/src/authz/omicron.polar
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,25 @@ has_relation(fleet: Fleet, "parent_fleet", ip_pool_list: IpPoolList)
has_permission(actor: AuthenticatedActor, "create_child", ip_pool: IpPool)
if silo in actor.silo and silo.fleet = ip_pool.fleet;

# Describes the policy for reading and writing the audit log
resource AuditLog {
permissions = [
"list_children", # retrieve audit log
"create_child", # create audit log entry
];

relations = { parent_fleet: Fleet };

# Fleet viewers can read the audit log
"list_children" if "viewer" on "parent_fleet";
}
# TODO: is this right? any op context should be able to write to the audit log?
# feels weird though
has_permission(_actor: AuthenticatedActor, "create_child", _audit_log: AuditLog);

has_relation(fleet: Fleet, "parent_fleet", audit_log: AuditLog)
if audit_log.fleet = fleet;

# Describes the policy for creating and managing web console sessions.
resource ConsoleSessionList {
permissions = [ "create_child" ];
Expand Down
1 change: 1 addition & 0 deletions nexus/auth/src/authz/oso_generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result<OsoInit, anyhow::Error> {
let classes = [
// Hand-written classes
Action::get_polar_class(),
AuditLog::get_polar_class(),
AnyActor::get_polar_class(),
AuthenticatedActor::get_polar_class(),
BlueprintConfig::get_polar_class(),
Expand Down
152 changes: 152 additions & 0 deletions nexus/db-model/src/audit_log.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/5.0/.

// Copyright 2025 Oxide Computer Company

use crate::schema::{audit_log, audit_log_complete};
use crate::SqlU16;
use chrono::{DateTime, Utc};
use diesel::prelude::*;
use nexus_types::external_api::views;
use uuid::Uuid;

#[derive(Queryable, Insertable, Selectable, Clone, Debug)]
#[diesel(table_name = audit_log)]
pub struct AuditLogEntryInit {
pub id: Uuid,
pub timestamp: DateTime<Utc>,
pub request_id: String,
/// The API endpoint being logged, e.g., `project_create`
pub request_uri: String,
pub operation_id: String,
pub source_ip: String,
// TODO: we probably want a dedicated enum for these columns and for that
// we need a fancier set of columns. For example, we may want to initialize
// the row with a _potential_ actor (probably a different field), like the
// username or whatever is being used for login. This should probably be
// preserved even after authentication determines an actual actor ID. See
// the Actor struct in nexus/auth/src/authn/mod.ts

// these are optional because of requests like login attempts, where there
// is no actor until after the operation.
pub actor_id: Option<Uuid>,
pub actor_silo_id: Option<Uuid>,

// TODO: fancier type for access method capturing possibility of login
// attempts. might make sense to roll this all into the actor enum because
// we have an access method if and only if we have an actor (I think)
/// API token or session cookie. Optional because it will not be defined
/// on unauthenticated requests like login attempts.
pub access_method: Option<String>,
}

// TODO: doc comments
// TODO: figure out how this relates to the other struct. currently we're not
// retrieving partial entries at all, but I think we will probably want to have
// that capability
#[derive(Queryable, Selectable, Clone, Debug)]
#[diesel(table_name = audit_log_complete)]
pub struct AuditLogEntry {
pub id: Uuid,
pub timestamp: DateTime<Utc>,
pub request_id: String,
pub request_uri: String,
pub operation_id: String,
pub source_ip: String,
pub actor_id: Option<Uuid>,
pub actor_silo_id: Option<Uuid>,
pub access_method: Option<String>,

// TODO: RFD 523 says: "Additionally, the response (or error) data should be
// included in the same log entry as the original request data. Separating
// the response from the request into two different log entries is extremely
// expensive for customers to identify which requests correspond to which
// responses." I guess the typical thing is to include a duration of the
// request rather than a second timestamp.
Comment on lines +61 to +66

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was me making a case for including something like the pub success_response: Option<Value> you have in a log entry. I can clarify that in the RFD.


// Seems like it has to be optional because at the beginning of the
// operation, we have not yet resolved the resource selector to an ID
pub resource_id: Option<Uuid>,

// Fields that are not present on init
/// Time log entry was completed with info about result of operation
pub time_completed: DateTime<Utc>,
pub http_status_code: SqlU16,

// Error information if the action failed
pub error_code: Option<String>,
pub error_message: Option<String>,
Comment on lines +77 to +79

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered using a enum for response elements instead of optionals for errors and successful responses?

Something like:

#[derive(Serialize, Deserialize)]
#[serde(untagged)]
enum Response {
    Success { success: Value },
    Error { error_code: String, error_message: String }
}

Not sure I have a preference over one way over the other in terms of output structure, though I kind of like the enum from a code perspective to help ensure you either get one or the other.

Copy link
Contributor Author

@david-crespo david-crespo Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes — most likely what I'll do is have views::AuditLogEntry look nice and structured but keep the model struct flat like the DB entry, and then the conversion from model to view is what mediates between those two formats.

// TODO: including a real response complicates things
// Response data on success (if applicable)
// pub success_response: Option<Value>,
Comment on lines +80 to +82

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this indeed complicates things, it is critical IMO. For example, if someone were to create a new instance this audit log should say what that new instance ID is as a result.

}

impl AuditLogEntryInit {
pub fn new(
request_id: String,
operation_id: String,
request_uri: String,
source_ip: String,
actor_id: Option<Uuid>,
actor_silo_id: Option<Uuid>,
access_method: Option<String>,
) -> Self {
Self {
id: Uuid::new_v4(),
timestamp: Utc::now(),
request_id,
request_uri,
operation_id,
actor_id,
actor_silo_id,
source_ip,
access_method,
}
}
}

#[derive(AsChangeset, Clone)]
#[diesel(table_name = audit_log)]
pub struct AuditLogCompletion {
pub time_completed: DateTime<Utc>,
pub http_status_code: SqlU16,
}

impl AuditLogCompletion {
pub fn new(http_status_code: u16) -> Self {
Self {
time_completed: Utc::now(),
http_status_code: SqlU16(http_status_code),
}
}
}

// TODO: AuditLogActor
// pub enum AuditLogActor {
// UserBuiltin { user_builtin_id: Uuid },
// TODO: include info about computed roles at runtime?
// SiloUser { silo_user_id: Uuid, silo_id: Uuid },
// Unauthenticated,
// }

impl From<AuditLogEntry> for views::AuditLogEntry {
fn from(entry: AuditLogEntry) -> Self {
Self {
id: entry.id,
timestamp: entry.timestamp,
request_id: entry.request_id,
request_uri: entry.request_uri,
operation_id: entry.operation_id,
source_ip: entry.source_ip,
resource_id: entry.resource_id,
actor_id: entry.actor_id,
actor_silo_id: entry.actor_silo_id,
access_method: entry.access_method,
time_completed: entry.time_completed,
http_status_code: entry.http_status_code.0,
error_code: entry.error_code,
error_message: entry.error_message,
}
}
}
2 changes: 2 additions & 0 deletions nexus/db-model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ extern crate newtype_derive;

mod address_lot;
mod allow_list;
mod audit_log;
mod bfd;
mod bgp;
mod block_size;
Expand Down Expand Up @@ -131,6 +132,7 @@ pub use self::macaddr::*;
pub use self::unsigned::*;
pub use address_lot::*;
pub use allow_list::*;
pub use audit_log::*;
pub use bfd::*;
pub use bgp::*;
pub use block_size::*;
Expand Down
Loading
Loading