namib-project / nftables-rs

Safe abstraction for nftables JSON API (libnftables-json).
https://crates.io/crates/nftables
Apache License 2.0
23 stars 13 forks source link

Panic when trying to delte a table that does not exist #7

Closed hnorkowski closed 11 months ago

hnorkowski commented 1 year ago

When I try to delete a table that does not exist the crate panics instead of returning an Result::Err as expected

Example Code:

fn remove_nftable() {
    let mut batch = Batch::new();

    batch.delete(schema::NfListObject::Table(schema::Table::new(
        types::NfFamily::IP6,
        "i-do-not-exist".to_string(),
    )));

    let ruleset = batch.to_nftables();

    match nftables::helper::apply_ruleset(&ruleset, None, None) {
        Ok(_) => println!("Removed Rule!"),
        Err(err) => println!("Could not remove rule: {:?}", err),
    };
}

Error:

internal:0:0-0: Error: Could not process rule: No such file or directory

thread 'main' panicked at 'assertion failed: output.status.success()', /home/hendrik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nftables-0.2.3/src/helper.rs:74:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:117:5
   3: nftables::helper::apply_ruleset_raw
             at /home/hendrik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nftables-0.2.3/src/helper.rs:74:13
   4: nftables::helper::apply_ruleset
             at /home/hendrik/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nftables-0.2.3/src/helper.rs:43:5
   5: rust_testing::remove_nftable
             at ./src/main.rs:149:11
   6: rust_testing::main
             at ./src/main.rs:22:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
JKRhb commented 1 year ago

Hi @hnorkowski, thank you for opening this issue! :) @bits0rcerer has provided a potential fix in #8 – could you try out the latest crate version and check if the problem has been resolved?

jwhb commented 1 year ago

@hnorkowski thanks for reporting. As @JKRhb pointed out @bits0rcerer improved the error handling in the helper functions.

I confirmed that your code will not panic and added a test based on your issue: https://github.com/namib-project/nftables-rs/blob/1bd71ea9923ea859b89e90e8c8b795768c5b91ea/tests/helper_tests.rs#L23-L37

Looking at your use case I assume that you want to ensure a table is deleted. An error by nft is expected if you attempt to delete a non-existing table. As a safe way to ensure a table does not exist, you can add instructions to first create the table and then delete it immediately.

This is how we solved this in another project:

https://github.com/namib-project/mud-controller-enforcer/blob/main/namib_enforcer/src/services/firewall_service.rs#L223-L233

We used this strategy to reset a table.

jwhb commented 11 months ago

I'm closing this issue, as the #8 should fix it.

Please reopen if this does not work for you.