SSH signature allowed signers file format support

[castedo]

Parsing the "allowed signers" file format is needed for SSH signature verification: #1394.

This issue it so track a new function in the dulwich library for parsing "allowed signers" files for SSH signatures. This is a feature implemented by ssh-keygen and I'm writing Python code to emulated its implementation. Even though the "allowed signers" format supports extra functionality that I doubt will be used for git ssh signatures, my thought is the dulwich parsing code should parse the full format without error. After parsing, other layers of Dulwich code can raise errors etc... saying certain features are not supported. But at least it would not be a cryptic file parsing error.

[castedo]

@jelmer i've implemented parsing of SSH "allowed signers" format. The unit tests are this file:

https://gitlab.com/perm.pub/dulwich-sshsig-union/-/blob/main/sshsiglib/tests/test_allowed_signers.py

The implementation is the load_allowed_signers_file function here:

https://gitlab.com/perm.pub/hidos/-/blob/9277d178bfe544f67a378437f65b2d5e4418a229/hidos/sshsiglib/ssh_keygen.py#L117

The ~100 lines above that function are needed by it. These ~120 lines are the smallest chunk of functionality that I can think of that I could turn into a PR if you want.

[castedo]

I have a very minimal level of support for the ssh-keygen "allowed signers" format. I've been calling it the "for-git" sub-format in comments and doc strings.

https://gitlab.com/perm.pub/sshsiglib/-/blob/e1c023d4556094fec9142287343cb65d1fbd710f/sshsig/allowed_signers.py

This minimal sub-format enables all the signature verification functionality that git enables. However, I have discovered there are TONS of fancy power user features in the "allowed signers" format that in theory a git user might have and cgit with classic real ssh-keygen will parse correctly. There really isn't reason a git user NEEDS the fancy format. They could stick to a simple "for-git" sub-format and get the same result.

I am currently not planning to implement any of these fancy power user bonus features of ssh-keygen's allowed signers file format. The sub-format is perfectly fine for git purposes.

It's also easy to imagine that many Dulwich users that want SSH sig verification do not actually want to get allowed public keys from this weird ssh-keygen allowed signers file format. In the end, the important interface is the list of public keys that are allowed to sign and not all the bonus feature of the allowed signers file format (that git doesn't actually need).

@jelmer So my recommendation now is to tentatively NOT plan on having ssh-keygen allower signers file format parsing within Dulwich. There are lots of different ways that users might want to grab the list of public keys that are acceptable. And some users might not even care, they just want to check if the signature is a real correct crypto signature for the git commit and don't care which public key was used.

Simpler interop approaches could be:

1. Dulwich expects a list of public keys as Python data objects to some verify function. How the users constructs that list of Pythong objects of public keys if up to them.

or

2. Dulwich just exposes Commit.gpg and Commit.without_gpg() (see #1447) and then a user can use whatever method they want to verify that Commit.gpgsig is a valid signature for the Commit.without_gpgsig. I'm inclined to think a sensible approch is a user imports a separate sshsig package and calls either check_signature or verify depending on whether they care about which public key has signed. Examples below.

import sshsig
sshsig.check_signature(commit.without_gpgsig(), commit.gpgsig)

import sshsig
sshsig.verify(commit.without_gpg(), commit.gpgsig, data_object_list_of_allowed_public_keys)

[jelmer]

I think we do want to have support for reading these files in Dulwich, whether directly or imported from elsewhere. We need it for feature parity with C git.

Let me reopen this. We should definitely provide the interface you're describing - where a prepared list is passed in. But longer term we'll also want the parsing support, even if it is not a strict requirement for the work you're doing.

[castedo]

Here is code using Dulwich and ssh-keygen to do SSH verification of commits using the full allowed signers files format as implemented by ssh-keygen:

https://github.com/jelmer/dulwich/discussions/1448#discussioncomment-11336661

This is essentially what C git does. So in a certain sense, the best way to hit C git parity is to do what is in that example.

My prediction is different dev-users will have different priorities and trade-offs on how they do verification, and there won't be one single way that makes sense. Some of the dimensions to these priorities and trade-offs I see are:

• How important is not depending on the ssh-keygen utility being installed?
• Does the verification really need a list of trusted/allowed public keys at all? or is the verification just for the the public key in the signature so that a search can be performed somewhere else (e.g. probably what GitHub does)
• Is a list of trusted/allowed public keys going to be sourced from an ssh-keygen style allowed signers file?
• Is the full format, with all its bells and whistles, of the ssh-keygen allowed signers file needed? (or is a sub-format sufficient?)

[jelmer]

I think you're right that different users will have different requirements, and we don't have to support every possible option immediately.

In line with that, how important it is to not depend on ssh-keygen varies too. Some people are on e.g. android where ssh-keygen is not available. Others are in cloud environments where they only have access to Python and no C extensions, and are e.g. using paramiko for SSH.

Specifying an allowed key list is a common way to do verification in Git, so I think we should strive to support that. Again, different use cases, so it's not a strict requirement - but eventually we should.

I'd ideally like to support the full format, but initially we can stick to a smaller subset of commonly used options - though ideally we'd error out on options we don't support rather than silently ignoring them.

Does that seem reasonable?

[castedo]

Sounds reasonable. To avoid confusion I wager it will be worthwhile to have three names and definitions. One of them I've been referring to as the "full ssh-keygen allowed signers file format" which seems like a good name.

Then there is the most minimal one discussed here: https://github.com/jelmer/dulwich/discussions/1449

And then in the middle is this future Dulwich level of file format support. Perhaps that can be referred to as the pure-python Dulwich allowed signers file format.

[castedo]

Regarding erroring out ... I've got load_allowed_signers_file function coded to parse the full ssh-keygen allowed signers format:

https://gitlab.com/perm.pub/sshsiglib/-/blob/efdfeebb91b1d359358c9250ba3c6819216f41b5/sshsig/allowed_signers.py#L99

But then the for_git_allowed_keys function will convert that to "just a list of public keys for git" and will raise ValueError for every power user feature of the full format.

[castedo]

Before I start forgetting many of the details of the full ssh-keygen allowed signers file format, here is a rough list of features which are supported by ssh-keygen but are not part for the minimal "hidos DSGL just-a-list-for-git" sub-format #1449.

First field (principals):

• comma separated principals
• quoted principals (with possible spaces)
• negation principal pattern ('!')
• single character matching ('?')
• general wildcard matching ('*')

Optional 2nd field (options):

• comma separated namespaces
• negation in namespace list ('!')
• namespace single character matching ('?')
• namespace wildcard matching ('*')
• certificate keys
• validation date ranges

Misc:

• negation logic is probably dependent on what public keys are supported in ssh-keygen because, I think, but I'm not sure, lines with unsupported public key might get ignored, but once the public key is supported the negation logic will reject verification

The following are some examples of files that are valid and handled by ssh-keygen.

The following line results in a rejected verification by git+ssh-keygen, because the negation in the namespaces option will reject use for git.

foo namespaces="g?t,!g*t" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIQdQut465od3lkVyVW6038PcD/wSGX/2ij3RcQZTAqt

This next line example will result in a rejected verification by git+ssh-keygen because of the negation in the principals (which can be quoted with spaces):

!"do not use this" namespaces="git" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIQdQut465od3lkVyVW6038PcD/wSGX/2ij3RcQZTAqt

BUT WAIT! It only gets more fun! The following will result in a SUCCESSFUL verification because there is a comma separated list of principals and the negation applies to the first principal INSIDE the quotes:

!"do not use this,ok never mind" namespaces="git" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIIQdQut465od3lkVyVW6038PcD/wSGX/2ij3RcQZTAqt

[castedo]

@jelmer

I'd ideally like to support the full format

I doubt you want to support the full format. See above. If any user actually wants to use any of these power features then they should install ssh-keygen on their computer and Dulwich should support interop between Dulwich and calling out to the real ssh-keygen (which is what cgit does).