Skip to content

Handle streaming request body in audit log #127798

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

Conversation

DaveCTurner
Copy link
Contributor

@DaveCTurner DaveCTurner commented May 7, 2025

The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a ClassCastException if the emit_request_body flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.

The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException`. This change fixes the handling of streaming
requests in the audit log to now report that the request body was not
available when writing the audit entry.
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

@@ -27,6 +27,9 @@ public class AuditUtil {

public static String restRequestContent(RestRequest request) {
if (request.hasContent()) {
if (request.isStreamedContent()) {
return "Request body had not been received at the time of the audit event";
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to eventually support this use case? If not, can we provide more details here? Something like

Audit logging with request body is not supported when the request is streamed. To disable request streaming, set [rest.incremental_bulk] to [false].

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I think eventually we'll need this for streaming requests (because eventually we will be handling all requests as streaming). It's not reasonable to log an arbitrarily-large body in a single audit event tho, instead we will need to record each chunk in the audit log as they arrive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also note that today because of how we try and log the whole body in a single message we end up truncating it anyway after a few kiB even with rest.incremental_bulk: true.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for explaining. The fix here is certainly a lot better than throwing ClassCastException. But we may still want to create an issue to say a future fix is pending?

we end up truncating it anyway after a few kiB

IIRC, we don't truncate audit logs. At least payloads of a few hundred KB are fully logged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started a design doc and opened ES-11760

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC, we don't truncate audit logs

You're right, TIL. And yet we seem to truncate other logs messages emitted by Log4J. I wonder why (but not hard enough to go digging further).

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

@DaveCTurner DaveCTurner added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport Automatically create backport pull requests when merged labels May 7, 2025
@elasticsearchmachine elasticsearchmachine merged commit 4ba94c7 into elastic:main May 7, 2025
22 checks passed
@DaveCTurner DaveCTurner deleted the 2025/05/07/audit-log-streaming-request-body branch May 7, 2025 15:17
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 7, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 7, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 7, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.19
9.0
8.17 Commit could not be cherrypicked due to conflicts
8.18

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

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 7, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.

Backport of elastic#127798 to `8.17`
@DaveCTurner
Copy link
Contributor Author

8.17 backport is #127843

elasticsearchmachine pushed a commit that referenced this pull request May 7, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.
elasticsearchmachine pushed a commit that referenced this pull request May 7, 2025
* Handle streaming request body in audit log

The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.

Backport of #127798 to `8.17`

* Enable incremental bulks in AuditIT
elasticsearchmachine pushed a commit that referenced this pull request May 8, 2025
* Handle streaming request body in audit log (#127798)

The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.

* Enable incremental bulks in AuditIT
elasticsearchmachine pushed a commit that referenced this pull request May 8, 2025
* Handle streaming request body in audit log (#127798)

The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.

* Enable incremental bulks in AuditIT
ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request May 9, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request May 9, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request May 12, 2025
The audit event for a successfully-authenticated REST request occurs
when we start to process the request. For APIs that accept a streaming
request body this means we have received the request headers, but not
its body, at the time of the audit event. Today such requests will fail
with a `ClassCastException` if the `emit_request_body` flag is set. This
change fixes the handling of streaming requests in the audit log to now
report that the request body was not available when writing the audit
entry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :Security/Audit X-Pack Audit logging Team:Security Meta label for security team v8.17.7 v8.18.2 v8.19.0 v9.0.1 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants