Skip to content

Handle oversized accounting arguments (>255 bytes) gracefully #185

Description

@Rod-Persky

Summary

When an accounting request is sent, individual arguments may exceed 255 bytes. The TACACS+ wire format encodes each argument's length as a single u8, so any argument longer than 255 bytes cannot fit in a single arg field. Currently, build_accounting_packet (or the underlying serialization) will fail with a generic error via ? — no context about which argument failed or why.

Problem

  1. A user-supplied argument (e.g. a long command line like cmd=<256+ chars>) silently causes serialization failure.
  2. The ? propagation provides no diagnostic context — the caller gets a bare conversion error rather than a message identifying the problematic argument.
  3. There is no policy decision about whether to split or reject.

Options

Option A: Split long arguments across multiple indexed fields

Split an oversized argument into continuation fields:

cmd=<first 255 bytes>     -> arg[i]
cmd-cont=<next chunk>     -> arg[i+1]
...

Pros: Preserves data, server gets the full value.
Cons: Requires server-side reassembly; no RFC 8907 standard for continuation semantics. Interop risk with third-party TACACS+ servers that don't understand the convention.

Option B: Reject with a clear error

Return an error from build_accounting_packet with context identifying which argument index and key exceeded the limit.

Pros: Simple, explicit, no interop risk. Callers can decide policy (truncate, skip, etc.).
Cons: Callers must handle the error and decide what to do.

Option C: Truncate with a warning

Truncate the argument value at 255 bytes and log a warning identifying the truncated argument.

Pros: Request still goes through.
Cons: Silent data loss; may cause incorrect accounting records on the server.

Recommendation

Option B as the default — reject with a contextual error. This keeps the protocol layer honest and pushes policy decisions to callers. If a specific caller (e.g. tacon, the agent) wants truncation or splitting, it can implement that before calling the flow.

Plan

Step 1: Add context to the serialization failure

File: libraries/tacacsrs_flow_abstractions/src/accounting.rs (or wherever build_accounting_packet assembles args)

Ensure that when an argument exceeds 255 bytes, the error includes:

  • The argument index
  • The argument key (portion before =, if present)
  • The actual length
  • The 255-byte limit

Example:

anyhow::bail!(
    "Accounting argument {index} (key={key:?}) is {len} bytes, exceeding the 255-byte TACACS+ arg field limit"
);

Step 2: Validate arguments before serialization

Add explicit validation in AccountingRequest::to_bytes() or build_accounting_packet that checks each argument's byte length before attempting to write. This gives a clear, early error rather than a mysterious u8 conversion failure deep in serialization.

Step 3: Consider a helper for callers that want truncation

Optionally, provide a utility function that truncates or splits arguments for callers that prefer lossy behavior:

/// Truncates arguments exceeding the TACACS+ 255-byte limit.
/// Returns the truncated arguments and logs warnings for any that were modified.
pub fn truncate_oversized_args(args: &[String]) -> Vec<String>;

This is not required for the initial fix but could be added later.

Step 4: Add tests

  • Test that a 256-byte argument produces a clear error from build_accounting_packet.
  • Test that a 255-byte argument succeeds.
  • Test that the error message includes the argument index and key.

Step 5: Verify

cargo test -p tacacsrs-flow-abstractions -p tacacsrs-flows --all-features
cargo clippy -p tacacsrs-flow-abstractions -p tacacsrs-flows --all-targets --all-features -- -D warnings

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions