Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1f9ca46
Initial auditEntry implementation
davidraskin Aug 2, 2020
4078b7f
Initial audit log implementation
davidraskin Aug 6, 2020
a6fc504
Merge conflict
davidraskin Aug 6, 2020
d300d24
Format
davidraskin Aug 6, 2020
ca0f229
Use LogEntryType and map for WriteLogEntriesRequest
davidraskin Aug 11, 2020
c139a02
Merge remote-tracking branch 'upstream/master'
davidraskin Aug 11, 2020
fac3b3b
Add function to initialize logEntriesRequests
davidraskin Aug 11, 2020
eea4b00
Format
davidraskin Aug 11, 2020
c8f5d2d
Merge with master
davidraskin Aug 12, 2020
b35ef06
Switched to using bool for outbound and bool for audit
davidraskin Aug 12, 2020
e86276d
Format and remove uneeded functions
davidraskin Aug 12, 2020
ea0b0b0
Undo stats modification for non cpp20
davidraskin Aug 12, 2020
ff49dcc
Takeout double setting of label
davidraskin Aug 12, 2020
4eca8d6
Update extensions/common/context.h
davidraskin Aug 14, 2020
8e426be
Update extensions/common/context.h
davidraskin Aug 14, 2020
4bc1b2b
Remove redundant request_info population
davidraskin Aug 17, 2020
5035a44
Merge remote-tracking branch 'origin/master'
davidraskin Aug 17, 2020
0eb5a5e
Addressed comments
davidraskin Aug 17, 2020
f317e92
Update comments + change when audit entry is added
davidraskin Aug 18, 2020
44e62df
Update comments. remove unnecessary include
davidraskin Aug 18, 2020
6324a29
format
davidraskin Aug 18, 2020
93ca7c2
Fix tcp audit logging
davidraskin Aug 18, 2020
6783506
Documentation of config
davidraskin Aug 18, 2020
da50b42
Merge remote-tracking branch 'upstream/master'
davidraskin Aug 18, 2020
1f05c1d
Integration test + address comments
davidraskin Aug 19, 2020
551a1f4
Format
davidraskin Aug 19, 2020
0fed677
test multiple auditentries + format comment
davidraskin Aug 19, 2020
d9a3d20
Remove test case list + add test to inventory
davidraskin Aug 20, 2020
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
11 changes: 11 additions & 0 deletions extensions/common/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -361,5 +361,16 @@ void populateTCPRequestInfo(bool outbound, RequestInfo* request_info,
request_info->request_protocol = kProtocolTCP;
}

bool getAuditPolicy() {
bool shouldAudit = false;
if (!getValue<bool>(
{"metadata", "filter_metadata", "envoy.common", "access_log_hint"},
&shouldAudit)) {
return false;
}

return shouldAudit;
}

} // namespace Common
} // namespace Wasm
4 changes: 4 additions & 0 deletions extensions/common/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,5 +234,9 @@ void populateExtendedRequestInfo(RequestInfo* request_info);
void populateTCPRequestInfo(bool outbound, RequestInfo* request_info,
const std::string& destination_namespace);

// Read value of 'access_log_hint' key in envoy dynamic metadata which
Comment thread
davidraskin marked this conversation as resolved.
Outdated
// determines whether to audit a request
Comment thread
davidraskin marked this conversation as resolved.
Outdated
bool getAuditPolicy();

} // namespace Common
} // namespace Wasm
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,15 @@ package stackdriver.config.v1alpha1;
import "google/protobuf/duration.proto";
import "google/protobuf/wrappers.proto";

// next id: 10
// next id: 11
message PluginConfig {
// Optional. Controls whether to export server access log.
// Optional. Controls whether to export server access log. This does not
// affect audit logging
bool disable_server_access_logging = 1;

// Optional. Controls whether to export audit log.
bool enable_server_access_auditting = 10;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can we name this audit_server_access ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure.


// Optional. FQDN of destination service that the request routed to, e.g.
// productpage.default.svc.cluster.local. If not provided, request host header
// will be used instead
Expand Down
156 changes: 110 additions & 46 deletions extensions/stackdriver/log/logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,17 @@ using google::protobuf::util::TimeUtil;

// Name of the HTTP server access log.
constexpr char kServerAccessLogName[] = "server-accesslog-stackdriver";
constexpr char kAuditAccessLogName[] = "server-accesslog-stackdriver-audit";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this naming convention important? does it need to match the historic name for the other access logs, or should we consider calling this server-audit-log (or something else less verbose)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's important. I just went the simplest route. I'll rename it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, have you thought about fill in protoPayload in logEntry, which supports google.cloud.audit.AuditLog: https://cloud.google.com/logging/docs/reference/audit/auditlog/rest/Shared.Types/AuditLog. Sound like that is a more standard way to do audit log at Google?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the AuditLog payload is used primarily for auditting GCP service calls, it doesn't apply to application logging, afaik.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bianpengyuan @davidraskin can you find out if google.cloud.audit.AuditLog cannot be used to audit other services?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mandarjog Maybe it can theoretically be used, because I think the only difference is the proto payload, but the Audit Log docs state that it is for GCP API calls. Additionally, configuring the Data Access Audit Logs is done through the Cloud Console, which could be confusing since that wouldn't be the case here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should these logs be actually called audit logs? https://cloud.google.com/logging/docs/audit cloud audit logs looks like are logs for telling who or what accessed a GCP resource... This is more about capturing auth info for application?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They're still audit logs I think. Just ones at the application level, separate from GCP audit logs. We can call them IstioAudit logs to reduce confusion.


Logger::Logger(const ::Wasm::Common::FlatNode& local_node_info,
std::unique_ptr<Exporter> exporter, int log_request_size_limit) {
// Initalize the current WriteLogEntriesRequest.
log_entries_request_ =
std::make_unique<google::logging::v2::WriteLogEntriesRequest>();

audit_entries_request_ =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check this out... I have made it a map: https://github.com/istio/proxy/pull/2955/files#diff-d116ca95f1292dece4d352210d220a13 so would be easier to add many types of entries now...

std::make_unique<google::logging::v2::WriteLogEntriesRequest>();

// Set log names.
const auto platform_metadata = local_node_info.platform_metadata();
const auto project_iter =
Expand All @@ -53,6 +57,9 @@ Logger::Logger(const ::Wasm::Common::FlatNode& local_node_info,
log_entries_request_->set_log_name("projects/" + project_id_ + "/logs/" +
kServerAccessLogName);

audit_entries_request_->set_log_name("projects/" + project_id_ + "/logs/" +
kAuditAccessLogName);

std::string resource_type = Common::kContainerMonitoredResource;
const auto cluster_iter =
platform_metadata
Expand All @@ -68,16 +75,33 @@ Logger::Logger(const ::Wasm::Common::FlatNode& local_node_info,
Common::getMonitoredResource(resource_type, local_node_info,
&monitored_resource);
log_entries_request_->mutable_resource()->CopyFrom(monitored_resource);
audit_entries_request_->mutable_resource()->CopyFrom(monitored_resource);

setCommonLabels(local_node_info, false);
setCommonLabels(local_node_info, true);
log_request_size_limit_ = log_request_size_limit;
exporter_ = std::move(exporter);
}

void Logger::setCommonLabels(const ::Wasm::Common::FlatNode& local_node_info,
bool for_audit) {
auto& entries_request =
(for_audit ? audit_entries_request_ : log_entries_request_);
// Set common labels shared by all entries.
auto label_map = log_entries_request_->mutable_labels();
(*label_map)["destination_name"] =
flatbuffers::GetString(local_node_info.name());
auto label_map = entries_request->mutable_labels();
(*label_map)["destination_workload"] =
flatbuffers::GetString(local_node_info.workload_name());
(*label_map)["destination_namespace"] =
flatbuffers::GetString(local_node_info.namespace_());
(*label_map)["mesh_uid"] = flatbuffers::GetString(local_node_info.mesh_id());

// Don't set if audit request
if (!for_audit) {
(*label_map)["destination_name"] =
flatbuffers::GetString(local_node_info.name());
(*label_map)["mesh_uid"] =
flatbuffers::GetString(local_node_info.mesh_id());
}

// Add destination app and version label if exist.
const auto local_labels = local_node_info.labels();
if (local_labels) {
Expand Down Expand Up @@ -105,45 +129,70 @@ Logger::Logger(const ::Wasm::Common::FlatNode& local_node_info,
flatbuffers::GetString(rev_iter->value());
}
}
log_request_size_limit_ = log_request_size_limit;
exporter_ = std::move(exporter);
}

void Logger::addTcpAuditEntry(const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info,
long int log_time) {
addTcpEntry(request_info, peer_node_info, log_time, true);
}

void Logger::addTcpLogEntry(const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info,
long int log_time) {
addTcpEntry(request_info, peer_node_info, log_time, false);
}

void Logger::addTcpEntry(const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info,
long int log_time, bool for_audit) {
// create a new log entry
auto* log_entries = log_entries_request_->mutable_entries();
auto* new_entry = log_entries->Add();

*new_entry->mutable_timestamp() =
google::protobuf::util::TimeUtil::NanosecondsToTimestamp(log_time);

addTCPLabelsToLogEntry(request_info, new_entry);
fillAndFlushLogEntry(request_info, peer_node_info, new_entry);
addTCPLabelsToEntry(request_info, new_entry);
fillAndFlushEntry(request_info, peer_node_info, new_entry, for_audit);
}

void Logger::addAuditEntry(const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info) {
addEntry(request_info, peer_node_info, true);
}

void Logger::addLogEntry(const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info) {
addEntry(request_info, peer_node_info, false);
}

void Logger::addEntry(const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info,
bool for_audit) {
auto& entries_request =
(for_audit ? audit_entries_request_ : log_entries_request_);
// create a new log entry
auto* log_entries = log_entries_request_->mutable_entries();
auto* log_entries = entries_request->mutable_entries();
auto* new_entry = log_entries->Add();

*new_entry->mutable_timestamp() =
google::protobuf::util::TimeUtil::NanosecondsToTimestamp(
request_info.start_time);
fillHTTPRequestInLogEntry(request_info, new_entry);
fillAndFlushLogEntry(request_info, peer_node_info, new_entry);
fillHTTPRequestInEntry(request_info, new_entry, for_audit);
fillAndFlushEntry(request_info, peer_node_info, new_entry, for_audit);
}

void Logger::fillAndFlushLogEntry(
const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info,
google::logging::v2::LogEntry* new_entry) {
void Logger::fillAndFlushEntry(const ::Wasm::Common::RequestInfo& request_info,
const ::Wasm::Common::FlatNode& peer_node_info,
google::logging::v2::LogEntry* new_entry,
bool for_audit) {
new_entry->set_severity(::google::logging::type::INFO);
auto label_map = new_entry->mutable_labels();

(*label_map)["source_name"] = flatbuffers::GetString(peer_node_info.name());
if (!for_audit) {
(*label_map)["source_name"] = flatbuffers::GetString(peer_node_info.name());
}
(*label_map)["source_workload"] =
flatbuffers::GetString(peer_node_info.workload_name());
(*label_map)["source_namespace"] =
Expand Down Expand Up @@ -176,22 +225,25 @@ void Logger::fillAndFlushLogEntry(

(*label_map)["destination_service_host"] =
request_info.destination_service_host;
(*label_map)["response_flag"] = request_info.response_flag;
(*label_map)["destination_principal"] = request_info.destination_principal;
(*label_map)["source_principal"] = request_info.source_principal;
(*label_map)["service_authentication_policy"] =
std::string(::Wasm::Common::AuthenticationPolicyString(
request_info.service_auth_policy));
(*label_map)["protocol"] = request_info.request_protocol;
(*label_map)["log_sampled"] = request_info.log_sampled ? "true" : "false";
(*label_map)["connection_id"] = std::to_string(request_info.connection_id);
(*label_map)["route_name"] = request_info.route_name;
(*label_map)["upstream_host"] = request_info.upstream_host;
(*label_map)["upstream_cluster"] = request_info.upstream_cluster;
(*label_map)["requested_server_name"] = request_info.request_serever_name;
(*label_map)["x-envoy-original-path"] = request_info.x_envoy_original_path;
(*label_map)["x-envoy-original-dst-host"] =
request_info.x_envoy_original_dst_host;

if (!for_audit) {
(*label_map)["response_flag"] = request_info.response_flag;
(*label_map)["service_authentication_policy"] =
std::string(::Wasm::Common::AuthenticationPolicyString(
request_info.service_auth_policy));
(*label_map)["protocol"] = request_info.request_protocol;
(*label_map)["log_sampled"] = request_info.log_sampled ? "true" : "false";
(*label_map)["connection_id"] = std::to_string(request_info.connection_id);
(*label_map)["route_name"] = request_info.route_name;
(*label_map)["upstream_host"] = request_info.upstream_host;
(*label_map)["upstream_cluster"] = request_info.upstream_cluster;
(*label_map)["requested_server_name"] = request_info.request_serever_name;
(*label_map)["x-envoy-original-path"] = request_info.x_envoy_original_path;
(*label_map)["x-envoy-original-dst-host"] =
request_info.x_envoy_original_dst_host;
}

// Insert trace headers, if exist.
if (request_info.b3_trace_sampled) {
Expand All @@ -203,45 +255,55 @@ void Logger::fillAndFlushLogEntry(

// Accumulate estimated size of the request. If the current request exceeds
// the size limit, flush the request out.
size_ += new_entry->ByteSizeLong();
if (size_ > log_request_size_limit_) {
flush();
auto& req_size = for_audit ? audit_size_ : size_;
req_size += new_entry->ByteSizeLong();
if (req_size > log_request_size_limit_) {
flush(for_audit);
}
}

bool Logger::flush() {
if (size_ == 0) {
bool Logger::flush(bool for_audit) {
auto& req_size = for_audit ? audit_size_ : size_;
auto& entries_request =
for_audit ? audit_entries_request_ : log_entries_request_;

if (req_size == 0) {
// This flush is triggered by timer and does not have any log entries.
return false;
}

// Reconstruct a new WriteLogRequest.
std::unique_ptr<google::logging::v2::WriteLogEntriesRequest> cur =
std::make_unique<google::logging::v2::WriteLogEntriesRequest>();
cur->set_log_name(log_entries_request_->log_name());
cur->mutable_resource()->CopyFrom(log_entries_request_->resource());
*cur->mutable_labels() = log_entries_request_->labels();
cur->set_log_name(entries_request->log_name());
cur->mutable_resource()->CopyFrom(entries_request->resource());
*cur->mutable_labels() = entries_request->labels();

// Swap the new request with the old one and export it.
log_entries_request_.swap(cur);
entries_request.swap(cur);
request_queue_.emplace_back(std::move(cur));

// Reset size counter.
size_ = 0;
// Reset size counters.
req_size = 0;

return true;
}

bool Logger::exportLogEntry(bool is_on_done) {
if (!flush() && request_queue_.empty()) {
// Try to flush both audit and regular log
bool flush_reg = flush(false);
bool flush_audit = flush(true);
if (!flush_reg && !flush_audit && request_queue_.empty()) {
// No log entry needs to export.
return false;
}

exporter_->exportLogs(request_queue_, is_on_done);
request_queue_.clear();
return true;
}

void Logger::addTCPLabelsToLogEntry(
void Logger::addTCPLabelsToEntry(
const ::Wasm::Common::RequestInfo& request_info,
google::logging::v2::LogEntry* log_entry) {
auto label_map = log_entry->mutable_labels();
Expand All @@ -259,9 +321,9 @@ void Logger::addTCPLabelsToLogEntry(
request_info.tcp_connection_state));
}

void Logger::fillHTTPRequestInLogEntry(
void Logger::fillHTTPRequestInEntry(
const ::Wasm::Common::RequestInfo& request_info,
google::logging::v2::LogEntry* log_entry) {
google::logging::v2::LogEntry* log_entry, bool for_audit) {
auto http_request = log_entry->mutable_http_request();
http_request->set_request_method(request_info.request_operation);
http_request->set_request_url(request_info.url_scheme + "://" +
Expand All @@ -278,7 +340,9 @@ void Logger::fillHTTPRequestInLogEntry(
request_info.duration);
http_request->set_referer(request_info.referer);
auto label_map = log_entry->mutable_labels();
(*label_map)["request_id"] = request_info.request_id;
if (!for_audit) {
(*label_map)["request_id"] = request_info.request_id;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

request id is good to have as it helps correlate problems with application logs...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure we can include it then.

}
}

} // namespace Log
Expand Down
Loading