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

Docs: Update Blocked Audit Devices #6484

Closed
wants to merge 8 commits into from
Closed

Conversation

brianshumate
Copy link
Member

  • Clarify multiple audit device behavior
  • Remove notes on incorrect behavior

- Clarify multiple audit device behavior
- Remove notes on incorrect behavior
@brianshumate
Copy link
Member Author

This should also help to address #5361.

@jefferai
Copy link
Member

jefferai commented Apr 5, 2019

This text is a bit too reductive, I think. It's not that Vault requires all devices to be writable, it's that if one device hangs for an indefinite period of time, the request will too. If it's not writable and simply returns an error then we move on to the next one.

@brianshumate
Copy link
Member Author

Thanks for the clarification! I will update the text accordingly.

an avenue for attack.

Be absolutely certain that your audit devices cannot block and monitor for
blocked requests with [related telemetry](/docs/internals/telemetry.html#vault-audit-log_request_failure).
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think audit failures are not necessarily blocked request.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree! Got too carried away with block; revised now to match the actual API / condition description.

@aphorise
Copy link
Contributor

What's the verdict / outcome here - curious if this PR will be merged before the 3 year marker? :-)
Information seems relevant still and not present on the site.

Please verify validity and merge.

Co-authored-by: Loann Le <84412881+taoism4504@users.noreply.github.com>
@vercel
Copy link

vercel bot commented Aug 22, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
vault ❌ Failed (Inspect) Aug 22, 2022 at 6:07PM (UTC)

Co-authored-by: Loann Le <84412881+taoism4504@users.noreply.github.com>
@@ -74,20 +74,20 @@ The existing logs that it did store are untouched.

## Blocked Audit Devices

If there are any audit devices enabled, Vault requires that at least
one be able to persist the log before completing a Vault request.
If the audit device request becomes blocked for an indefinite period of time,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should be a little more explicit about what we mean by "blocking"? I just worry that some might interpret e.g. a full disk as a cause that that they should worry about here.

Also, though a lesser concern, "audit device request" is a bit of a weird phrasing.

How about something more like:

We can distinguish between two kinds of audit device failure. A blocking failure is one where an attempt to write to the audit device never completes. This is unlikely with a local disk device, but it could happen with a network-based audit device.

With multiple audit devices, if any of them fail in a non-blocking fashion, Vault requests can still complete successfully provided at least one audit device successfully writes the audit record. But if any of them fail in a blocking fashion, Vault requests will hang until the blocking is resolved. In other words, Vault will not complete any requests until the blocked audit device can write.


Vault will not respond to requests if audit devices are blocked because
audit logs are critically important and ignoring blocked requests opens
an avenue for attack. Be absolutely certain that your audit devices cannot
block.
an avenue for attack.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know you didn't write this paragraph but it feels disingenuous to me. I believe the reason for the current behaviour is because we don't know how to fix it, or we haven't taken the time, not because this is a security measure. If that were the case, we would apply the same policy to non-blocking audit device failures.

Or maybe this is an unfortunate use of the word "blocking" and what the paragraph really means to say is: Vault will not respond to requests when no audit devices can record them, because
audit logs are critically important and ignoring auditing failures opens an avenue for attack.

block.
an avenue for attack.

Be absolutely certain that your audit devices cannot block and monitor for
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a little misleading, being within this section on blocking failures, because if we get into the situation where an audit device isn't returning, we won't be populating this metric.

Copy link
Contributor

Choose a reason for hiding this comment

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

hey @ncabatoff any chance you can kindly provide a commit with some of these suggestions?

@brianshumate
Copy link
Member Author

Thanks for all of the input on this ancient issue!

I am in favor of closing this for an improved version to be submitted as a new and clean PR.

brianshumate added a commit that referenced this pull request Aug 25, 2022
- Update blocked audit device to use feedback from #6484
- This PR supersedes #6484
@brianshumate
Copy link
Member Author

I have opened a clean PR to incorporate the great feedback here into an updated section.

Please review in #16881 and I will close this PR now.

ncabatoff pushed a commit that referenced this pull request Aug 26, 2022
- Update blocked audit device to use feedback from #6484
- This PR supersedes #6484
@brianshumate brianshumate deleted the docs-audit-devices branch February 23, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants