Skip to content

Add case for PAM locked account message#19168

Open
Tacklebox wants to merge 1 commit into
mainfrom
mborden/pam_faillock
Open

Add case for PAM locked account message#19168
Tacklebox wants to merge 1 commit into
mainfrom
mborden/pam_faillock

Conversation

@Tacklebox
Copy link
Copy Markdown
Contributor

@Tacklebox Tacklebox commented May 22, 2026

Proposed commit message

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@Tacklebox Tacklebox requested a review from a team as a code owner May 22, 2026 11:50
@andrewkroh andrewkroh added Integration:system System Team:Security-Linux Platform Linux Platform Security team [elastic/sec-linux-platform] labels May 22, 2026
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform)

@elastic-vault-github-plugin-prod
Copy link
Copy Markdown

🚀 Benchmarks report

Package system 👍(1) 💚(0) 💔(2)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
auth 6329.11 5208.33 -1120.78 (-17.71%) 💔
security 1213.59 892.86 -320.73 (-26.43%) 💔

To see the full report comment with /test benchmark fullreport

@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

Copy link
Copy Markdown

@ricardoamaro ricardoamaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent is correct and the field values produced are right, but there are two blockers before this can merge.

Blocker 1: Performance regression (inline comment on line 38)

The new grok pattern uses %{DATA} inside grok-pam-users, which runs on every auth message without a pam_faillock( guard. The benchmark already shows -17.71% EPS on auth and -26.43% on security. The fix: a dedicated processor inserted before grok-pam-users, gated on ctx.message.contains('pam_faillock('), using %{USERNAME} instead of %{DATA}. See inline comment for the exact YAML.

Blocker 2: No test cases

changed_files: 1 — only message.yml was modified. There are no new pipeline test fixtures for a pam_faillock locked-account message. Without a test, there is no CI-level verification that user.name, event.outcome, event.action, and event.category are populated correctly. Add at least one .log + -expected.json pair to packages/system/data_stream/auth/_dev/test/pipeline/ with a message like:

pam_faillock(sshd:auth): Consecutive login failures for user testuser account temporarily locked

Also:

  • changelog.yml entry is missing (checklist box is ticked but the diff does not include any changelog change).
  • Two minor issues flagged inline: outcome condition specificity (line 147) and account_lockedaccount-locked (line 167).

ignore_failure: true
patterns:
- 'for user %{QUOTE}?%{DATA:_temp.foruser}%{QUOTE}? by %{QUOTE}?%{DATA:_temp.byuser}%{QUOTE}?(?:\(uid=%{NUMBER:_temp.byuid}\))?$'
- 'pam_faillock\([^)]+\): %{DATA} for user %{USERNAME:_temp.foruser} account temporarily locked'
Copy link
Copy Markdown

@ricardoamaro ricardoamaro May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] %{DATA} in unguarded processor, root cause of the EPS regression

This pattern is added to the grok-pam-users processor, which runs on every auth message (the only guard is ctx.message != null && ctx.message != ""). %{DATA} resolves to .*? and requires backtracking on every non-matching message.

The fix is a dedicated processor placed before grok-pam-users, gated on pam_faillock(, using %{USERNAME} instead of %{DATA}:

- grok:
    description: Grok username from pam_faillock locked account messages.
    tag: grok-pam-faillock-users
    field: message
    ignore_missing: true
    ignore_failure: true
    if: ctx.message != null && ctx.message.contains('pam_faillock(')
    patterns:
      - 'for user %{USERNAME:_temp.pam_user} account temporarily locked'

This also avoids a correctness risk: the existing for user %{DATA:_temp.foruser}$ pattern (pattern 3 in this same processor) would still fire for pam_faillock messages that don't match the new pattern, potentially writing "test account temporarily locked" into user.name. Inserting before grok-pam-users prevents that entirely.

tag: set_outcome_failure_account_locked
field: event.outcome
value: failure
if: ctx.message != null && ctx.message.contains('account temporarily locked')
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] Outcome condition is less specific than the action condition

This fires on any message containing 'account temporarily locked', including potential non-pam_faillock sources. The action setter a few processors later (tagged set_action_account_locked) correctly also requires pam_faillock(. Make these consistent:

if: ctx.message != null && ctx.message.contains('pam_faillock(') && ctx.message.contains('account temporarily locked')

- set:
tag: set_action_account_locked
field: event.action
value: account_locked
Copy link
Copy Markdown

@ricardoamaro ricardoamaro May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] Underscore vs hyphen. Inconsistent with existing action values

All other action values in this pipeline use hyphens: logged-on, logged-off. This should be:

value: account-locked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Integration:system System Team:Security-Linux Platform Linux Platform Security team [elastic/sec-linux-platform]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants