diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9e7646da8..68253e515 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -394,6 +394,8 @@ jobs: uses: softprops/action-gh-release@c062e08bd532815e2082a85e87e3ef29c3e6d191 # v2.0.8 with: draft: true + name: v${{ github.ref_name }} + body_path: ${{ github.workspace }}/docs/changelog/CHANGELOG-latest.md files: | ./artifacts/**/*.tar.gz ./artifacts/**/*.sha256 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0fc547ac1..d49804e34 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -209,11 +209,9 @@ jobs: name: coverage_${{ matrix.suffix }}_${{ matrix.arch }} path: ${{ github.workspace }}/Debug/coverage-${{ matrix.suffix }}-${{ matrix.arch }}.json - upload-coverage: + generate-coverage: needs: [ coverage ] runs-on: ubuntu-24.04 - environment: - name: dd-protected-coverage steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 with: @@ -223,10 +221,6 @@ jobs: with: path: artifacts - - uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 - with: - node-version: 24 - - name: Install dependencies run: | sudo apt update @@ -234,7 +228,7 @@ jobs: python3 -m venv .venv source .venv/bin/activate pip install gcovr==7.2 - npm install -g @datadog/datadog-ci + - name: Generate coverage run: | source .venv/bin/activate @@ -243,10 +237,28 @@ jobs: mkdir -p coverage gcovr --merge-mode-functions merge-use-line-0 --json-add-tracefile "artifacts/*/coverage-*.json" --html-details coverage/coverage.html - - name: Submit coverage (DataDog) - run: datadog-ci coverage upload coverage.xml - env: - DD_API_KEY: ${{ secrets.DD_API_KEY }} + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 + with: + name: coverage + path: ${{ github.workspace }}/coverage/ + + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 + with: + name: coverage-xml + path: ${{ github.workspace }}/coverage.xml + + upload-to-codecov: + needs: [ generate-coverage ] + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + with: + submodules: recursive + + - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 + with: + name: coverage-xml + path: . - name: Submit coverage (CodeCov) uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 @@ -255,10 +267,34 @@ jobs: flags: waf_test verbose: true files: coverage.xml - - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 + + upload-to-datadog: + needs: [ generate-coverage ] + runs-on: ubuntu-24.04 + if: github.ref == 'refs/heads/master' + environment: + name: dd-protected-coverage + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 with: - name: coverage - path: ${{ github.workspace }}/coverage/ + submodules: recursive + + - uses: actions/download-artifact@634f93cb2916e3fdff6788551b99b062d0335ce0 + with: + name: coverage-xml + path: . + + - uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 + with: + node-version: 24 + + - name: Install datadog-ci + run: npm install -g @datadog/datadog-ci + + - name: Submit coverage (DataDog) + run: datadog-ci coverage upload coverage.xml + env: + DD_API_KEY: ${{ secrets.DD_API_KEY }} lint: runs-on: ubuntu-24.04 diff --git a/BINDING_IMPL_NOTES.md b/BINDING_IMPL_NOTES.md deleted file mode 100644 index 49919f8e5..000000000 --- a/BINDING_IMPL_NOTES.md +++ /dev/null @@ -1,158 +0,0 @@ -# Implementation notes for bindings - - -## `ddwaf_handle` - -`ddwaf_handle` represents a rule set, with associated rule data, exclusions, -and so on. It is created through `ddwaf_init` and `ddwaf_update`. - -A tracer will want to keep only one live `ddwaf_handle`. The first one will be -created through `ddwaf_init`, and then it will replace it through -`ddwaf_update` as it gets new configurations through remote config. - -The two relevant operations on `ddwaf_handle` are `ddwaf_context_init` and -`ddwaf_update`. Neither of these operations changes the `ddwaf_handle`. So, at -this point, it is sufficient to ensure that threads calling -`ddwaf_context_init` and `ddwaf_update` see a fully constructed `ddwaf_handle` -— put in other words, it is sufficient that the write to the pointer/reference -happens after `ddwaf_init/update` is called (as seen by other threads). For -this, a release-acquire operation suffices: - -```c++ -std::atomic cur_ddwaf_handle; // global variable; the live handle - -// Initialization thread -void initialize_handle( - const ddwaf_object *ruleset, const ddwaf_config *config, ddwaf_object *diagnostics) -{ - ddwaf_handle new_handle = ddwaf_init(ruleset, config, diagnostics); - if (new_handle == nullptr) { /* handle error */} - cur_ddwaf_handle.store(new_handle, std::memory_order_release); -} - -// Request thread -ddwaf_context create_context() { - ddwaf_handle handle = cur_ddwaf_handle.load(std::memory_order_acquire); - if (handle == nullptr) { /* handle error */ } - return ddwaf_context_init(handle); -} -``` - -However, there is a potential problem when you update the live handle. While -libddwaf refcounts the `ddwaf_handle` and ensures that its memory is not -reclaimed until all associated `ddwaf_context`s have been destroyed, it cannot -prevent a use-after-free in this situation: - -```c++ -// Remote config thread -void update_handle(const ddwaf_object *ruleset, ddwaf_object *diagnostics) -{ - ddwaf_handle old_handle = cur_ddwaf_handle.load(std::memory_order_acquire); - ddwaf_handle new_handle = ddwaf_update(old_handle, ruleset, diagnostics); - cur_ddwaf_handle.store(new_handle, std::memory_order_release); - - // will free the memory if no ddwaf_contexts are associated with it: - ddwaf_destroy(old_handle); -} - -// Request thread -ddwaf_context create_context() { - ddwaf_handle handle = cur_ddwaf_handle.load(std::memory_order_acquire); - if (handle == nullptr) { /* handle error */ } - // XXX: the handle fetched may have been destroyed in the interim - return ddwaf_context_init(handle); -} -``` - -That is, the old `ddwaf_handle` can only be destroyed once we can be guaranteed -that no other thread will try to create a new context from it (or update it -through `ddwaf_update`, though that is less of a problem because generally the -tracers will not want to update the global `ddwaf_handle` from several threads -simultaneously). - -There are many possible strategies to deal with this memory reclamation -problem, but the most straightforward are: - -* If the runtime uses garbage collection, we can delay the call to - `ddwaf_destroy` until the garbage collector determines the object wrapping - the native `ddwaf_handle` has no more references. This is relatively easy to - do and doesn't involve extra usage of locks, but the memory used by the - `ddwaf_handle` will not be available until the garbage collector decides - to reclaim the wrapped object. Because the garbage collector does not see - the memory being used by the `ddwaf_handle`, if garbage collection never - happens, there is a risk that memory consumption gets too high. This is a - rather unlikely scenario though. - -* You can use read-write locks: - -```c++ -// global variables -std::shared_mutex mutex; -ddwaf_handle cur_ddwaf_handle; - -// Initialization thread -void initialize_handle( - const ddwaf_object *ruleset, const ddwaf_config *config, ddwaf_object *diagnostics) -{ - ddwaf_handle new_handle = ddwaf_init(ruleset, config, diagnostics); - if (new_handle == nullptr) { /* handle error */} - - std::unique_lock lock{mutex}; // acquire write lock - cur_ddwaf_handle = new_handle; - // write lock released on return -} - -// Remote config thread -void update_handle(const ddwaf_object *ruleset, ddwaf_object *diagnostics) -{ - std::unique_lock lock{mutex}; // acquire write lock - ddwaf_handle old_handle = cur_ddwaf_handle.load(std::memory_order_acquire); - ddwaf_handle new_handle = ddwaf_update(old_handle, ruleset, diagnostics); - cur_ddwaf_handle = new_handle; - ddwaf_destroy(old_handle); - // write lock released on return -} - -// Request thread -ddwaf_context create_context() { - std::shared_lock lock{mutex}; // acquire read lock - if (cur_ddwaf_handle == nullptr) { /* handle error */ } - return ddwaf_context_init(cur_ddwaf_handle); - // read lock released on return -} -``` - -## `ddwaf_context` - -On the other hand, `ddwaf_context` is not thread-safe. If a `ddwaf_context` is -used by multiple threads (in web servers where the processing of the request -can move between several threads, or happen in several threads simultaneously), -you need to use locks so that calls to `ddwaf_context_eval/destroy` are not run -concurrently, and that changes made to the `ddwaf_context` in one thread through -`ddwaf_context_eval/destroy` are visible to the other threads subsequently run -`ddwaf_context_eval/destroy` on the same context. You also need to ensure that no calls -to `ddwaf_context_eval/destroy` happen after `ddwaf_destroy` is called. - -```c++ -// each request will have one of these associated with it -struct ctx_wrapper { - std::mutex mutex; - ddwaf_context ctx; -}; - -DDWAF_RET_CODE run_waf( - ctx_wrapper &wrapper, ddwaf_object *data, ddwaf_object *result, uint64_t timeout) -{ - std::lock_guard lock{wrapper.mutex}; // acquire exclusive lock - if (wrapper.ctx == nullptr) { /* context already destroyed */ } - return ddwaf_context_eval(wrapper.ctx, data, result, timeout); - // lock is released on return -} - -void destroy_ctx(ctx_wrapper &wrapper) { - std::lock_guard lock{wrapper.mutex}; - if (wrapper.ctx == nullptr) { /* context already destroyed */ } - ddwaf_context_destroy(wrapper.ctx); - wrapper.ctx = nullptr; -} -``` diff --git a/README.md b/README.md index 6a8617470..b7658106a 100644 --- a/README.md +++ b/README.md @@ -100,28 +100,47 @@ rules: int main() { + auto alloc = ddwaf_get_default_allocator(); + YAML::Node doc = YAML::Load(waf_rule.data()); - auto rule = doc.as();//= convert_yaml_to_args(doc); - ddwaf_handle handle = ddwaf_init(&rule, nullptr, nullptr); - ddwaf_object_free(&rule); + auto rule = doc.as(); + + ddwaf_builder builder = ddwaf_builder_init(); + ddwaf_object diagnostics; + bool success = ddwaf_builder_add_or_update_config(builder, "config", 6, &rule, &diagnostics); + ddwaf_object_destroy(&rule, alloc); + ddwaf_object_destroy(&diagnostics, alloc); + + if (!success) { + ddwaf_builder_destroy(builder); + return EXIT_FAILURE; + } + + ddwaf_handle handle = ddwaf_builder_build_instance(builder); + ddwaf_builder_destroy(builder); + if (handle == nullptr) { return EXIT_FAILURE; } - ddwaf_context context = ddwaf_context_init(handle); + ddwaf_context context = ddwaf_context_init(handle, alloc); if (context == nullptr) { ddwaf_destroy(handle); return EXIT_FAILURE; } - ddwaf_object root, tmp; - ddwaf_object_map(&root); - ddwaf_object_map_add(&root, "arg1", ddwaf_object_string(&tmp, "string 1")); - ddwaf_object_map_add(&root, "arg2", ddwaf_object_string(&tmp, "string 2")); + ddwaf_object root; + ddwaf_object_set_map(&root, 2, alloc); + + ddwaf_object *arg1 = ddwaf_object_insert_literal_key(&root, "arg1", 4, alloc); + ddwaf_object_set_string_literal(arg1, "string 1", 8); + + ddwaf_object *arg2 = ddwaf_object_insert_literal_key(&root, "arg2", 4, alloc); + ddwaf_object_set_string_literal(arg2, "string 2", 8); ddwaf_object ret; - auto code = ddwaf_context_eval(context, &root, nullptr, &ret, 1000000 /* microseconds */); + auto code = ddwaf_context_eval(context, &root, alloc, &ret, 1000000 /* microseconds */); std::cout << "Output second run: " << code << '\n'; if (code == DDWAF_MATCH) { YAML::Emitter out(std::cout); @@ -131,7 +150,7 @@ int main() out << object_to_yaml(ret); } - ddwaf_object_free(&ret); + ddwaf_object_destroy(&ret, alloc); ddwaf_context_destroy(context); ddwaf_destroy(handle); @@ -151,62 +170,71 @@ struct as_if { explicit as_if(const Node& node_) : node(node_) {} - static ddwaf_object yaml_to_object_helper(const Node& node) + static ddwaf_object yaml_to_object_helper(const Node& node, ddwaf_allocator alloc) { ddwaf_object arg; switch (node.Type()) { case NodeType::Sequence: - ddwaf_object_array(&arg); + ddwaf_object_set_array(&arg, 0, alloc); break; case NodeType::Map: - ddwaf_object_map(&arg); + ddwaf_object_set_map(&arg, 0, alloc); break; case NodeType::Scalar: - ddwaf_object_string(&arg, node.Scalar().c_str()); + { + auto scalar = node.Scalar(); + ddwaf_object_set_string(&arg, scalar.c_str(), scalar.length(), alloc); break; + } case NodeType::Null: - ddwaf_object_null(&arg); + ddwaf_object_set_null(&arg); break; case NodeType::Undefined: default: - ddwaf_object_invalid(&arg); + ddwaf_object_set_invalid(&arg); break; } return arg; } - ddwaf_object operator()() const { - std::list> stack; + ddwaf_object operator()() const + { + auto alloc = ddwaf_get_default_allocator(); + std::list> stack; - ddwaf_object root = yaml_to_object_helper(node); + ddwaf_object root = yaml_to_object_helper(node, alloc); if (root.type == DDWAF_OBJ_MAP || root.type == DDWAF_OBJ_ARRAY) { - stack.emplace_back(root, node, node.begin()); + stack.emplace_back(&root, node, node.begin()); } while (!stack.empty()) { auto current_depth = stack.size(); auto &[parent_obj, parent_node, it] = stack.back(); - for (;it != parent_node.end(); ++it) { + for (; it != parent_node.end(); ++it) { YAML::Node child_node = parent_node.IsMap() ? it->second : *it; - auto child_obj = yaml_to_object_helper(child_node); - if (parent_obj.type == DDWAF_OBJ_MAP) { + auto child_obj = yaml_to_object_helper(child_node, alloc); + ddwaf_object *child_ptr = nullptr; + if (parent_obj->type == DDWAF_OBJ_MAP) { auto key = it->first.as(); - ddwaf_object_map_add(&parent_obj, key.c_str(), &child_obj); - } else if (parent_obj.type == DDWAF_OBJ_ARRAY) { - ddwaf_object_array_add(&parent_obj, &child_obj); + child_ptr = ddwaf_object_insert_key(parent_obj, key.c_str(), key.size(), alloc); + *child_ptr = child_obj; + } else if (parent_obj->type == DDWAF_OBJ_ARRAY) { + child_ptr = ddwaf_object_insert(parent_obj, alloc); + *child_ptr = child_obj; } if (child_obj.type == DDWAF_OBJ_MAP || child_obj.type == DDWAF_OBJ_ARRAY) { - auto &child_ptr = parent_obj.array[parent_obj.nbEntries - 1]; stack.emplace_back(child_ptr, child_node, child_node.begin()); ++it; break; } } - if (current_depth == stack.size()) { stack.pop_back(); } + if (current_depth == stack.size()) { + stack.pop_back(); + } } return root; } @@ -228,19 +256,25 @@ YAML::Node object_to_yaml_helper(const ddwaf_object &obj) YAML::Node output; switch (obj.type) { case DDWAF_OBJ_BOOL: - output = obj.boolean; + output = ddwaf_object_get_bool(&obj); break; case DDWAF_OBJ_SIGNED: - output = obj.intValue; + output = ddwaf_object_get_signed(&obj); break; case DDWAF_OBJ_UNSIGNED: - output = obj.uintValue; + output = ddwaf_object_get_unsigned(&obj); break; case DDWAF_OBJ_FLOAT: - output = obj.f64; + output = ddwaf_object_get_float(&obj); break; case DDWAF_OBJ_STRING: - output = std::string{obj.stringValue, obj.nbEntries}; + case DDWAF_OBJ_LITERAL_STRING: + case DDWAF_OBJ_SMALL_STRING: + { + size_t length; + const char* str = ddwaf_object_get_string(&obj, &length); + output = std::string{str, length}; + } break; case DDWAF_OBJ_MAP: output = YAML::Load("{}"); @@ -260,7 +294,7 @@ YAML::Node object_to_yaml_helper(const ddwaf_object &obj) YAML::Node object_to_yaml(const ddwaf_object &obj) { - std::list> stack; + std::list> stack; YAML::Node root = object_to_yaml_helper(obj); if (obj.type == DDWAF_OBJ_MAP || obj.type == DDWAF_OBJ_ARRAY) { @@ -271,21 +305,34 @@ YAML::Node object_to_yaml(const ddwaf_object &obj) auto current_depth = stack.size(); auto &[parent_obj, parent_node, index] = stack.back(); - for (;index type == DDWAF_OBJ_MAP || child_obj->type == DDWAF_OBJ_ARRAY) { + stack.emplace_back(*child_obj, child_node, 0); + ++index; + break; + } } else if (parent_obj.type == DDWAF_OBJ_ARRAY) { + child_obj = ddwaf_object_at_value(&parent_obj, index); + auto child_node = object_to_yaml_helper(*child_obj); parent_node.push_back(child_node); - } - if (child_obj.type == DDWAF_OBJ_MAP || child_obj.type == DDWAF_OBJ_ARRAY) { - stack.emplace_back(child_obj, child_node, 0); - ++index; - break; + if (child_obj->type == DDWAF_OBJ_MAP || child_obj->type == DDWAF_OBJ_ARRAY) { + stack.emplace_back(*child_obj, child_node, 0); + ++index; + break; + } } } @@ -317,39 +364,25 @@ rules: ``` applied to the `http.server.query` value `http://localhost/?q=