Skip to content

Instantly share code, notes, and snippets.

@s-hertel
Last active March 7, 2022 18:47
Show Gist options
  • Save s-hertel/aa08ecf4d44bb9578d815689f9d46118 to your computer and use it in GitHub Desktop.
Save s-hertel/aa08ecf4d44bb9578d815689f9d46118 to your computer and use it in GitHub Desktop.
State table for ansible-galaxy signature verification
  Unconfigured keyring (default) Configured keyring
    R == -1 ("all") R >= 1 (default)
F == 0 F > 0 S >= R S < R
U >= 1 Invalid option error 1 Success 2 Fail Success Fail
G >= 1 Warn Success 2 Fail Success Fail
U == 0 and G == 0 Success Success 3 Fail 4
  1. Gives error that the keyring needs to be configured if user wants to use signature verification. I wondered if this should be a warning instead, but made it an error because it seems like this would probably be done by mistake. Signature verification can be toggled off to opt out without ambiguity.
  2. If I > 0 and S == 0, there were only ignored signatures. I'm considering this a success because no non-ignored signatures could be considered "all". Should this be a warning?
  3. Sucessful because no signatures could be considered "all".
  4. Currently a failure because None < R, but should this just be a warning?
U
Number of user-provided signatures.
G
Number of galaxy-provided signatures.
R
Number of required successful signatures.
S
Number of signatures that are successful. This does not include signatures which fail but are ignored.
F
Number of signatures that fail. This does not include signatures which fail but are ignored.
I
Number of signatures that fail but are ignored. This is used to limit the errors a user will see and prevent certain errors from incrementing F.

Update 03-07-2022:

I added support to make signature verification strict (e.g. --required-valid-signature-count +all). The behavior on the left side of the original table (unconfigured keyring which is the default, i.e. keyring is None) is the same, so I didn't put it in this table. I put asterisks next to the things that are a change in behavior.

Assuming the keyring is not None:

  R == all R >= 1
S == 0 and not strict Success (F == 0) U/G == 0 U/G >= 1
Success ** Fail (S < R)
S == 0 and strict Fail * Fail
F == 0 Success Only S is relevant to R >= 1
F > 0 Fail
S >= R Only F is relevant to R == all Success
S < R Fail

* Prior to 'strict', this was a verification success. Changed to give 'all' an option to be as strict as R >= 1 and require signature verification occurs.

** Prior to 'strict', this was a verification fail because R < S. Changed to make R >= 1 more lenient, like 'all', if no signatures are found.

@nitzmahone
Copy link

This is great- exactly what we need for ensuring that we've got all the cases covered. Thanks for putting it together!

I suspect there are a few problem cases above- a couple that will break things when we someday start configuring a default keyring, and others where security folks will likely be unhappy with it and give us grief once they understand what's happening.

The cases I'm most concerned about future grief on are mostly around the conflation of "unconfigured keyring" and "effectively unconfigured keyring" (ie, one that has no applicable keys, so all signatures are ignored):

  1. not having a way to force unconfigured keyring + >0 signatures (U/G) as an error - this basically signals to an attacker that you can bypass the whole thing by just blasting the keyring entirely. The warning definitely helps, but dunno if it'll be considered "good enough".
  2. not having a way to force "all signatures" validation as an error (ie, "all content must be signed, and all non-ignored signatures must validate; your "note 2" is spot-on, and may be a case where we need a discrete knob to make that and other cases ignore/warn/fail). In general I'd consider "all" a much stricter setting, so having it always succeed when there are no signatures seems backward.

My initial thoughts to address all these was to make the R config value a composite string rather than just using -1 as a special case- the same thing could probably be accomplished with an explicit "require all content to be signed" switch, but I'll leave that as an exercise for someone else. Maybe make + the special modifier that turns on the signatures-required behavior as specified in the chart above for configured keyring, and leave the current default as the non-+ version:

R: 1 # always succeeds if U/G==0 or all sigs are ignored
R: +1 # strict validation of at least one signature- existing behavior as specified, except also fails on unconfigured keyring
R: all # always succeeds if U/G==0 or all sigs are ignored
R: +all # strict validation of all non-ignored signatures, and if U/G>1, S must be at least 1

... or something like that. The concerns that we probably at least need a plan for are:

  • signatures always required (that can't be bypassed by just unconfiguring/removing/emptying the keyring)
  • ensuring that all is always stricter than R>=1

@s-hertel
Copy link
Author

The cases I'm most concerned about future grief on are mostly around the conflation of "unconfigured keyring" and "effectively unconfigured keyring" (ie, one that has no applicable keys, so all signatures are ignored):

@nitzmahone I'm still digesting your reply, but my chart is ambiguous about what a "configured" keyring is. By "configured", I mean the ansible-galaxy caller provides a non-null value to --keyring (or the global config). There is no keyring inspection and setting the keyring is just how to opt-into signature verification (rather than the original plan of only having a way to opt-out and having the keyring set to a sensible default). Regardless of whether or not the keyring or its directory exist or if it contains any keys, we just pass it along to gpg.

If the keyring isn't functional, gpg seems to have sensible errors. For example, if a keyring is provided and the keyring directory does not exist it will cause the gpg error codes ERROR, FAILURE, and UNEXPECTED, so it always fail signature verification (because no signatures will be successful and R == 1 by default). If they also set the required number of signatures to -1/all, then they will also have to ignore all three of those GPG errors to 'pass' signature verification.

ensuring that all is always stricter than R>=1

This is really counter intuitive to me. The behavior in devel is like "all" (albeit with no option to ignore errors yet) - use all signatures we find, but don't expect them. If this changes to be more strict than R >= 1, there will be no way to reproduce that behavior - the user will need to find out if there are supposed to be signatures beforehand on the Galaxy server and install that collection separately with signature verification turned off if there aren't any.

I really like your idea to solve the "all content must be signed, and all non-ignored signatures must validate" with a modifier. I'll work on that.

@s-hertel
Copy link
Author

s-hertel commented Mar 7, 2022

I edited the gist to include an updated state table for configured keyring (i.e. is not None). Wanted to highlight how this impacts your primary concerns:

  • signatures always required (that can't be bypassed by just unconfiguring/removing/emptying the keyring)

For R == 1 (the default when the keyring is configured) this was the behavior prior to adding strict into the mix. Now this is not the default behavior for configured keyring (the caller will need to set the required count to +1 instead of the default (1)). I wonder if the default should be +1 now instead of 1, since having a strict default and opting-into more flexible settings seems safer.

R == +all now allows failing if no signature verification occurs (regardless of if all errors are ignored or if there were no signatures).

Removed/empty keyrings are still an error.

Unconfigured keyrings (i.e. None, not provided to ansible-galaxy) still behave as outlined in the first table.

  • ensuring that all is always stricter than R>=1

I tried to find middle-ground by making +all as strict as +1 to ensure valid signature verification occurs and making 1 as lenient as all if there are no signatures. I think the latter is useful if someone expects at most 1 signature from the server to be valid but doesn't know if the collection is signed. If R == 1 and signatures are found but not successful (and are all ignored), this is still an error, which feels right but seems like a cornercase that may trip people up. Maybe for the sake of simplicity it needs to be a success regardless of whether (U == 0 and G == 0) or (G >= 1 or U >= 1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment