Skip to content

Commit

Permalink
Add support for checking cert email against user config before signin…
Browse files Browse the repository at this point in the history
…g. (#246)

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 <billy@chainguard.dev>
  • Loading branch information
wlynch authored Feb 23, 2023
1 parent 0a4c62a commit 38ef0f7
Show file tree
Hide file tree
Showing 5 changed files with 171 additions and 4 deletions.
34 changes: 34 additions & 0 deletions docs/committer-verification.md
Original file line number Diff line number Diff line change
@@ -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
```
9 changes: 7 additions & 2 deletions internal/commands/root/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
13 changes: 12 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"):
Expand All @@ -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")
}
}
}
Expand Down
47 changes: 46 additions & 1 deletion internal/signature/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto/x509"
"encoding/pem"
"fmt"
"strings"

cms "github.com/github/smimesign/ietf-cms"
)
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
72 changes: 72 additions & 0 deletions internal/signature/sign_test.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 38ef0f7

Please sign in to comment.