Skip to content

AMI: manager.conf read=/write= permissions are ignored — every user is granted ALL read+write (fail-open default) #127

Description

@ryanmurf

Summary

The manager.conf parser silently discards the read= and write= permission directives. Every AMI user is constructed via AmiUser::new, which hard-codes read_perm = ALL and write_perm = ALL. An operator who configures a restricted (e.g. monitoring-only) AMI account gets a full-privilege account instead.

Location

  • crates/rustisk-cli/src/main.rs:1182-1184 — the user-section branch of the parser:
    Some(_) => {
        // read/write permissions are handled via AmiUser::new (defaults to ALL)
    }
    read=/write= lines fall into this no-op arm and are dropped.
  • crates/asterisk-ami/src/auth.rs:36-37AmiUser::new sets read_perm: EventCategory::ALL, write_perm: EventCategory::ALL.

Attack / impact

Operator writes:

[monitor]
secret = ...
read = system,call
write =            ; intended: no write access

The parser ignores both lines; monitor is created with read=ALL, write=ALL. The operator believes they provisioned a least-privilege account but shipped an admin one. This is a fail-open default (Asterisk's semantics are the opposite: an omitted write grants nothing).

Note the read side is actually enforced downstream (AmiSession::should_receive_event honors read_perm), so the read grant is real too — a "read=system" user still receives every event category. The write side is doubly moot today because dispatch never checks write_perm at all (see companion issue #126).

Recommended fix

  1. Parse read=/write= in the user section with EventCategory::parse_list and build users via AmiUser::with_permissions.
  2. Decide the default when a directive is absent. Asterisk-correct is NONE; but flipping the default to deny is the part that can break existing minimal configs, so gate that behind the same coordinated change as AMI: write-permission (write_perm) is never enforced — any authenticated user can Originate / run CLI Command / write config #126 and call it out in release notes. At minimum, honor an explicitly-provided read=/write= immediately (that alone is non-breaking and lets operators restrict).

Ship together with #126 (enforcement) — parsing perms without enforcing them, or enforcing without parsing them, each accomplishes nothing on its own.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions