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

Consider adding justification modes #257

Open
raserva opened this issue Jul 20, 2022 · 5 comments
Open

Consider adding justification modes #257

raserva opened this issue Jul 20, 2022 · 5 comments
Labels
enhancement New feature or request

Comments

@raserva
Copy link
Contributor

raserva commented Jul 20, 2022

I think this is fine for now, but we should consider an enhancement in the future of making this an enum like JustificationMode. I can see customer's wanting any of the following behavior:

  • Do nothing with justifications
  • If a justification is provided, check the signature and add the result as another field (e.g. justification_verified: true or false)
  • If a justification is provided, check the signature and reject the request if it's invalid. If a justification is not provided, accept the request.
  • Always require justification and reject requests without justification or requests where justification validation fails

Originally posted by @sethvargo in #254 (comment)

@yolocs
Copy link
Contributor

yolocs commented Jul 21, 2022

I'm having a slightly different list:

  • Justification not required (the flag we already have) in which case the code shouldn't even care about justification
  • Require justification
    • check the signature; if valid, add the justification to the audit log
    • else if breakglass, add the breakglass justification to the audit log and continue
    • else if not valid, reject the request

I think the only difference is "If a justification is not provided, accept the request". I wonder what use case is that?

@sethvargo
Copy link
Contributor

I think there's a "dry run" scenario where we want to know whether justification would have succeeded, but we don't want to reject requests. This would be very helpful when rolling out the new system.

@yolocs
Copy link
Contributor

yolocs commented Feb 9, 2023

We can already turn on / off justification with a sub-block in the configuration.
In the justification config, we can already configure whether allow breakglass.

I'm debating whether we need a "dry run" flag to indicate whether to enforce justification. It seems beneficial per Seth's comment.

When in dryrun, we allow no justification or invalid jvs token. We just log a warning. Otherwise, we demand justification must be present (breakglass or not is a separate knob).

@abcxyz abcxyz deleted a comment from anmluo Feb 11, 2023
@yolocs
Copy link
Contributor

yolocs commented Feb 12, 2023

After some thought, I think the "dry run" mode can be covered in the "best effort" log mode. When enabled, any error during audit logging will be (std) logged but we won't return the error. The log mode has a global default and can be overridden on each log.

So now the behavior is: When justification is enabled and the log mode is "fail close". If justification token is missing or invalid, we will return error.

If there is no objection, then I will close this issue.

@anmluo
Copy link

anmluo commented Feb 12, 2023

No objection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

4 participants