Add SSH signature verification for git commits by bb-Ricardo · Pull Request #1141 · fluxcd/pkg

bb-Ricardo · 2026-02-27T12:04:07Z

This PR adds support of SSH signature validation.

resolves: fluxcd/flux2#4145

adds new package git/signatures
adds validation of SSH signed commits to ssh_signature.go
moves GPG signature validation to gpg_signature.go
adds text fixtures for all SSH and GPG key types including commits and signatures
adds tests for all key/signature combinations
adds wrapper for "Verify(keyRings ...string)" function

stefanprodan · 2026-02-28T08:04:40Z

@bb-Ricardo please run make tidy and force push to unblock the test.

bb-Ricardo · 2026-03-10T07:40:56Z

Hi, was just wondering if this PR is sufficient or if anything else is needed?

stefanprodan · 2026-03-10T08:30:24Z

Hey @hiddeco and @pjbgf could you please review?

stefanprodan

Tests are failing with open signatures/... no such file or directory please run make test-git before pushing commits.

bb-Ricardo · 2026-03-10T08:54:15Z

sorry for that, now the tests passed locally.

bb-Ricardo · 2026-03-17T19:52:09Z

@hiddeco - would you mind to have a look at this PR again? Thank you.

pjbgf

@bb-Ricardo thanks for working on this. Overall LGTM, however I'd remove any references to DSA as per thread below.

stefanprodan · 2026-03-19T10:35:46Z

@bb-Ricardo please use rebase not merge. Undo the last merge, and properly rebase your fork, GH has a button for this if you go to your forked repo.

bb-Ricardo · 2026-03-19T11:32:46Z

@bb-Ricardo please use rebase not merge. Undo the last merge, and properly rebase your fork, GH has a button for this if you go to your forked repo.

Yes, sorry. I used the seemingly convenient button GitHub offered in this PR. I removed all occurrences of GPG DSA keys in the tests and test fixtures.
Also rebased onto the current main.

bb-Ricardo · 2026-04-20T08:31:00Z

Hi,

was wondering if anything else is needed. Thank you

- adds new package git/signatures - adds validation of SSH signed commits to ssh_signature.go - moves GPG signature validation to gpg_signature.go - adds text fixtures for all SSH and GPG key types including commits and signatures - adds tests for all key/signature combinations - adds wrapper for "Verify(keyRings ...string)" function Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

…tureType' Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com> Signed-off-by: Ricardo <ricardo@bitchbrothers.com> Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

hiddeco · 2026-05-18T08:15:58Z

+			err := sshsig.Verify(bytes.NewReader(payload), sig, pubKey, sig.HashAlgorithm, SSHSignatureNamespace)
+			if err == nil {


The sshsig library has certain error sentinels that can be quite informative (e.g. sshsig.ErrPublicKeyMismatch / ErrNamespaceMismatch / ErrUnsupportedHashAlgorithm. VerifyPGPSignature). This information is now swallowed instead of being made available to the user if all keys fail.

hiddeco · 2026-05-18T08:31:52Z

@@ -0,0 +1,5 @@
+sign-user@example.com namespaces="git" ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQCpreiO+8XsB4xXGNmwuO48a7WPghb5ihCJNPyQZpnaPfq6vhNVWSgq8AIjBmJOJYo4HZyiHqpS4OBc86glk6qMv8YHRt4VRVBP+DjPLDIsOR7+2HBlOPHMm8lTDi+iMPHBDxqFy7mSDB4+v7n700+49vYhWjZJpesnnE6JoitxSVhmqp75jeNRNU6PD00z+gMUcviv8UOs/Apg1Cw5f+4T9yOnjlOHaFH/ButvZ0t2VF0cs28tfCuLAoumjine5Gm6tCRQlZOoapNJzvnYT+86f/PEU/4kDYf3wT7S+NnUDfCsIpDVlOXPvjnQ/DudhqEnnXvfch+eBCI7rtJBHIGPKFdmC4cUROa0UDGR6o/JxLtx4ZTbkGpq6MVwdrb7qJ+Oib1U8xVimWFfarkm7deVXWD3wB5Wa8Ko/a/WuYfE3gYRhb8iXPYd71FsEy4F41JCMZDcIqMiQRe3e2gvY+z2sf02kHOFeWJmrAY9FFjPL85VD0Dg++jrExkGFjcBTw9gUG5OPGpwqQ9WHO8E8DPza+i5J/wu4DODyLrLxuXHPeSYUjcvh5ln8P70qL+Irwn1mgn2PkIZW0XCPBt6Iylg55t5sfyy03P0Kmb4U3TrppMeig7Lr9LDU4Doh7Fj6oLYDGFUV+F52SSuPs5SfrWd6Apiz+VPjsAh5btPPJNlzQ== test-rsa@example.com


I do not believe any of the verified_signers_* files are actually utilized in tests?

hiddeco · 2026-05-18T08:32:15Z

+### Script structure
+The script uses separate functions for different key types:
+- `generate_rsa_dsa_key()` - For RSA keys with key length validation
+- `generate_ecc_key()` - For ECC/ECDSA/EdDSA keys with curve validation
+- `create_signed_object()` - For creating signed commits and tags
+- `create_unsigned_commit()` - For creating unsigned test commits


This seems to be outdated.

hiddeco · 2026-05-18T08:33:09Z

+./generate_ssh_fixtures.sh
+```
+
+This script will automatically create all SSH keys, authorized_keys files, verified signers files, signed commits, and signed tags.


This seems to not be accurate anymore. (There are probably more things in this doc.)

hiddeco · 2026-05-18T08:38:39Z

+		// which is not supported by github.com/ProtonMail/go-crypto (only version 4 is supported).
+		// The error occurs when trying to read the armored key ring:
+		// "unable to read armored key ring: openpgp: invalid data: first packet was not a public/private key"
+		{"ed448 valid signature", "commit_ed448_signed.txt", "tag_ed448_signed.txt", "key_ed448.pub", true},


Should we also test tag_unsigned.txt?

hiddeco · 2026-05-18T08:39:50Z

+			sig:      "-----BEGIN SSH SIGNATURE-----\n-----END SSH SIGNATURE-----",
+			keyRings: []string{armoredKeyRingFixture},
+			wantErr:  "unable to verify openPGP signature, detected signature format: ssh",
+		},


Testing a malformed signature (e.g. malformed body and/or missing -----END... would probably be good to test as well.

hiddeco · 2026-05-18T08:40:23Z

+			sig:      "-----BEGIN SSH SIGNATURE-----\n-----END SSH SIGNATURE-----",
+			keyRings: []string{armoredKeyRingFixture},
+			wantErr:  "unable to verify openPGP signature, detected signature format: ssh",
+		},


Testing multi-keyring case would have surfaced the issue I commented on gpg_signature.go itself.

hiddeco · 2026-05-18T08:40:52Z

+create_signed_object() {
+    local object_type=$1
+    local key_name=$2
+    local key_type=$3


Not being used.

hiddeco · 2026-05-18T08:41:59Z

+echo "Temporary directory: $TEMP_DIR"
+echo "Output directory: $SCRIPT_DIR"
+echo ""
+


Would probably be good to do a quick command -v safety check to fail early.

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

bb-Ricardo · 2026-05-19T05:55:42Z

Thank you @hiddeco for taking the time to review the code. Will try to fix the rest of the code during the day.

…alfored key is included Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

bb-Ricardo requested review from aryan9600 and hiddeco as code owners February 27, 2026 12:04

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch 2 times, most recently from 96523af to 1561774 Compare February 28, 2026 00:35

bb-Ricardo requested a review from a team as a code owner February 28, 2026 08:48

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from eedb46c to 048e862 Compare February 28, 2026 09:07

bb-Ricardo mentioned this pull request Feb 28, 2026

Improve UX for verifying commits and Tags go-git/go-git#1869

Open

2 tasks

stefanprodan reviewed Feb 28, 2026

View reviewed changes

Comment thread git/signatures/ssh_signature.go Outdated

Comment thread git/signatures/signature.go Outdated

Comment thread git/signature/ssh_signature.go

bb-Ricardo mentioned this pull request Feb 28, 2026

Feature Request: support validation of additional git commit signature types fluxcd/source-controller#1996

Open

stefanprodan reviewed Feb 28, 2026

View reviewed changes

Comment thread git/signatures/ssh_signature_test.go Outdated

Comment thread git/signatures/ssh_signature_test.go Outdated

Comment thread git/signatures/ssh_signature.go Outdated

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from e247a34 to 5e6ab4d Compare March 2, 2026 23:16

stefanprodan added enhancement New feature or request area/git Git and SSH related issues and pull requests labels Mar 10, 2026

stefanprodan reviewed Mar 10, 2026

View reviewed changes

hiddeco reviewed Mar 10, 2026

View reviewed changes

Comment thread git/signatures/ssh_signature.go Outdated

Comment thread git/signatures/ssh_signature.go Outdated

Comment thread git/signatures/signature.go Outdated

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from 83338dc to 51ceefd Compare March 10, 2026 14:30

pjbgf requested changes Mar 19, 2026

View reviewed changes

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from b6f0ece to bd9abd8 Compare March 19, 2026 11:30

bb-Ricardo requested a review from pjbgf March 23, 2026 15:00

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from bd9abd8 to fa037d1 Compare April 9, 2026 04:59

bb-Ricardo requested a review from stefanprodan April 9, 2026 05:00

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from fa037d1 to 0811c24 Compare April 20, 2026 08:30

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from 0811c24 to af44b2c Compare April 27, 2026 20:33

bb-Ricardo and others added 13 commits May 8, 2026 22:51

adds validation of key fingerprint string

f68d32c

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

adds better git signature detection format

9613fbe

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

adds detection for added signature prefixes

d249a1f

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

adds tests for git.go

79b31e9

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

adds requested changes regarding naming

e2adc4a

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

cleans up tests and removed duplicate test files

e21d896

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

fixes text fixtures public key names

52df5ec

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

adds 'SignatureTypeEmpty' check and improves description of 'GetSigna…

86f85a6

…tureType' Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

Update git/signatures/testdata/gpg_signatures/generate_gpg_fixtures.sh

7906728

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com> Signed-off-by: Ricardo <ricardo@bitchbrothers.com> Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

removes insecure DSA keys from signature test

d505af1

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

updates git go.mod dependencies

70dca9f

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

fixes tests for unsigned tags

074c9de

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from af44b2c to 074c9de Compare May 8, 2026 21:34

hiddeco reviewed May 18, 2026

View reviewed changes

matheuscscp changed the title ~~adds SSH signature validation for git commits~~ Add SSH signature verification for git commits May 18, 2026

bb-Ricardo added 3 commits May 18, 2026 23:22

renames package to signature and fixes requested changes

9ab4520

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

switches signarture type from public to private

5f6af02

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

moves buildTag and buildCommitWithRef to internal/build package

9425447

Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

improves signature validation to test all provided keys/keyrings if m…

f67d6c3

…alfored key is included Signed-off-by: Ricardo Bartels <ricardo.bartels@telekom.de>

bb-Ricardo force-pushed the rb-feature/adds-ssh-signature-validation branch from 0d42c77 to f67d6c3 Compare May 19, 2026 05:56

		err := sshsig.Verify(bytes.NewReader(payload), sig, pubKey, sig.HashAlgorithm, SSHSignatureNamespace)
		if err == nil {

		@@ -0,0 +1,5 @@
		sign-user@example.com namespaces="git" ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQCpreiO+8XsB4xXGNmwuO48a7WPghb5ihCJNPyQZpnaPfq6vhNVWSgq8AIjBmJOJYo4HZyiHqpS4OBc86glk6qMv8YHRt4VRVBP+DjPLDIsOR7+2HBlOPHMm8lTDi+iMPHBDxqFy7mSDB4+v7n700+49vYhWjZJpesnnE6JoitxSVhmqp75jeNRNU6PD00z+gMUcviv8UOs/Apg1Cw5f+4T9yOnjlOHaFH/ButvZ0t2VF0cs28tfCuLAoumjine5Gm6tCRQlZOoapNJzvnYT+86f/PEU/4kDYf3wT7S+NnUDfCsIpDVlOXPvjnQ/DudhqEnnXvfch+eBCI7rtJBHIGPKFdmC4cUROa0UDGR6o/JxLtx4ZTbkGpq6MVwdrb7qJ+Oib1U8xVimWFfarkm7deVXWD3wB5Wa8Ko/a/WuYfE3gYRhb8iXPYd71FsEy4F41JCMZDcIqMiQRe3e2gvY+z2sf02kHOFeWJmrAY9FFjPL85VD0Dg++jrExkGFjcBTw9gUG5OPGpwqQ9WHO8E8DPza+i5J/wu4DODyLrLxuXHPeSYUjcvh5ln8P70qL+Irwn1mgn2PkIZW0XCPBt6Iylg55t5sfyy03P0Kmb4U3TrppMeig7Lr9LDU4Doh7Fj6oLYDGFUV+F52SSuPs5SfrWd6Apiz+VPjsAh5btPPJNlzQ== test-rsa@example.com

Conversation

bb-Ricardo commented Feb 27, 2026

Uh oh!

stefanprodan commented Feb 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bb-Ricardo commented Mar 10, 2026

Uh oh!

stefanprodan commented Mar 10, 2026

Uh oh!

stefanprodan left a comment

Choose a reason for hiding this comment

Uh oh!

bb-Ricardo commented Mar 10, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bb-Ricardo commented Mar 17, 2026

Uh oh!

pjbgf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stefanprodan commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bb-Ricardo commented Mar 19, 2026

Uh oh!

bb-Ricardo commented Apr 20, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bb-Ricardo commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

stefanprodan commented Mar 19, 2026 •

edited

Loading