Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NPE in auditing authenticationSuccess for non-existing run-as user #91171

Merged
merged 3 commits into from
Nov 1, 2022

Conversation

ywangd
Copy link
Member

@ywangd ywangd commented Oct 28, 2022

When run-as fails because the target user does not exist, the authentication is created with a null lookup realm. It is then rejected at authorization time. But for authentication, it is treated as success. This can lead to NPE when auditing the authenticationSuccess event.

This PR fixes the NPE by checking whether lookup realm is null before using it.

Relates: #91126 (comment)

…xist

When run-as fails because the target user does not exist, the
authentication is created with a null lookup realm. It is then rejected
at authorization time. But for authentication, it is treated as success.
This can lead to NPE when auditing the authenticationSuccess event.

This PR fixes the NPE by checking whether lookup realm is null before
using it.

Relates: elastic#91126 (comment)
@ywangd ywangd requested a review from n1v0lg October 28, 2022 05:06
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Oct 28, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Collaborator

Hi @ywangd, I've created a changelog YAML for you.

Copy link
Contributor

@n1v0lg n1v0lg left a comment

Choose a reason for hiding this comment

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

LGTM

Test failures are unrelated. I'm hitting them on my PRs as well.

@ywangd
Copy link
Member Author

ywangd commented Oct 28, 2022

@elasticmachine update branch

@ywangd ywangd merged commit 42430e8 into elastic:main Nov 1, 2022
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.5 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 91171

ywangd added a commit to ywangd/elasticsearch that referenced this pull request Nov 2, 2022
elastic#91171)

When run-as fails because the target user does not exist, the
authentication is created with a null lookup realm. It is then rejected
at authorization time. But for authentication, it is treated as success.
This can lead to NPE when auditing the authenticationSuccess event.

This PR fixes the NPE by checking whether lookup realm is null before
using it.

Relates: elastic#91126 (comment)
elasticsearchmachine pushed a commit that referenced this pull request Nov 2, 2022
#91171) (#91240)

When run-as fails because the target user does not exist, the
authentication is created with a null lookup realm. It is then rejected
at authorization time. But for authentication, it is treated as success.
This can lead to NPE when auditing the authenticationSuccess event.

This PR fixes the NPE by checking whether lookup realm is null before
using it.

Relates: #91126 (comment)
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Nov 3, 2022
* main: (1300 commits)
  update c2id/c2id-server-demo docker image to support ARM (elastic#91144)
  Allow legacy index settings on legacy indices (elastic#90264)
  Skip prevoting if single-node discovery (elastic#91255)
  Chunked encoding for snapshot status API (elastic#90801)
  Allow different decay values depending on the score function (elastic#91195)
  Fix handling indexed envelopes crossing the dateline in mvt API (elastic#91105)
  Ensure cleanups succeed in JoinValidationService (elastic#90601)
  Add overflow behaviour test for RecyclerBytesStreamOutput (elastic#90638)
  More actionable error for ancient indices (elastic#91243)
  Fix APM configuration file delete (elastic#91058)
  Clean up handshake test class (elastic#90966)
  Improve H3#hexRing logic and add H3#areNeighborCells method (elastic#91140)
  Restrict direct use of `ApplicationPrivilege` constructor (elastic#91176)
  [ML] Allow NLP truncate option to be updated when span is set (elastic#91224)
  Support multi-intersection for FieldPermissions (elastic#91169)
  Support intersecting multi-sets of queries with DocumentPermissions (elastic#91151)
  Ensure TermsEnum action works correctly with API keys (elastic#91170)
  Fix NPE in auditing authenticationSuccess for non-existing run-as user (elastic#91171)
  Ensure PKI's delegated_by_realm metadata respect run-as (elastic#91173)
  [ML] Update API documentation for anomaly score explanation (elastic#91177)
  ...

# Conflicts:
#	x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackClientPlugin.java
#	x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/downsample/RollupShardIndexer.java
#	x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/downsample/TransportRollupIndexerAction.java
#	x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/v2/RollupActionSingleNodeTests.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Security/Audit X-Pack Audit logging Team:Security Meta label for security team v8.5.1 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants