Skip to content

Commit

Permalink
Always serialize schemas when available (#2406)
Browse files Browse the repository at this point in the history
  • Loading branch information
Anilm3 authored Dec 5, 2023
1 parent 598b629 commit fecdcfc
Show file tree
Hide file tree
Showing 15 changed files with 47 additions and 39 deletions.
17 changes: 14 additions & 3 deletions appsec/src/extension/commands/client_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,21 @@ static dd_result _pack_command(
mpack_start_map(w, 2);

dd_mpack_write_lstr(w, "enabled");
mpack_write_bool(w, get_global_DD_EXPERIMENTAL_API_SECURITY_ENABLED());

dd_mpack_write_lstr(w, "sample_rate");
mpack_write(w, get_global_DD_API_SECURITY_REQUEST_SAMPLE_RATE());
#define MIN_SE_SAMPLE_RATE 0.0001

double se_sample_rate = get_global_DD_API_SECURITY_REQUEST_SAMPLE_RATE();
if (se_sample_rate >= MIN_SE_SAMPLE_RATE) {
mpack_write_bool(w, true);

dd_mpack_write_lstr(w, "sample_rate");
mpack_write(w, se_sample_rate);
} else {
mpack_write_bool(w, false);

dd_mpack_write_lstr(w, "sample_rate");
mpack_write(w, 0.0);
}

mpack_finish_map(w);

Expand Down
1 change: 0 additions & 1 deletion appsec/src/extension/configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ extern bool runtime_config_first_init;
CONFIG(CUSTOM(STRING), DD_APPSEC_AUTOMATED_USER_EVENTS_TRACKING, "safe", .parser = dd_parse_automated_user_events_tracking) \
CONFIG(STRING, DD_APPSEC_HTTP_BLOCKED_TEMPLATE_HTML, "") \
CONFIG(STRING, DD_APPSEC_HTTP_BLOCKED_TEMPLATE_JSON, "") \
CONFIG(BOOL, DD_EXPERIMENTAL_API_SECURITY_ENABLED, "false") \
CONFIG(DOUBLE, DD_API_SECURITY_REQUEST_SAMPLE_RATE, "0.1")
// clang-format on

Expand Down
10 changes: 0 additions & 10 deletions appsec/src/helper/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -471,16 +471,6 @@ bool client::handle_command(network::request_shutdown::request &command)

response->triggers = std::move(res->events);
response->force_keep = res->force_keep;
for (const auto &[key, value] : res->schemas) {
std::string schema = value;
if (value.length() > max_plain_schema_allowed) {
auto encoded = compress(schema);
if (encoded) {
schema = base64_encode(encoded.value(), false);
}
}
response->meta.emplace(key, std::move(schema));
}

DD_STDLOG(DD_STDLOG_ATTACK_DETECTED);
} else {
Expand Down
1 change: 0 additions & 1 deletion appsec/src/helper/client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ namespace dds {
class client {
public:
// Below this limit the encoding+compression might result on a longer string
static constexpr int max_plain_schema_allowed = 260;
client(std::shared_ptr<service_manager> service_manager,
network::base_broker::ptr &&broker)
: service_manager_(std::move(service_manager)),
Expand Down
5 changes: 1 addition & 4 deletions appsec/src/helper/engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ std::optional<engine::result> engine::context::publish(parameter &&param)

std::vector<std::string> event_data;
std::unordered_set<std::string> event_actions;
std::map<std::string, std::string> schemas;

for (auto &sub : common_->subscribers) {
auto it = listeners_.find(sub);
Expand All @@ -89,7 +88,6 @@ std::optional<engine::result> engine::context::publish(parameter &&param)
std::make_move_iterator(event->data.begin()),
std::make_move_iterator(event->data.end()));
event_actions.merge(event->actions);
schemas.merge(event->schemas);
}
} catch (std::exception &e) {
SPDLOG_ERROR("subscriber failed: {}", e.what());
Expand All @@ -100,8 +98,7 @@ std::optional<engine::result> engine::context::publish(parameter &&param)
return std::nullopt;
}

dds::engine::result res{
action_type::record, {}, std::move(event_data), std::move(schemas)};
dds::engine::result res{action_type::record, {}, std::move(event_data)};
// Currently the only action the extension can perform is block
if (!event_actions.empty()) {
// The extension can only handle one action, so we pick the first one
Expand Down
1 change: 0 additions & 1 deletion appsec/src/helper/engine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ class engine {
action_type type;
std::unordered_map<std::string, std::string> parameters;
std::vector<std::string> events;
std::map<std::string, std::string> schemas;
bool force_keep;
};

Expand Down
3 changes: 1 addition & 2 deletions appsec/src/helper/network/proto.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,9 @@ struct request_shutdown {

std::map<std::string, std::string> meta;
std::map<std::string_view, double> metrics;
std::map<std::string_view, std::string> schemas;

MSGPACK_DEFINE(
verdict, parameters, triggers, force_keep, meta, metrics, schemas);
verdict, parameters, triggers, force_keep, meta, metrics);
};
};

Expand Down
1 change: 0 additions & 1 deletion appsec/src/helper/subscriber/base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class subscriber {
struct event {
std::vector<std::string> data;
std::unordered_set<std::string> actions;
std::map<std::string, std::string> schemas;
};

class listener {
Expand Down
30 changes: 23 additions & 7 deletions appsec/src/helper/subscriber/waf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
//
// This product includes software developed at Datadog
// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc.
#include "../std_logging.hpp"
#include "ddwaf.h"
#include <atomic>
#include <chrono>
#include <cstdlib>
Expand All @@ -18,7 +16,11 @@
#include <string_view>

#include "../json_helper.hpp"
#include "../std_logging.hpp"
#include "../tags.hpp"
#include "base64.h"
#include "compression.hpp"
#include "ddwaf.h"
#include "waf.hpp"

namespace dds::waf {
Expand All @@ -39,11 +41,6 @@ dds::subscriber::event format_waf_result(ddwaf_result &res)
output.data.emplace_back(std::move(parameter_to_json(event)));
}

const parameter_view schemas{res.derivatives};
for (const auto &schema : schemas) {
output.schemas.emplace(
schema.key(), std::move(parameter_to_json(schema)));
}
} catch (const std::exception &e) {
SPDLOG_ERROR("failed to parse WAF output: {}", e.what());
}
Expand Down Expand Up @@ -209,6 +206,11 @@ std::optional<subscriber::event> instance::listener::call(
// NOLINTNEXTLINE
total_runtime_ += res.total_runtime / 1000.0;

const parameter_view schemas{res.derivatives};
for (const auto &schema : schemas) {
schemas_.emplace(schema.key(), std::move(parameter_to_json(schema)));
}

switch (code) {
case DDWAF_MATCH:
return format_waf_result(res);
Expand Down Expand Up @@ -236,6 +238,20 @@ void instance::listener::get_meta_and_metrics(
{
meta[std::string(tag::event_rules_version)] = ruleset_version_;
metrics[tag::waf_duration] = total_runtime_;

for (const auto &[key, value] : schemas_) {
std::string schema = value;
if (value.length() > max_plain_schema_allowed) {
auto encoded = compress(schema);
if (encoded) {
schema = base64_encode(encoded.value(), false);
}
}

if (schema.length() <= max_schema_size) {
meta.emplace(key, std::move(schema));
}
}
}

instance::instance(parameter &rule, std::map<std::string, std::string> &meta,
Expand Down
4 changes: 3 additions & 1 deletion appsec/src/helper/subscriber/waf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ void initialise_logging(spdlog::level::level_enum level);
class instance : public dds::subscriber {
public:
static constexpr int default_waf_timeout_us = 10000;

static constexpr int max_plain_schema_allowed = 260;
static constexpr int max_schema_size = 25000;
using ptr = std::shared_ptr<instance>;
class listener : public dds::subscriber::listener {
public:
Expand All @@ -46,6 +47,7 @@ class instance : public dds::subscriber {
std::chrono::microseconds waf_timeout_;
double total_runtime_{0.0};
std::string_view ruleset_version_;
std::map<std::string, std::string> schemas_;
};

// NOLINTNEXTLINE(google-runtime-references)
Expand Down
3 changes: 0 additions & 3 deletions appsec/tests/extension/api_security_env_variables.phpt
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
--TEST--
Set and test API security ini settings
--ENV--
DD_EXPERIMENTAL_API_SECURITY_ENABLED=1
DD_API_SECURITY_REQUEST_SAMPLE_RATE=0.8
--FILE--
<?php
var_dump(ini_get("datadog.experimental_api_security_enabled"));
var_dump(ini_get("datadog.api_security_request_sample_rate"));
?>
--EXPECTF--
string(1) "1"
string(3) "0.8"
Binary file modified appsec/tests/extension/rinit_rshutdown_basic.phpt
Binary file not shown.
3 changes: 1 addition & 2 deletions appsec/tests/helper/broker_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ TEST(BrokerTest, SendRequestShutdown)
packer.pack_array(1); // Array of messages
packer.pack_array(2); // First message
pack_str(packer, "request_shutdown"); // Type
packer.pack_array(7);
packer.pack_array(6);
pack_str(packer, "block");
packer.pack_map(2);
pack_str(packer, "type");
Expand All @@ -179,7 +179,6 @@ TEST(BrokerTest, SendRequestShutdown)
packer.pack_true(); // Force keep
packer.pack_map(0);
packer.pack_map(0);
packer.pack_map(0);
const auto &expected_data = ss.str();

network::header_t h;
Expand Down
2 changes: 1 addition & 1 deletion appsec/tests/helper/client_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2365,7 +2365,7 @@ TEST(ClientTest, SchemasOverTheLimitAreCompressed)
int body_length = 0;
int i = 0;
// Lets generate a body which goes over max_plain_schema_allowed limit
while (body_length < client::max_plain_schema_allowed) {
while (body_length < waf::instance::max_plain_schema_allowed) {
body.add(std::to_string(i), parameter::int64(i));
auto schema = parameter::array();
schema.add(parameter::int64(4));
Expand Down
5 changes: 3 additions & 2 deletions appsec/tests/helper/waf_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,8 @@ TEST(WafTest, SchemasAreAdded)
EXPECT_FALSE(doc.HasParseError());
EXPECT_TRUE(doc.IsObject());

EXPECT_FALSE(res->schemas.empty());
EXPECT_STREQ(res->schemas["_dd.appsec.s.arg2"].c_str(), "[8]");
ctx->get_meta_and_metrics(meta, metrics);
EXPECT_FALSE(meta.empty());
EXPECT_STREQ(meta["_dd.appsec.s.arg2"].c_str(), "[8]");
}
} // namespace dds

0 comments on commit fecdcfc

Please sign in to comment.