Skip to content

Commit cbc3be6

Browse files
pjbgfpjbgf
authored
Merge pull request #11 from go-git/expand-sign
objectsigner: Add context to Sign and KeyID to ssh signer
2 parents 990a634 + fc77819 commit cbc3be6

8 files changed

Lines changed: 107 additions & 30 deletions

File tree

‎plugin/objectsigner/gpg/example_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package gpg_test
22

33
import (
4+
"context"
45
"fmt"
56
"strings"
67

@@ -21,7 +22,7 @@ func ExampleFromKey() {
2122
panic(err)
2223
}
2324

24-
sig, err := signer.Sign(strings.NewReader("signed commit message\n"))
25+
sig, err := signer.Sign(context.Background(), strings.NewReader("signed commit message\n"))
2526
if err != nil {
2627
panic(err)
2728
}

‎plugin/objectsigner/gpg/gpg.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package gpg
44

55
import (
66
"bytes"
7+
"context"
78
"errors"
89
"fmt"
910
"io"
@@ -36,8 +37,10 @@ type signer struct {
3637
}
3738

3839
// Sign reads message and returns an ASCII-armored detached GPG
39-
// signature created with the signer's OpenPGP key.
40-
func (s *signer) Sign(message io.Reader) ([]byte, error) {
40+
// signature created with the signer's OpenPGP key. The context is accepted
41+
// for interface uniformity across signers; native OpenPGP signing is purely
42+
// local and does not consult it.
43+
func (s *signer) Sign(_ context.Context, message io.Reader) ([]byte, error) {
4144
if message == nil {
4245
return nil, ErrNilMessage
4346
}

‎plugin/objectsigner/gpg/gpg_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestSign(t *testing.T) {
6262
signer, err := gpg.FromKey(generateTestKey(t))
6363
require.NoError(t, err)
6464

65-
sig, err := signer.Sign(test.message)
65+
sig, err := signer.Sign(t.Context(), test.message)
6666
if test.wantErr != "" {
6767
require.ErrorContains(t, err, test.wantErr)
6868
require.Nil(t, sig)
@@ -85,7 +85,7 @@ func TestSignVerifyRoundTrip(t *testing.T) {
8585

8686
message := "signed commit message\n"
8787

88-
sig, err := signer.Sign(strings.NewReader(message))
88+
sig, err := signer.Sign(t.Context(), strings.NewReader(message))
8989
require.NoError(t, err)
9090

9191
keyring := openpgp.EntityList{key}
@@ -106,10 +106,10 @@ func TestSignDifferentMessagesProduceDifferentSignatures(t *testing.T) {
106106
signer, err := gpg.FromKey(key)
107107
require.NoError(t, err)
108108

109-
sig1, err := signer.Sign(strings.NewReader("message one"))
109+
sig1, err := signer.Sign(t.Context(), strings.NewReader("message one"))
110110
require.NoError(t, err)
111111

112-
sig2, err := signer.Sign(strings.NewReader("message two"))
112+
sig2, err := signer.Sign(t.Context(), strings.NewReader("message two"))
113113
require.NoError(t, err)
114114

115115
assert.NotEqual(t, sig1, sig2, "different messages produced identical signatures")

‎plugin/objectsigner/program/program.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,17 +150,17 @@ func resolveProgram(program string, lookPath func(string) (string, error)) (stri
150150
}
151151

152152
// Sign reads message and returns the signature produced by the external
153-
// binary.
154-
func (s *signer) Sign(message io.Reader) ([]byte, error) {
153+
// binary. The context cancels the external program invocation.
154+
func (s *signer) Sign(ctx context.Context, message io.Reader) ([]byte, error) {
155155
if message == nil {
156156
return nil, ErrNilMessage
157157
}
158158

159159
switch s.format {
160160
case FormatOpenPGP, FormatX509:
161-
return s.signStdio(context.Background(), message)
161+
return s.signStdio(ctx, message)
162162
case FormatSSH:
163-
return s.signSSH(context.Background(), message)
163+
return s.signSSH(ctx, message)
164164
default:
165165
return nil, fmt.Errorf("%w: %q", ErrUnsupportedFormat, s.format)
166166
}

‎plugin/objectsigner/program/program_test.go‎

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,61 @@ func TestSign_NilMessage(t *testing.T) {
135135

136136
signer, _ := newTestSigner(t, FormatOpenPGP, "ABC", nil)
137137

138-
sig, err := signer.Sign(nil)
138+
sig, err := signer.Sign(t.Context(), nil)
139139
require.ErrorIs(t, err, ErrNilMessage)
140140
require.Nil(t, sig)
141141
}
142142

143+
// TestSign_ThreadsContext asserts Sign threads the caller's context into the
144+
// command invocation for every format, rather than building the command with a
145+
// fresh context.Background(). The proof is propagation: cancelling the context
146+
// passed to Sign is observable through the context the command was created
147+
// with, which is only possible if it is the same context.
148+
func TestSign_ThreadsContext(t *testing.T) {
149+
t.Parallel()
150+
151+
tests := []struct {
152+
name string
153+
format Format
154+
signingKey string
155+
}{
156+
{name: "openpgp", format: FormatOpenPGP, signingKey: "KEYID"},
157+
{name: "x509", format: FormatX509, signingKey: "KEYID"},
158+
{name: "ssh", format: FormatSSH, signingKey: "/path/to/key"},
159+
}
160+
161+
for _, test := range tests {
162+
t.Run(test.name, func(t *testing.T) {
163+
t.Parallel()
164+
165+
signer, calls := newTestSigner(t, test.format, test.signingKey, func(cmd *mockCommand) error {
166+
if test.format == FormatSSH {
167+
return writeSignatureFile(cmd.args[len(cmd.args)-1])
168+
}
169+
170+
_, err := io.WriteString(cmd.stdout, "SIG\n")
171+
require.NoError(t, err)
172+
173+
return nil
174+
})
175+
176+
ctx, cancel := context.WithCancel(t.Context())
177+
t.Cleanup(cancel)
178+
179+
_, err := signer.Sign(ctx, strings.NewReader("body"))
180+
require.NoError(t, err)
181+
182+
require.Len(t, calls(), 1)
183+
got := calls()[0].ctx
184+
require.NotNil(t, got)
185+
186+
require.NoError(t, got.Err())
187+
cancel()
188+
assert.ErrorIs(t, got.Err(), context.Canceled)
189+
})
190+
}
191+
}
192+
143193
func TestSign_StdioFormats(t *testing.T) {
144194
t.Parallel()
145195

@@ -163,7 +213,7 @@ func TestSign_StdioFormats(t *testing.T) {
163213
return nil
164214
})
165215

166-
sig, err := signer.Sign(strings.NewReader("commit body\n"))
216+
sig, err := signer.Sign(t.Context(), strings.NewReader("commit body\n"))
167217
require.NoError(t, err)
168218
assert.Equal(t, "STDIO-SIG\n", string(sig))
169219
assert.Equal(t, "commit body\n", stdin)
@@ -184,7 +234,7 @@ func TestSign_StdioFailure(t *testing.T) {
184234
return errStdioExit
185235
})
186236

187-
sig, err := signer.Sign(strings.NewReader("body"))
237+
sig, err := signer.Sign(t.Context(), strings.NewReader("body"))
188238
require.Error(t, err)
189239
assert.Contains(t, err.Error(), "stdio failed")
190240
require.Nil(t, sig)
@@ -206,7 +256,7 @@ func TestSign_SSH(t *testing.T) {
206256
return writeSignatureFile(bufferFile)
207257
})
208258

209-
sig, err := signer.Sign(strings.NewReader("commit body\n"))
259+
sig, err := signer.Sign(t.Context(), strings.NewReader("commit body\n"))
210260
require.NoError(t, err)
211261
assert.Equal(t, "SSH-SIG\n", string(sig))
212262
assert.Equal(t, "commit body\n", buffer)
@@ -234,7 +284,7 @@ func TestSign_SSHExpandsHomePath(t *testing.T) {
234284
return writeSignatureFile(bufferFile)
235285
})
236286

237-
sig, err := signer.Sign(strings.NewReader("commit body\n"))
287+
sig, err := signer.Sign(t.Context(), strings.NewReader("commit body\n"))
238288
require.NoError(t, err)
239289
assert.Equal(t, "SSH-SIG\n", string(sig))
240290

@@ -290,7 +340,7 @@ func TestSign_SSHLiteralKey(t *testing.T) {
290340
return writeSignatureFile(bufferFile)
291341
})
292342

293-
sig, err := signer.Sign(strings.NewReader("commit body\n"))
343+
sig, err := signer.Sign(t.Context(), strings.NewReader("commit body\n"))
294344
require.NoError(t, err)
295345
assert.Equal(t, "SSH-SIG\n", string(sig))
296346
assert.Equal(t, "commit body\n", buffer)
@@ -320,7 +370,7 @@ func TestSign_SSHFailure(t *testing.T) {
320370
return errSSHExit
321371
})
322372

323-
sig, err := signer.Sign(strings.NewReader("body"))
373+
sig, err := signer.Sign(t.Context(), strings.NewReader("body"))
324374
require.Error(t, err)
325375
assert.Contains(t, err.Error(), "ssh failed")
326376
require.Nil(t, sig)
@@ -335,7 +385,7 @@ func TestSign_SSHPathPrefixedSshDash(t *testing.T) {
335385
return writeSignatureFile(bufferFile)
336386
})
337387

338-
sig, err := signer.Sign(strings.NewReader("body"))
388+
sig, err := signer.Sign(t.Context(), strings.NewReader("body"))
339389
require.NoError(t, err)
340390
require.NotNil(t, sig)
341391

@@ -359,7 +409,7 @@ func TestSign_StdioOutputTooLarge(t *testing.T) {
359409
return nil
360410
})
361411

362-
sig, err := signer.Sign(strings.NewReader("body"))
412+
sig, err := signer.Sign(t.Context(), strings.NewReader("body"))
363413
require.ErrorIs(t, err, ErrOutputLimitExceeded)
364414
assert.Contains(t, err.Error(), "stdout")
365415
require.Nil(t, sig)
@@ -379,7 +429,7 @@ func TestSign_StderrTooLarge(t *testing.T) {
379429
return nil
380430
})
381431

382-
sig, err := signer.Sign(strings.NewReader("body"))
432+
sig, err := signer.Sign(t.Context(), strings.NewReader("body"))
383433
require.ErrorIs(t, err, ErrOutputLimitExceeded)
384434
assert.Contains(t, err.Error(), "stderr")
385435
require.Nil(t, sig)
@@ -401,12 +451,13 @@ func TestSign_SSHSignatureTooLarge(t *testing.T) {
401451
return nil
402452
})
403453

404-
sig, err := signer.Sign(strings.NewReader("body"))
454+
sig, err := signer.Sign(t.Context(), strings.NewReader("body"))
405455
require.ErrorIs(t, err, ErrSignatureTooLarge)
406456
require.Nil(t, sig)
407457
}
408458

409459
type mockCommand struct {
460+
ctx context.Context //nolint:containedctx // captured to assert Sign threads its context into the command.
410461
run func(*mockCommand) error
411462
stdin io.Reader
412463
stdout io.Writer
@@ -457,8 +508,9 @@ func stubCommand(run func(*mockCommand) error) (
457508
) {
458509
calls := make([]*mockCommand, 0, 1)
459510

460-
commandContext := func(_ context.Context, binary string, args ...string) command {
511+
commandContext := func(ctx context.Context, binary string, args ...string) command {
461512
cmd := &mockCommand{
513+
ctx: ctx,
462514
run: run,
463515
stdin: nil,
464516
stdout: nil,

‎plugin/objectsigner/ssh/example_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package ssh_test
22

33
import (
4+
"context"
45
"crypto/ed25519"
56
"crypto/rand"
67
"fmt"
@@ -29,7 +30,7 @@ func ExampleFromKey() {
2930
panic(err)
3031
}
3132

32-
sig, err := signer.Sign(strings.NewReader("signed commit message\n"))
33+
sig, err := signer.Sign(context.Background(), strings.NewReader("signed commit message\n"))
3334
if err != nil {
3435
panic(err)
3536
}

‎plugin/objectsigner/ssh/ssh.go‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package ssh
55

66
import (
7+
"context"
78
"errors"
89
"fmt"
910
"io"
@@ -69,8 +70,10 @@ type signer struct {
6970

7071
// Sign reads the message from r and returns an armored SSH signature created
7172
// with the signer's SSH key and hash algorithm.
72-
// The signature uses the "git" namespace.
73-
func (s *signer) Sign(message io.Reader) ([]byte, error) {
73+
// The signature uses the "git" namespace. The context is accepted for
74+
// interface uniformity across signers; SSH signing is purely local and does
75+
// not consult it.
76+
func (s *signer) Sign(_ context.Context, message io.Reader) ([]byte, error) {
7477
if message == nil {
7578
return nil, ErrNilMessage
7679
}
@@ -82,3 +85,9 @@ func (s *signer) Sign(message io.Reader) ([]byte, error) {
8285

8386
return sshsig.Armor(sig), nil
8487
}
88+
89+
// KeyID returns the SHA256 fingerprint of the signer's SSH public key,
90+
// in the form "SHA256:...".
91+
func (s *signer) KeyID() string {
92+
return gossh.FingerprintSHA256(s.signer.PublicKey())
93+
}

‎plugin/objectsigner/ssh/ssh_test.go‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestSign(t *testing.T) {
7171
signer, err := ssh.FromKey(sshSigner, ssh.WithHashAlgorithm(test.algo))
7272
require.NoError(t, err)
7373

74-
sig, err := signer.Sign(test.message)
74+
sig, err := signer.Sign(t.Context(), test.message)
7575
if test.wantErr == "" {
7676
require.NoError(t, err)
7777
assert.NotEmpty(t, sig)
@@ -95,7 +95,7 @@ func TestSignVerifyRoundTrip(t *testing.T) {
9595

9696
message := "signed commit message\n"
9797

98-
sig, err := signer.Sign(strings.NewReader(message))
98+
sig, err := signer.Sign(t.Context(), strings.NewReader(message))
9999
require.NoError(t, err)
100100

101101
ssig, err := sshsig.Unarmor(sig)
@@ -113,15 +113,26 @@ func TestSignDifferentMessagesProduceDifferentSignatures(t *testing.T) {
113113
signer, err := ssh.FromKey(sshSigner)
114114
require.NoError(t, err)
115115

116-
sig1, err := signer.Sign(strings.NewReader("message one"))
116+
sig1, err := signer.Sign(t.Context(), strings.NewReader("message one"))
117117
require.NoError(t, err)
118118

119-
sig2, err := signer.Sign(strings.NewReader("message two"))
119+
sig2, err := signer.Sign(t.Context(), strings.NewReader("message two"))
120120
require.NoError(t, err)
121121

122122
assert.NotEqual(t, sig1, sig2, "different messages produced identical signatures")
123123
}
124124

125+
func TestKeyID(t *testing.T) {
126+
t.Parallel()
127+
128+
key := generateTestSigner(t)
129+
signer, err := ssh.FromKey(key)
130+
require.NoError(t, err)
131+
132+
assert.Equal(t, gossh.FingerprintSHA256(key.PublicKey()), signer.KeyID())
133+
assert.Contains(t, signer.KeyID(), "SHA256:")
134+
}
135+
125136
//nolint:ireturn // gossh.NewSignerFromKey returns gossh.Signer (interface); no concrete type is accessible
126137
func generateTestSigner(t *testing.T) gossh.Signer {
127138
t.Helper()

0 commit comments

Comments
 (0)