> to verify a ssh signature we first call ssh-keygen -Y find-principal to > look up the signing principal by their public key from the > allowedSignersFile. If the key is found then we do a verify. Otherwise > we only validate the signature but can not verify the signers identity. Is this the same behavior as GPG signing in Git? > Verification uses the gpg.ssh.allowedSignersFile (see ssh-keygen(1) "ALLOWED > SIGNERS") which contains valid public keys and a principal (usually > user@domain). Depending on the environment this file can be managed by > the individual developer or for example generated by the central > repository server from known ssh keys with push access. If the > repository only allows signed commits / pushes then the file can even be > stored inside it. Storing the allowedSignersFile in the repo is technically possible even if the repository does not allow signed commits/pushes, right? I would reword the last sentence as "This file is usually stored outside the repository, but if the repository only allows signed commits/pushes, the user might choose to store it in the repository". > Using SSH CA Keys with these files is also possible. Add > "cert-authority" as key option between the principal and the key to mark > it as a CA and all keys signed by it as valid for this CA. Is this functionality provided by SSH? I don't see "cert-authority" anywhere in the diff below. Also, I notice that the tests are all provided at the end. I think that it would be better for the tests to be incrementally provided along with the commit that introduces the relevant functionality, so it is clearer to the reviewers how it is supposed to work (and also for us to observe test coverage). > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c > index a34742513ac..62b11c5f3a4 100644 > --- a/builtin/receive-pack.c > +++ b/builtin/receive-pack.c > @@ -131,6 +131,8 @@ static int receive_pack_config(const char *var, const char *value, void *cb) > { > int status = parse_hide_refs_config(var, value, "receive"); > > + git_gpg_config(var, value, NULL); > + > if (status) > return status; Check the return value of git_gpg_config() to see if that config was processed by that function - if yes, we can return early. > +static void parse_ssh_output(struct signature_check *sigc) > +{ > + const char *line, *principal, *search; > + > + /* > + * ssh-keysign output should be: > + * Good "git" signature for PRINCIPAL with RSA key SHA256:FINGERPRINT > + * Good "git" signature for PRINCIPAL WITH WHITESPACE with RSA key SHA256:FINGERPRINT > + * or for valid but unknown keys: > + * Good "git" signature with RSA key SHA256:FINGERPRINT > + */ Is this "ssh-keysign" or "ssh-keygen" output? Also, is this output documented to be stable even across locales? > + sigc->result = 'B'; > + sigc->trust_level = TRUST_NEVER; A discussion of trust levels should also be in the commit message or user documentation. > + if (!strcmp(var, "gpg.ssh.revocationFile")) { The "F" in "revocationFile" has to be lowercase. (If tests were included, as I suggested above, it might have been easier to catch this.)