Skip to content

[Cisco Ise] Parse TACACS accounting start_time/stop_time with sub-second epoch precision#19894

Open
robester0403 wants to merge 3 commits into
elastic:mainfrom
robester0403:cisco-ise-TACACS-time-fix
Open

[Cisco Ise] Parse TACACS accounting start_time/stop_time with sub-second epoch precision#19894
robester0403 wants to merge 3 commits into
elastic:mainfrom
robester0403:cisco-ise-TACACS-time-fix

Conversation

@robester0403

Copy link
Copy Markdown
Contributor

Proposed commit message

Parse TACACS accounting start_time/stop_time with sub-second epoch precision

In the log data stream, Cisco ISE TACACS accounting emits start_time/stop_time AVPairs as nanosecond epoch values (e.g. start_time=1782407978339676873), but the pipeline parsed them as UNIX (seconds). The result overflowed to year +292278994, which Elasticsearch then rejected at index time — so cisco_ise.log.avpair.start_time/stop_time were silently _ignored and lost.

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

Related issues

Partially Resolves: #19832

Screenshots

Screenshot 2026-06-30 at 5 31 49 PM

@robester0403 robester0403 requested a review from a team as a code owner June 30, 2026 21:45
@robester0403 robester0403 added bugfix Pull request that fixes a bug issue Team:Integration-Experience Security Integrations Integration Experience [elastic/integration-experience] labels Jun 30, 2026
@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

Pinging @elastic/integration-experience (Team:Integration-Experience)

@vera-review-bot

Copy link
Copy Markdown

👀 I have started reviewing the PR

@github-actions

Copy link
Copy Markdown
Contributor

Elastic Docs Style Checker (Vale)

Summary: 1 suggestion found

💡 Suggestions (1): Optional style improvements. Apply when helpful.
File Line Rule Message
packages/cisco_ise/changelog.yml 4 Elastic.Versions Use 'or later' instead of 'or higher' when referring to versions.

The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale.

Comment thread packages/cisco_ise/changelog.yml
@vera-review-bot

Copy link
Copy Markdown

👀 I have started reviewing the PR

@vera-review-bot

Copy link
Copy Markdown

Vera Review Bot

For the current commit state, I did not find any issues.


🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills

⚠️ Automated review — verify suggestions before applying.

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@andrewkroh andrewkroh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider using a script processor to centralize and document the approach (prior art). It's probably more readable. Something like this then a single date processor that works on UNIX_MS. It eliminates the digit counting but uses a regex per doc.

- script:
    description: >-
      Normalize the epoch to milliseconds regardless of source unit (s / ms / µs / ns).
      Cisco ISE TACACS+ accounting emits nanoseconds; older data uses seconds.
    lang: painless
    if: ctx.cisco_ise?.log?.avpair?.start_time instanceof String
    source: |
      def s = ctx.cisco_ise.log.avpair.start_time;
      if (s == "0" || !(s ==~ /^\d+$/)) { return; }
      long v = Long.parseLong(s);
      if      (v >= 1000000000000000000L) { v /= 1000000L; } // ns -> ms
      else if (v >= 1000000000000000L)    { v /= 1000L; }    // µs -> ms
      else if (v <  10000000000L)         { v *= 1000L; }    // s  -> ms
      ctx.cisco_ise.log.avpair.start_time = v;

field: cisco_ise.log.avpair.stop_time
pattern: '^(\d{13})\d+$'
replacement: '$1'
if: ctx.cisco_ise?.log?.avpair?.stop_time instanceof String && ctx.cisco_ise.log.avpair.stop_time.length() > 13

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

start_time has a != "0" guard. stop_time does not. Align them.

- gsub:
tag: gsub_cisco_ise_log_avpair_start_time_18d7aae2
field: cisco_ise.log.avpair.start_time
pattern: '^(\d{13})\d+$'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

None of the new processors have a description. The digit-count magic needs to be explained.

@elastic-vault-github-plugin-prod

Copy link
Copy Markdown

✅ All changelog entries have the correct PR link.

if (s == "0" || !(s ==~ /^\d+$/)) { return; }
long v = Long.parseLong(s);
if (v >= 1000000000000000000L) { v /= 1000000L; } // ns -> ms
else if (v >= 1000000000000000L) { v /= 1000L; } // µs -> ms

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Severity: 🔵 Low confidence: medium path: packages/cisco_ise/data_stream/log/elasticsearch/ingest_pipeline/pipeline_tacacs_accounting.yml:245

The new epoch-normalization script has four unit branches (s/ms/µs/ns) but the pipeline tests only exercise the seconds and nanosecond paths; add a microsecond and a millisecond test input so the µs and ms branches are covered.

Details

The normalize script classifies start_time/stop_time into seconds (<1e10, *1000), milliseconds (1e10-1e15, left as-is), microseconds (>=1e15, /1e3) and nanoseconds (>=1e18, /1e6). The added test record covers the nanosecond branch and the pre-existing records cover the seconds branch, but no fixture exercises the microsecond (>=1e15) or the millisecond (1e10-1e15 pass-through) branches. Those two branches are therefore unverified by the test suite, so a future regression in the boundary arithmetic would not be caught.

Recommendation:

Add TACACS accounting test inputs whose AVPair start_time/stop_time carry microsecond and already-millisecond epoch values, with matching expected timestamps, so every branch of the normalizer is exercised. For example (µs and ms for 2020-03-26T11:30:45Z):

..., AVPair=start_time=1585222245000000, AVPair=stop_time=1585222372000000, ...   # microseconds -> expect 2020-03-26T11:30:45.000Z
..., AVPair=start_time=1585222245000,    AVPair=stop_time=1585222372000,    ...   # milliseconds -> expect 2020-03-26T11:30:45.000Z

🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills

⚠️ Automated review — verify suggestions before applying.

@vera-review-bot

Copy link
Copy Markdown

Review summary

Issues found across the latest commits 8b2255d — 1 low
  • 🔵 The new epoch-normalization script has four unit branches (s/ms/µs/ns) but the pipeline tests only exercise the seconds and nanosecond paths (link) (Unresolved)

A new commit triggers another review — at most once every 15 minutes. I skip the PR while it's approved or has merge conflicts.

🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills

⚠️ Automated review — verify suggestions before applying.

@infra-vault-gh-plugin-prod

Copy link
Copy Markdown

💚 Build Succeeded

History

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

Labels

bugfix Pull request that fixes a bug issue Integration:cisco_ise Cisco ISE Team:Integration-Experience Security Integrations Integration Experience [elastic/integration-experience]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants