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

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions