diff --git a/client/client_test.go b/client/client_test.go index 80d9f9b8b..140fd2ab7 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -12164,7 +12164,7 @@ func testGitResolveSourceMetadata(t *testing.T, sb integration.Sandbox) { commit, err := commitObj.ToCommit() require.NoError(t, err) - require.Equal(t, "msg\n", commit.Message) + require.Equal(t, "msg", commit.Message) require.Equal(t, "test", commit.Author.Name) require.Equal(t, "test@example.com", commit.Author.Email) require.Equal(t, "test", commit.Committer.Name) @@ -12179,7 +12179,7 @@ func testGitResolveSourceMetadata(t *testing.T, sb integration.Sandbox) { tag, err := tagObj.ToTag() require.NoError(t, err) - require.Equal(t, "v0.1release\n", tag.Message) + require.Equal(t, "v0.1release", tag.Message) require.Equal(t, "v0.1", tag.Tag) require.Equal(t, "test", tag.Tagger.Name) require.Equal(t, "test@example.com", tag.Tagger.Email) diff --git a/source/git/identifier.go b/source/git/identifier.go index 7b85fc2c4..02f490185 100644 --- a/source/git/identifier.go +++ b/source/git/identifier.go @@ -21,6 +21,15 @@ type GitIdentifier struct { MountSSHSock string KnownSSHHosts string SkipSubmodules bool + + VerifySignature *GitSignatureVerifyOptions +} + +type GitSignatureVerifyOptions struct { + PubKey []byte + RejectExpiredKeys bool + RequireSignedTag bool // signed tag must be present + IgnoreSignedTag bool // even if signed tag is present, verify signature on commit object } func NewGitIdentifier(remoteURL string) (*GitIdentifier, error) { diff --git a/source/git/source.go b/source/git/source.go index 1ba7cabcc..1b4f02c3c 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -28,6 +28,8 @@ import ( srctypes "github.com/moby/buildkit/source/types" "github.com/moby/buildkit/util/bklog" "github.com/moby/buildkit/util/gitutil" + "github.com/moby/buildkit/util/gitutil/gitobject" + "github.com/moby/buildkit/util/gitutil/gitsign" "github.com/moby/buildkit/util/progress/logs" "github.com/moby/buildkit/util/urlutil" "github.com/moby/locker" @@ -255,58 +257,67 @@ func (gs *Source) ResolveMetadata(ctx context.Context, id *GitIdentifier, sm *se return nil, err } - if !opt.ReturnObject { + if !opt.ReturnObject && id.VerifySignature == nil { return md, nil } gsh.cacheCommit = md.Checksum gsh.sha256 = len(md.Checksum) == 64 - repo, err := gsh.remoteFetch(ctx, nil) - if err != nil { + + if err := gsh.addGitObjectsToMetadata(ctx, jobCtx, md); err != nil { return nil, err } - defer repo.Release() - // if ref was commit sha then we don't know the type of the object yet - buf, err := repo.Run(ctx, "cat-file", "-t", md.Checksum) - if err != nil { - return nil, err - } - objType := strings.TrimSpace(string(buf)) - - if objType != "commit" && objType != "tag" { - return nil, errors.Errorf("expected commit or tag object, got %s", objType) - } - - if objType == "tag" && md.CommitChecksum == "" { - buf, err := repo.Run(ctx, "rev-parse", md.Checksum+"^{commit}") - if err != nil { + if id.VerifySignature != nil { + if err := verifyGitSignature(md, id.VerifySignature); err != nil { return nil, err } - md.CommitChecksum = strings.TrimSpace(string(buf)) - } else if objType == "commit" { - md.CommitChecksum = "" } - commitChecksum := md.Checksum - if md.CommitChecksum != "" { - buf, err := repo.Run(ctx, "cat-file", "tag", md.Checksum) - if err != nil { - return nil, err - } - md.TagObject = buf - commitChecksum = md.CommitChecksum - } - - buf, err = repo.Run(ctx, "cat-file", "commit", commitChecksum) - if err != nil { - return nil, err - } - md.CommitObject = buf - return md, nil } +func verifyGitSignature(md *Metadata, opts *GitSignatureVerifyOptions) error { + var tagVerifyError error + if !opts.IgnoreSignedTag { + if len(md.TagObject) > 0 { + tagObj, err := gitobject.Parse(md.TagObject) + if err != nil { + return errors.Wrap(err, "failed to parse git tag object") + } + if err := tagObj.VerifyChecksum(md.Checksum); err != nil { + return errors.Wrap(err, "tag object checksum verification failed") + } + tagVerifyError = gitsign.VerifySignature(tagObj, opts.PubKey, &gitsign.VerifyPolicy{ + RejectExpiredKeys: opts.RejectExpiredKeys, + }) + if tagVerifyError == nil { + return nil + } + } + } + if opts.RequireSignedTag { + if tagVerifyError != nil { + return tagVerifyError + } + return errors.New("signed tag required but no signed tag found") + } + commitObj, err := gitobject.Parse(md.CommitObject) + if err != nil { + return errors.Wrap(err, "failed to parse git commit object") + } + expected := md.Checksum + if md.CommitChecksum != "" { + expected = md.CommitChecksum + } + if err := commitObj.VerifyChecksum(expected); err != nil { + return errors.Wrap(err, "commit object checksum verification failed") + } + return gitsign.VerifySignature(commitObj, opts.PubKey, &gitsign.VerifyPolicy{ + RejectExpiredKeys: opts.RejectExpiredKeys, + }) +} + func (gs *Source) Resolve(ctx context.Context, id source.Identifier, sm *session.Manager, _ solver.Vertex) (source.SourceInstance, error) { gitIdentifier, ok := id.(*GitIdentifier) if !ok { @@ -587,17 +598,74 @@ func (gs *gitSourceHandler) resolveMetadata(ctx context.Context, jobCtx solver.J return md, nil } +func (gs *gitSourceHandler) addGitObjectsToMetadata(ctx context.Context, jobCtx solver.JobContext, md *Metadata) error { + repo, err := gs.remoteFetch(ctx, jobCtx) + if err != nil { + return err + } + defer repo.Release() + + // if ref was commit sha then we don't know the type of the object yet + buf, err := repo.Run(ctx, "cat-file", "-t", md.Checksum) + if err != nil { + return err + } + objType := strings.TrimSpace(string(buf)) + + if objType != "commit" && objType != "tag" { + return errors.Errorf("expected commit or tag object, got %s", objType) + } + + if objType == "tag" && md.CommitChecksum == "" { + buf, err := repo.Run(ctx, "rev-parse", md.Checksum+"^{commit}") + if err != nil { + return err + } + md.CommitChecksum = strings.TrimSpace(string(buf)) + } else if objType == "commit" { + md.CommitChecksum = "" + } + + commitChecksum := md.Checksum + if md.CommitChecksum != "" { + buf, err := repo.Run(ctx, "cat-file", "tag", md.Checksum) + if err != nil { + return err + } + md.TagObject = buf + commitChecksum = md.CommitChecksum + } + + buf, err = repo.Run(ctx, "cat-file", "commit", commitChecksum) + if err != nil { + return err + } + md.CommitObject = buf + return nil +} + func (gs *gitSourceHandler) CacheKey(ctx context.Context, jobCtx solver.JobContext, index int) (string, string, solver.CacheOpts, bool, error) { md, err := gs.resolveMetadata(ctx, jobCtx) if err != nil { return "", "", nil, false, err } + gs.sha256 = len(md.Checksum) == 64 + + if gs.src.VerifySignature != nil { + gs.cacheCommit = md.Checksum + if err := gs.addGitObjectsToMetadata(ctx, jobCtx, md); err != nil { + return "", "", nil, false, err + } + if err := verifyGitSignature(md, gs.src.VerifySignature); err != nil { + return "", "", nil, false, err + } + } + if gitutil.IsCommitSHA(md.Ref) { cacheKey := gs.shaToCacheKey(md.Ref, md.Ref) gs.cacheKey = cacheKey gs.cacheCommit = md.Ref - gs.sha256 = len(md.Ref) == 64 // gs.src.Checksum is verified when checking out the commit return cacheKey, md.Ref, nil, true, nil } @@ -609,12 +677,11 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, jobCtx solver.JobConte } cacheKey := gs.shaToCacheKey(shaForCacheKey, md.Ref) gs.cacheKey = cacheKey - gs.sha256 = len(md.Checksum) == 64 gs.cacheCommit = md.Checksum return cacheKey, md.Checksum, nil, true, nil } -func (gs *gitSourceHandler) remoteFetch(ctx context.Context, g session.Group) (_ *gitRepo, retErr error) { +func (gs *gitSourceHandler) remoteFetch(ctx context.Context, jobCtx solver.JobContext) (_ *gitRepo, retErr error) { gs.locker.Lock(gs.src.Remote) cleanup := func() error { return gs.locker.Unlock(gs.src.Remote) } @@ -624,6 +691,11 @@ func (gs *gitSourceHandler) remoteFetch(ctx context.Context, g session.Group) (_ } }() + var g session.Group + if jobCtx != nil { + g = jobCtx.Session() + } + repo, err := gs.tryRemoteFetch(ctx, g, false) if err != nil { var wce *wouldClobberExistingTagError @@ -701,7 +773,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, jobCtx solver.JobConte return gs.cache.Get(ctx, sis[0].ID(), nil) } - repo, err := gs.remoteFetch(ctx, g) + repo, err := gs.remoteFetch(ctx, jobCtx) if err != nil { return nil, err } diff --git a/source/git/source_test.go b/source/git/source_test.go index eca7e3d52..51dae8372 100644 --- a/source/git/source_test.go +++ b/source/git/source_test.go @@ -1454,7 +1454,7 @@ func testResolveMetadataObject(t *testing.T, keepGitDir bool, format string) { require.NoError(t, err) require.Equal(t, "v1.2.3", tag.Tag) - require.Equal(t, "this is an annotated tag\n", tag.Message) + require.Equal(t, "this is an annotated tag", tag.Message) require.Equal(t, "test-user", tag.Tagger.Name) require.Equal(t, "test-user@example.com", tag.Tagger.Email) @@ -1474,7 +1474,7 @@ func testResolveMetadataObject(t *testing.T, keepGitDir bool, format string) { commit, err := commitObject.ToCommit() require.NoError(t, err) - require.Equal(t, "second\n", commit.Message) + require.Equal(t, "second", commit.Message) require.Equal(t, "test-user", commit.Author.Name) require.Equal(t, "test-user@example.com", commit.Author.Email) require.Equal(t, "test-user", commit.Committer.Name) @@ -1728,6 +1728,7 @@ func testCheckSignatures(t *testing.T, keepGitDir bool, format string) { ob, err := gitobject.Parse(md.CommitObject) require.NoError(t, err) + require.Greater(t, len(ob.Signature), 50) fixturesBase := os.Getenv("BUILDKIT_TEST_SIGN_FIXTURES") @@ -1753,6 +1754,7 @@ func testCheckSignatures(t *testing.T, keepGitDir bool, format string) { ob, err = gitobject.Parse(md.TagObject) require.NoError(t, err) + require.Greater(t, len(ob.Signature), 50) sshkey1, err := os.ReadFile(fixturesBase + "/user1.ssh.pub") require.NoError(t, err) @@ -1778,6 +1780,219 @@ func testCheckSignatures(t *testing.T, keepGitDir bool, format string) { require.ErrorContains(t, err, "public key does not match") } +func TestVerifySignaturesSHA1(t *testing.T) { + testVerifySignatures(t, false, "sha1") +} + +func TestVerifySignaturesKeepGitDirSHA1(t *testing.T) { + testVerifySignatures(t, true, "sha1") +} + +func TestVerifySignaturesSHA256(t *testing.T) { + testVerifySignatures(t, false, "sha256") +} + +func TestVerifySignaturesKeepGitDirSHA256(t *testing.T) { + testVerifySignatures(t, true, "sha256") +} + +func testVerifySignatures(t *testing.T, keepGitDir bool, format string) { + if runtime.GOOS == "windows" { + t.Skip("Depends on unimplemented containerd bind-mount support on Windows") + } + t.Parallel() + requireSignFixtures(t) + ctx := namespaces.WithNamespace(context.Background(), "buildkit-test") + ctx = logProgressStreams(ctx, t) + + gs := setupGitSource(t, t.TempDir()) + + repo := setupGitRepo(t, format) + + fixturesBase := os.Getenv("BUILDKIT_TEST_SIGN_FIXTURES") + + user1GPGPub, err := os.ReadFile(fixturesBase + "/user1.gpg.pub") + require.NoError(t, err) + + user2GPGPub, err := os.ReadFile(fixturesBase + "/user2.gpg.pub") + require.NoError(t, err) + + user1SSHPub, err := os.ReadFile(fixturesBase + "/user1.ssh.pub") + require.NoError(t, err) + + user2SSHPub, err := os.ReadFile(fixturesBase + "/user2.ssh.pub") + require.NoError(t, err) + + // a/v1.2.3 commit is signed by user1 gpg + // v1.2.3 commit is signed by user1 ssh + // v1.2.3 is a signed tag by user2 ssh + // v1.2.3-special is not signed + + id := &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "a/v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user2GPGPub, // wrong key + }, + } + + gsi, err := gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + _, _, _, _, err = gsi.CacheKey(ctx, nil, 0) + require.ErrorContains(t, err, "signature made by unknown entity") + require.ErrorContains(t, err, "signature by") + + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "v1.2.3-special", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user2GPGPub, + }, + } + + gsi, err = gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + _, _, _, _, err = gsi.CacheKey(ctx, nil, 0) + require.ErrorContains(t, err, "git object is not signed") + + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "a/v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user1GPGPub, // correct + }, + } + + gsi, err = gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + _, pin, _, done, err := gsi.CacheKey(ctx, nil, 0) + require.NoError(t, err) + require.True(t, done) + + expLen := 40 + if format == "sha256" { + expLen = 64 + } + require.Equal(t, expLen, len(pin)) + + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "a/v1.2.3", // not signed tag + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user1GPGPub, + RequireSignedTag: true, + }, + } + + gsi, err = gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + _, _, _, _, err = gsi.CacheKey(ctx, nil, 0) + require.ErrorContains(t, err, "signed tag required but no signed tag found") + + // signed tag can be validated via commit signature + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user1SSHPub, + RequireSignedTag: false, + }, + } + + gsi, err = gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + _, pin, _, done, err = gsi.CacheKey(ctx, nil, 0) + require.NoError(t, err) + require.True(t, done) + require.Equal(t, expLen, len(pin)) + + // but not when signed tag is required + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user1SSHPub, + RequireSignedTag: true, + }, + } + + gsi, err = gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + _, _, _, _, err = gsi.CacheKey(ctx, nil, 0) + require.ErrorContains(t, err, "failed to verify ssh signature") + + // correct key for signed tag + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user2SSHPub, + RequireSignedTag: true, + }, + } + + gsi, err = gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + _, pin, _, done, err = gsi.CacheKey(ctx, nil, 0) + require.NoError(t, err) + require.True(t, done) + require.Equal(t, expLen, len(pin)) + + // repeat three last checks via ResolveSourceMetadata + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user1SSHPub, + RequireSignedTag: false, + }, + } + + _, err = gs.ResolveMetadata(ctx, id, nil, nil, MetadataOpts{}) + require.NoError(t, err) + + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user1SSHPub, + RequireSignedTag: true, + }, + } + + _, err = gs.ResolveMetadata(ctx, id, nil, nil, MetadataOpts{}) + require.ErrorContains(t, err, "failed to verify ssh signature") + + id = &GitIdentifier{ + Remote: repo.mainURL, + KeepGitDir: keepGitDir, + Ref: "v1.2.3", + VerifySignature: &GitSignatureVerifyOptions{ + PubKey: user2SSHPub, + RequireSignedTag: true, + }, + } + + _, err = gs.ResolveMetadata(ctx, id, nil, nil, MetadataOpts{}) + require.NoError(t, err) +} + func TestSubdir(t *testing.T) { testSubdir(t, false) } diff --git a/util/gitutil/gitobject/parse.go b/util/gitutil/gitobject/parse.go index 3ccc53c06..7c35fd973 100644 --- a/util/gitutil/gitobject/parse.go +++ b/util/gitutil/gitobject/parse.go @@ -110,6 +110,7 @@ func Parse(raw []byte) (*GitObject, error) { } obj.Message = strings.Join(messageLines, "\n") + obj.Message = strings.TrimSuffix(obj.Message, "\n") // body ends with newline but no extra newline between message and signature if len(sigLines) > 0 { obj.Signature = strings.Join(sigLines, "\n") } diff --git a/util/gitutil/gitobject/parse_test.go b/util/gitutil/gitobject/parse_test.go index f7bf168bb..5acf710ee 100644 --- a/util/gitutil/gitobject/parse_test.go +++ b/util/gitutil/gitobject/parse_test.go @@ -159,7 +159,8 @@ type commit tag v0.25.0-rc1 tagger Tonis Tiigi 1758658255 -0700 -v0.25.0-rc1` +v0.25.0-rc1 +` require.Equal(t, expectedTagSignedData, obj.SignedData) sum, err = obj.Checksum(sha1.New) diff --git a/util/gitutil/gitsign/gitsign.go b/util/gitutil/gitsign/gitsign.go index a8bcfa4fc..355aac215 100644 --- a/util/gitutil/gitsign/gitsign.go +++ b/util/gitutil/gitsign/gitsign.go @@ -35,6 +35,10 @@ type VerifyPolicy struct { } func VerifySignature(obj *gitobject.GitObject, pubKeyData []byte, policy *VerifyPolicy) error { + if len(obj.Signature) == 0 { + return errors.New("git object is not signed") + } + s, err := ParseSignature([]byte(obj.Signature)) if err != nil { return err