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.

Sign up for GitHub

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

Jump to bottom

Use extended audit capabilities in plugins #477

Open
cipherboy opened this issue Aug 20, 2024 · 3 comments
Open

Use extended audit capabilities in plugins #477

cipherboy opened this issue Aug 20, 2024 · 3 comments
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed roadmap:safer Roadmap item; safer category

Comments

@cipherboy
Copy link
Member

cipherboy commented Aug 20, 2024 •
edited
Loading

Is your feature request related to a problem? Please describe.

I happened to stumble upon @DrDaveD's issue on his Vault config about auditing traffic from the OIDC plugin to its token issuer on the OpenBao side. In particular, while OpenBao has logging of request/response pairs, it lacks the ability for plugins to audit log their request/responses to external parties.

Describe the solution you'd like

In particular, the extended system view, only accessible to builtin plugins, already has such an Auditor method:

type ExtendedSystemView interface {
    Auditor() Auditor
    ForwardGenericRequest(context.Context, *Request) (*Response, error)

    // APILockShouldBlockRequest returns whether a namespace for the requested
    // mount is locked and should be blocked
    APILockShouldBlockRequest() (bool, error)
}

So far, it does not look like it is being used in any of our plugins. One of the issues is that, because it is in ExtendedSystemView, it cannot be used from external plugins, only builtin plugins. We could perhaps rewrite this into the regular SystemView implementation by consuming the Auditor interface:

    AuditRequest(ctx context.Context, input *LogInput) error
    AuditResponse(ctx context.Context, input *LogInput) error

Then this could be proxied over the GRPC bridge.

Describe alternatives you've considered

n/a

Explain any additional use-cases

By adding this to the GRPC mechanism, we can ensure external plugins can also audit log requests, which will help us expose audit events where relevant.

Additional context

n/a
@cipherboy cipherboy added the feature New feature or request label Aug 20, 2024
@cipherboy
Copy link
Member Author

This is not something I'll work on, so if someone else wants to take it, feel free!

@cipherboy cipherboy changed the title Use extended audit capabilities to plugins Use extended audit capabilities in plugins Aug 20, 2024
@cipherboy cipherboy added good first issue Good for newcomers help wanted Extra attention is needed labels Aug 20, 2024
@DrDaveD
Copy link
Contributor

DrDaveD commented Aug 20, 2024

Thank you for making the issue.

For external plugins that want to work with a framework that both has this feature and one that doesn't (i.e. either Vault or an older version of OpenBao), can you think of a way to determine at run time whether or not this new feature would be available? If not I guess the next best thing would be to determine it at compile time somehow.

@cipherboy
Copy link
Member Author

cipherboy commented Aug 20, 2024 •
edited
Loading

@DrDaveD I think we can do what we did with paginated list: suggest plugins that want to take advantage of this build against the OpenBao SDK (which should presently be compatible with Vault as a plugin server as well, though this is untested), but have any attempt to use logger return an error (because the GRPC client / compile time code knows about it, but the remote server won't, and thus would fail in a way detectable by the GRPC client that can return a common error message).

Then the GRPC client can ignore the error versus propagating it up and the plugin can "think" it succeeded. Or, it can be returned to the plugin to decide if it wants to handle it explicitly or not (envvar or compile-time constant).

My 2c.

@cipherboy cipherboy mentioned this issue Sep 30, 2024
Open
@cipherboy cipherboy added the roadmap:safer Roadmap item; safer category label Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature New feature or request good first issue Good for newcomers help wanted Extra attention is needed roadmap:safer Roadmap item; safer category
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cipherboy @DrDaveD