From 60b6656d931c898280eeda6cc3fa46eeadfd1cc6 Mon Sep 17 00:00:00 2001 From: Billy Lynch Date: Tue, 21 Feb 2023 18:38:49 -0500 Subject: [PATCH] Add support for checking cert email against user config before signing. This change adds a new config option: gitsign.matchCommitter. This option checks whether the certificate fetched matches the user configured email/name. For human users, this generally means that the SAN email in the cert matches the `user.email` Git config option. For non-email based identities (e.g. machine users), the SAN URI can be specified as the user name (since the URI isn't a valid email). Gitsign requires at least one condition to match for the check to succeed. This change does *not* enforce any constraints on verification, since this requires additional checking to know what IdP is considered valid. Signed-off-by: Billy Lynch --- docs/committer-verification.md | 34 ++++++++++++++++ internal/commands/root/sign.go | 9 ++++- internal/config/config.go | 13 +++++- internal/signature/sign.go | 47 ++++++++++++++++++++- internal/signature/sign_test.go | 72 +++++++++++++++++++++++++++++++++ 5 files changed, 171 insertions(+), 4 deletions(-) create mode 100644 docs/committer-verification.md create mode 100644 internal/signature/sign_test.go diff --git a/docs/committer-verification.md b/docs/committer-verification.md new file mode 100644 index 00000000..3c47e41f --- /dev/null +++ b/docs/committer-verification.md @@ -0,0 +1,34 @@ +# Committer Verification + +Gitsign can be optionally configured to verify that the committer user identity +matches the git user configuration (i.e. `user.name` and `user.email`) + +To enable committer verification, run `git config gitsign.matchCommitter true`. + +Committer verification is done by matching the certificate Subject Alternative +Name (SAN) against the issued Fulcio certificate in the following order: + +1. An `EmailAddresses` cert value matches the committer `user.email`. This + should be used for most human committer verification. +2. A `URI` cert value matches the committer `user.name`. This should be used for + most automated user committer verification. + +In the event that multiple SAN values are provided, verification will succeed if +at least one value matches. + +## Configuring Automated Users + +If running in an environment where the authenticated user does **not** have a +user email to bind against (i.e. GitHub Actions, other workload identity +workflows), users are expected to be identified by the SAN URI instead. + +See https://github.com/sigstore/fulcio/blob/main/docs/oidc.md for more details + +### GitHub Actions + +```sh +# This configures the SAN URI for the expected identity in the Fulcio cert. +$ git config user.name "https://myorg/myrepo/path/to/workflow" +# This configures GitHub UI to recognize the commit as coming from a GitHub Action. +$ git config user.email 1234567890+github-actions@users.noreply.github.com +``` diff --git a/internal/commands/root/sign.go b/internal/commands/root/sign.go index c9dbe500..0eff4305 100644 --- a/internal/commands/root/sign.go +++ b/internal/commands/root/sign.go @@ -78,12 +78,17 @@ func commandSign(o *options, s *gsio.Streams, args ...string) error { return fmt.Errorf("failed to create rekor client: %w", err) } - sig, cert, tlog, err := git.Sign(ctx, rekor, userIdent, dataBuf.Bytes(), signature.SignOptions{ + opts := signature.SignOptions{ Detached: o.FlagDetachedSignature, TimestampAuthority: o.Config.TimestampURL, Armor: o.FlagArmor, IncludeCerts: o.FlagIncludeCerts, - }) + } + if o.Config.MatchCommitter { + opts.UserName = o.Config.CommitterName + opts.UserEmail = o.Config.CommitterEmail + } + sig, cert, tlog, err := git.Sign(ctx, rekor, userIdent, dataBuf.Bytes(), opts) if err != nil { return fmt.Errorf("failed to sign message: %w", err) } diff --git a/internal/config/config.go b/internal/config/config.go index 28ecd49d..59d3c24c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -58,6 +58,11 @@ type Config struct { // Path to log status output. Helpful for debugging when no TTY is available in the environment. LogPath string + + // Committer details + CommitterName string + CommitterEmail string + MatchCommitter bool } // Get fetches the gitsign config options for the repo in the current working @@ -112,7 +117,7 @@ func Get() (*Config, error) { // doesn't support deprecated subsection syntaxes // (https://github.com/sigstore/gitsign/issues/142). func realExec() (io.Reader, error) { - cmd := exec.Command("git", "config", "--get-regexp", `^gitsign\.`) + cmd := exec.Command("git", "config", "--get-regexp", `.*`) stdout := new(bytes.Buffer) stderr := new(bytes.Buffer) cmd.Stdout = stdout @@ -147,6 +152,10 @@ func parseConfig(r io.Reader) map[string]string { func applyGitOptions(out *Config, cfg map[string]string) { for k, v := range cfg { switch { + case strings.EqualFold(k, "user.name"): + out.CommitterName = v + case strings.EqualFold(k, "user.email"): + out.CommitterEmail = v case strings.EqualFold(k, "gitsign.fulcio"): out.Fulcio = v case strings.EqualFold(k, "gitsign.fulcioRoot"): @@ -167,6 +176,8 @@ func applyGitOptions(out *Config, cfg map[string]string) { out.TimestampURL = v case strings.EqualFold(k, "gitsign.timestampCertChain"): out.TimestampCert = v + case strings.EqualFold(k, "gitsign.matchCommitter"): + out.MatchCommitter = strings.EqualFold(v, "true") } } } diff --git a/internal/signature/sign.go b/internal/signature/sign.go index 9abc64c6..ae9fe7b9 100644 --- a/internal/signature/sign.go +++ b/internal/signature/sign.go @@ -21,6 +21,7 @@ import ( "crypto/x509" "encoding/pem" "fmt" + "strings" cms "github.com/github/smimesign/ietf-cms" ) @@ -40,6 +41,14 @@ type SignOptions struct { // 1 includes leaf cert. // >1 includes n from the leaf. IncludeCerts int + + // UserName specifies the email to match against. If present, signing + // will fail if the Fulcio identity SAN URI does not match the git committer name. + UserName string + + // UserEmail specifies the email to match against. If present, signing + // will fail if the Fulcio identity SAN email does not match the git committer email. + UserEmail string } // Identity is a copy of smimesign.Identity to allow for compatibility without @@ -63,8 +72,23 @@ type Identity interface { func Sign(ident Identity, body []byte, opts SignOptions) ([]byte, *x509.Certificate, error) { cert, err := ident.Certificate() if err != nil { - return nil, nil, fmt.Errorf("failed to get idenity certificate: %w", err) + return nil, nil, fmt.Errorf("failed to get identity certificate: %w", err) } + + // If specified, check if retrieved identity matches the expected identity. + if opts.UserName != "" || opts.UserEmail != "" { + if !matchSAN(cert, opts.UserName, opts.UserEmail) { + san := []string{} + if len(cert.EmailAddresses) > 0 { + san = append(san, fmt.Sprintf("email: %v", cert.EmailAddresses)) + } + if len(cert.URIs) > 0 { + san = append(san, fmt.Sprintf("uri: %v", cert.URIs)) + } + return nil, nil, fmt.Errorf("gitsign.matchCommitter: certificate identity does not match config - want %s <%s>, got %s", opts.UserName, opts.UserEmail, strings.Join(san, ",")) + } + } + signer, err := ident.Signer() if err != nil { return nil, nil, fmt.Errorf("failed to get idenity signer: %w", err) @@ -164,3 +188,24 @@ func chainWithoutRoot(chain []*x509.Certificate) []*x509.Certificate { return chain } + +// matchSAN checks whether a given cert SAN matches the given user name/email. +// At least 1 of the following needs to match to be considered successful: +// +// 1. SAN email == user email (typical for most human users) +// 2. SAN URI == user name (for non-email based identities like CI) +func matchSAN(cert *x509.Certificate, name, email string) bool { + for _, e := range cert.EmailAddresses { + if e == email { + return true + } + } + + for _, u := range cert.URIs { + if u.String() == name { + return true + } + } + + return false +} diff --git a/internal/signature/sign_test.go b/internal/signature/sign_test.go new file mode 100644 index 00000000..18cffa2b --- /dev/null +++ b/internal/signature/sign_test.go @@ -0,0 +1,72 @@ +// Copyright 2023 The Sigstore Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package signature + +import ( + "crypto/x509" + "net/url" + "testing" +) + +func TestMatchSAN(t *testing.T) { + for _, tc := range []struct { + testname string + cert *x509.Certificate + name string + email string + want bool + }{ + { + testname: "email match", + cert: &x509.Certificate{ + EmailAddresses: []string{"foo@example.com"}, + }, + name: "Foo Bar", + email: "foo@example.com", + want: true, + }, + { + testname: "uri match", + cert: &x509.Certificate{ + URIs: []*url.URL{parseURL("https://github.com/foo/bar")}, + }, + name: "https://github.com/foo/bar", + email: "foo@example.com", + want: true, + }, + { + testname: "no match", + cert: &x509.Certificate{}, + name: "https://github.com/foo/bar", + email: "foo@example.com", + want: false, + }, + } { + t.Run(tc.testname, func(t *testing.T) { + got := matchSAN(tc.cert, tc.name, tc.email) + if got != tc.want { + t.Fatalf("got %t, want %t", got, tc.want) + } + }) + } +} + +func parseURL(raw string) *url.URL { + u, err := url.Parse(raw) + if err != nil { + panic(err) + } + return u +}