Conversation
…tion Currently signatureType is private, preventing external code from determining what type of signature is present on a commit or tag. This is needed for implementing a pluggable Verifier interface that can route to the appropriate verification implementation. Export SignatureType with its constants and add a String() method for debugging and logging purposes. Add DetectSignatureType() as the public API for signature format detection.
Signature verification needs to express not just validity but also trust. Git's model includes undefined, never, marginal, full, and ultimate trust levels. This allows callers to enforce minimum trust requirements (e.g., "only accept commits signed by fully trusted keys"). Add TrustLevel enum with String() and AtLeast() comparison method. This follows the same pattern as existing go-git enums like SignatureType.
The current Verify() methods return *openpgp.Entity, which: - Couples the API to OpenPGP implementation details - Doesn't provide trust level information - Doesn't work for SSH/X509 signatures Add VerificationResult struct that provides: - Type: signature format (OpenPGP, SSH, X509) - Valid: cryptographic validity - TrustLevel: key trust level - KeyID: signing key identifier - PrimaryKeyFingerprint: full fingerprint - Signer: identity string (name <email>) - SignedAt: signature timestamp - Error: failure details This enables unified verification results across all signature types.
Mirror the existing Signer interface with a Verifier interface that supports multiple signature formats. VerifierChain routes verification to the appropriate verifier based on detected signature type. This enables: - OpenPGP verification (existing functionality, new interface) - SSH verification (future commit) - Custom verifiers (user-provided) The interface returns *object.VerificationResult to provide rich verification information regardless of signature format.
Wrap existing OpenPGP verification logic in the new Verifier interface. This enables Commit.VerifySignature(verifier) to work with OpenPGP signatures while maintaining full backward compatibility. OpenPGPVerifier can be created from: - Armored keyring string (NewOpenPGPVerifier) - Existing openpgp.EntityList (NewOpenPGPVerifierFromKeyring) Keys in the keyring are considered TrustFull. Unknown keys result in TrustUndefined with Valid=false.
Git supports SSH signatures since OpenSSH 8.2. The signature format is documented in PROTOCOL.sshsig. This adds parsing support for: - Armored SSH signature blocks (-----BEGIN SSH SIGNATURE-----) - Binary signature blob (SSHSIG magic, version, public key, etc.) - Key fingerprint extraction (SHA256:...) The parser extracts the embedded public key, namespace (should be "git"), hash algorithm, and signature blob for verification. Includes fuzz test for robustness against malformed input.
Implement the Verifier interface for SSH signatures. SSHVerifier uses an allowed signers map (principal -> public key) similar to Git's allowed_signers file. Verification process: 1. Parse armored SSH signature 2. Verify namespace is "git" 3. Compute signed data (SSHSIG + namespace + hash of message) 4. Verify signature using embedded public key 5. Check if key is in allowed signers (determines trust level) Keys in AllowedSigners get TrustFull; valid signatures from unknown keys get TrustUndefined.
Enable signing commits and tags with SSH keys. This complements SSHVerifier and allows go-git to create SSH-signed commits that are compatible with git CLI verification. SSHSigner wraps ssh.Signer and produces armored signatures in the PROTOCOL.sshsig format with namespace "git" and SHA-512 hashing. The signature format is compatible with: - git verify-commit (with gpg.format=ssh) - ssh-keygen -Y verify Includes round-trip test that signs and verifies with SSHVerifier.
…ethod This change introduces a unified signature verification API that works with any signature type (OpenPGP, SSH, etc.) through a pluggable Verifier interface. The new Commit.VerifySignature(verifier) method complements the existing Commit.Verify() method by: - Supporting multiple signature types via the Verifier interface - Returning rich VerificationResult with trust levels, key IDs, and signer info - Providing ErrNoSignature error for unsigned commits The Verifier interface is defined in plumbing/object to avoid circular imports with the git package where concrete implementations (OpenPGPVerifier, SSHVerifier) are defined. This enables tools to perform proper signature verification without shelling out to git verify-commit.
Add NewSSHSignerFromFile and NewSSHSignerFromFileWithPassphrase functions for convenient loading of SSH private keys from the filesystem. These functions: - Support ~/prefix expansion to user home directory - Handle both encrypted and unencrypted private keys - Provide clear error messages for common failure cases This simplifies the common use case of signing commits with an SSH key stored on disk, similar to how git uses ssh-keygen for signing.
Use io.ReadFull instead of r.Read in readSSHString to ensure all
requested bytes are read. The Read method may return fewer bytes than
the buffer size without returning an error, which could cause incorrect
parsing of SSH signatures when fields are only partially read.
Fixes issue introduced in 9812fe2e ("git: add SSH signature format parsing").
Use entity.PrimaryIdentity() instead of iterating over the Identities
map and breaking after the first element. Go map iteration order is
undefined, so the previous implementation could return different
signer identities across runs for entities with multiple identities.
PrimaryIdentity() provides deterministic selection by preferring
non-revoked identities, those marked as primary, or the latest-created
identity, in that order.
Fixes issue found in commit ca592af7 ("git: add OpenPGPVerifier
implementing Verifier interface").
Add nil check for the signer parameter in NewSSHSigner and change
the return signature to (*SSHSigner, error). A nil signer would cause
a panic at runtime when Sign() is called on the returned SSHSigner.
This fixes an issue found in commit ceedf94e ("git: add SSHSigner
implementing Signer interface for SSH keys").
If the verifier parameter is nil and PGPSignature is non-empty, calling verifier.Verify() causes a nil pointer dereference panic at runtime. This adds an early check that returns ErrNilVerifier instead. Fixes issue found in commit ade92b05.
The AllowedSigners field was exported, allowing callers to modify
the map after construction. This could enable adding malicious keys
or removing legitimate ones during a verification operation.
Changes:
- Rename AllowedSigners to allowedSigners (unexported)
- Copy the input map in NewSSHVerifier to prevent external modification
- Update tests to verify behavior through public API
Fixes issue in commit 33b6a0d8 ("git: add SSHVerifier for SSH signature
verification").
Fix errcheck, gofumpt formatting, and modernize linter warnings: - Handle error returns from file.Close, r.Read, and binary.Write - Apply gofumpt formatting - Use maps.Copy and range over int idioms
Add support for parsing SSH allowed signers files following Git's gpg.ssh.allowedSignersFile format. The parser handles: - Single and multiple comma-separated principals - Wildcard principal (*) - Comments and empty lines - Optional fields (namespaces=, valid-after=, valid-before=, cert-authority) - Home directory expansion (~/) Security considerations: - Duplicate principals are rejected with an error - Line length limited to 64KB to prevent DoS - Strict option prefix validation
Add support for GPG configuration in git config, following existing
patterns for User, Author, and Committer sections.
New configuration options:
- gpg.format: signature format ("openpgp" or "ssh")
- gpg.ssh.allowedSignersFile: path to SSH allowed signers file
This enables reading Git's gpg.ssh.allowedSignersFile setting for
SSH signature verification workflows.
Add a convenience function to create an SSHVerifier directly from an allowed_signers file path. This simplifies the common workflow of loading trusted keys from a file for signature verification. Features: - Supports ~/ prefix for home directory expansion - Proper error propagation for file and parse errors - Comprehensive test coverage
Add a convenience function to create an SSHVerifier directly from git config. This reads gpg.ssh.allowedSignersFile from the config and creates a verifier with those trusted keys. Behavior: - Returns (nil, nil) if config is nil or has no allowedSignersFile - Returns (verifier, nil) on success - Returns (nil, error) if file cannot be read/parsed
Add comprehensive integration tests covering the full SSH signature verification workflow: - Trusted signature: Key in allowed_signers → Valid + TrustFull - Untrusted signature: Key not in allowed_signers → Valid + TrustUndefined - Unsigned commit: Returns ErrNoSignature - Invalid signature: Tampered signature → Valid=false - Config-based flow: Load verifier from config and verify - Multiple allowed signers: Correct principal matching - Nil verifier: Returns ErrNilVerifier - Wrong signature type: SSH verifier rejects PGP signatures
Add an example demonstrating SSH signature verification workflow: - Open a Git repository - Load SSHVerifier from allowed_signers file - Get HEAD commit and verify its signature - Display verification result (Valid, Type, TrustLevel, KeyID, Signer) - Show trust status interpretation The example includes test support in common_test.go that clones the current repository (which has SSH-signed commits) and creates a temporary allowed_signers file for testing.
Refactor test files to use github.com/stretchr/testify for consistency with the rest of the codebase (e.g., config/config_test.go). Changes: - Use require.NoError/require.Len for fatal checks - Use assert.Equal/True/False for non-fatal assertions - Use assert.ErrorIs for error type checking - Use assert.Contains for substring assertions This improves test readability and follows established patterns.
Add a test that verifies the full worktree.Commit flow with SSH signing works correctly end-to-end: 1. Create an in-memory repository 2. Generate an SSH key pair 3. Commit through worktree.Commit with Signer option 4. Verify the signature using SSHVerifier This ensures the Signer integration in CommitOptions is properly wired.
Add Tag.VerifySignature to verify signatures using the Verifier interface, matching the existing Commit.VerifySignature API. This enables SSH and other signature types for tags. Add Signer field to CreateTagOptions to support flexible tag signing, using the same signObject pattern as commit signing.
Demonstrates how to verify SSH signatures on annotated tags using the Tag.VerifySignature method with an SSHVerifier.
616a115 to
ca6d8c0
Compare
pjbgf
left a comment
There was a problem hiding this comment.
Thank you for proposing this PR.
Let's get the commit 36ec59e into its own PR, with the small amendment on the comment as per below. Please also expose user.signingkey and commit.gpgsign, as they align with the data needed to make config-based decisions around signing.
Around the other commits, there are a few topics that we may want to discuss individually instead of on a large PR. One of the potential directions of travel for the project is to start avoiding full-on implementations in-tree, whist providing extensible features that may be implemented in satellite libraries (inside or not of github.com/go-git/), so that users not interested in the specific feature don't need to take on that code - nor its dependencies. Or more importantly, they are able to replace them with other implementations.
|
|
||
| GPG struct { | ||
| // Format specifies the signature format to use when signing commits and tags. | ||
| // Valid values are "openpgp" (default) and "ssh". |
There was a problem hiding this comment.
| // Valid values are "openpgp" (default) and "ssh". | |
| // Valid values are "openpgp" (default), "x509" and "ssh". |
|
I had a look at This PR and #1860. @hiddeco already implemented signing and verification for SSH signatures https://github.com/hiddeco/sshsig Maybe you could add the allowed signer parsing as PR to the project https://github.com/hiddeco/sshsig/pull/7/changes Then we could use this in plugin as dependency. |
|
|
||
| // TrustLevel represents the trust level of a signing key. | ||
| // The levels follow Git's trust model, from lowest to highest. | ||
| type TrustLevel int8 |
There was a problem hiding this comment.
I'm under the impression this aligns with GPG verification, but not SSH. I'd assume the same would apply for potentially other signature methods - e.g. X.509, but that I'm not sure.
Ideally the basic verifier API would be agnostic and still be able to provide additional information if needed. I haven't thought about this part of the API yet, but it could be done via typed errors.
There was a problem hiding this comment.
Yes, this also crossed my mind that you need different abstractions. Two which were already made are "verified" and "trusted".
Verified: public matches signature
Trusted: additional key properties (owner, trust level of PGP üublic key or virified_signers for SSH) match the committe/author.
Additionally S/MIME again can be done similar. Here we could/need to check if the cert issue timeframe matches the commit timestamp and the CN or eMails in the SAN. Or is signed by a certain CA.
Not sure how we should abstract these layers. A simple verified for SSH and GPG is not that difficult but trust (level) has more attributes to consider.
There was a problem hiding this comment.
+1. On the trust levels, I'm not sure we need to go that far. It could be an option (e.g. WithAcceptGPGPartialTrust for the specific verifier), but from an API perspective that would just mean we return an error or not - ideally the default failing on the safe side.
There was a problem hiding this comment.
I have been wondering about this part too. On github you have also 3 states see https://docs.github.com/en/authentication/managing-commit-signature-verification/displaying-verification-statuses-for-all-of-your-commits , but I am not so much of a fan. I kind of like the idea of:
- Verified: there is a valid signed commit
- Trusted: the signature was valid according to some rules/configuration/... at the time of the commit
- Unverified for commit that either don't have a signature or we could not verify it.
Still, not great as it bake a specific semantic of the verifier into the API. The idea of just relying on error is maybe a better approach. No error would be fully trusted anything else can be drilled into using errors.Is and would allow semantic extensibility for the verifier plugin. Will try to put that in my proposal PR later this week.
This is a draft PR to gather feedback from the community on improving support for signature in go-git. I am interested mostly on ed25519 ssh key used to sign and verify git commit. That is what I focused on. The documentation that I found the most useful on the topic was this blog: https://calebhearth.com/sign-git-with-ssh .
This PR is not ready for inclusion has since I started working on it, I have learned more about the project expectation and I need to go back, split it and clean it to match better the rest of upstream code base. The intent at this point is to see if it would be useful to more people than just me. I hope it could address #400 .