From 70efbcfe1ae4a9eec460d158ad34cc65450e49ac Mon Sep 17 00:00:00 2001 From: Tom Meadows Date: Mon, 11 Dec 2023 17:54:32 +0000 Subject: [PATCH] Improving `--signer-fulcio-token` flag to accept both path and raw token string (#82) * modified `--signer-fulcio-token` flag to accept either a path to a token or a raw token string * modified `--signer-fulcio-token` flag to accept either a path to a token or a raw token string Signed-off-by: chaosinthecrd * adding an unhappy path to the tests Signed-off-by: chaosinthecrd * updated function and description * updated function and description Signed-off-by: chaosinthecrd * removing ineffectual assignment in test Signed-off-by: chaosinthecrd * updated to add token path flag and remove idToken function magic Signed-off-by: chaosinthecrd * removing ineffectual assignments Signed-off-by: chaosinthecrd * removing whitespace Signed-off-by: chaosinthecrd * fixing small issue and adding test to makefile to speed things up Signed-off-by: chaosinthecrd --------- Signed-off-by: chaosinthecrd --- Makefile | 6 ++++++ hack/test.token | 1 + signer/fulcio/fulcio.go | 39 +++++++++++++++++++++++++++++++----- signer/fulcio/fulcio_test.go | 21 +++++++++++++++++++ 4 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 hack/test.token diff --git a/Makefile b/Makefile index f717d8a7..2738f0f0 100644 --- a/Makefile +++ b/Makefile @@ -18,3 +18,9 @@ controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessar $(CONTROLLER_GEN): $(LOCALBIN) test -s $(LOCALBIN)/controller-gen && $(LOCALBIN)/controller-gen --version | grep -q $(CONTROLLER_TOOLS_VERSION) || \ GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION) + +test: ## Run the go unit tests + go test -v -coverprofile=profile.cov -covermode=atomic ./... + +help: ## Display this help screen + @grep -h -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' diff --git a/hack/test.token b/hack/test.token new file mode 100644 index 00000000..fbcb3807 --- /dev/null +++ b/hack/test.token @@ -0,0 +1 @@ +eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJmb29iYXIiLCJuYW1lIjoiSm9obiBEb2UiLCJpYXQiOjE1MTYyMzkwMjIsIkVtYWlsIjoidGVzdEBpbi10b3RvLmlvIn0.IswtNc6aJL3zAf-lSGvuz7Okf2tBr-I3ulJ_SRUMt0k diff --git a/signer/fulcio/fulcio.go b/signer/fulcio/fulcio.go index 8983f8c0..449e7553 100644 --- a/signer/fulcio/fulcio.go +++ b/signer/fulcio/fulcio.go @@ -93,7 +93,7 @@ func init() { ), registry.StringConfigOption( "token", - "Raw token to use for authentication", + "Raw token string to use for authentication to fulcio (cannot be used in conjunction with --fulcio-token-path)", "", func(sp signer.SignerProvider, token string) (signer.SignerProvider, error) { fsp, ok := sp.(FulcioSignerProvider) @@ -105,6 +105,20 @@ func init() { return fsp, nil }, ), + registry.StringConfigOption( + "token-path", + "Path to the file containing a raw token to use for authentication to fulcio (cannot be used in conjunction with --fulcio-token)", + "", + func(sp signer.SignerProvider, tokenPath string) (signer.SignerProvider, error) { + fsp, ok := sp.(FulcioSignerProvider) + if !ok { + return sp, fmt.Errorf("provided signer provider is not a fulcio signer provider") + } + + WithTokenPath(tokenPath)(&fsp) + return fsp, nil + }, + ), ) } @@ -113,6 +127,7 @@ type FulcioSignerProvider struct { OidcIssuer string OidcClientID string Token string + TokenPath string } type Option func(*FulcioSignerProvider) @@ -141,6 +156,12 @@ func WithToken(tokenOption string) Option { } } +func WithTokenPath(tokenPathOption string) Option { + return func(fsp *FulcioSignerProvider) { + fsp.TokenPath = tokenPathOption + } +} + func New(opts ...Option) FulcioSignerProvider { fsp := FulcioSignerProvider{} for _, opt := range opts { @@ -194,7 +215,7 @@ func (fsp FulcioSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer, var raw string switch { - case fsp.Token == "" && os.Getenv("GITHUB_ACTIONS") == "true": + case fsp.Token == "" && fsp.TokenPath == "" && os.Getenv("GITHUB_ACTIONS") == "true": tokenURL := os.Getenv("ACTIONS_ID_TOKEN_REQUEST_URL") if tokenURL == "" { return nil, errors.New("ACTIONS_ID_TOKEN_REQUEST_URL is not set") @@ -209,10 +230,18 @@ func (fsp FulcioSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer, if err != nil { return nil, err } - - case fsp.Token != "": + // we want to fail if both flags used (they're mutually exclusive) + case fsp.TokenPath != "" && fsp.Token != "": + return nil, errors.New("only one of --fulcio-token-path or --fulcio-raw-token can be used") + case fsp.Token != "" && fsp.TokenPath == "": raw = fsp.Token + case fsp.TokenPath != "" && fsp.Token == "": + f, err := os.ReadFile(fsp.TokenPath) + if err != nil { + return nil, fmt.Errorf("failed to read fulcio token from filepath %s: %w", fsp.TokenPath, err) + } + raw = string(f) case fsp.Token == "" && isatty.IsTerminal(os.Stdin.Fd()): tok, err := oauthflow.OIDConnect(fsp.OidcIssuer, fsp.OidcClientID, "", "", oauthflow.DefaultIDTokenGetter) if err != nil { @@ -281,7 +310,7 @@ func (fsp FulcioSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer, func getCert(ctx context.Context, key *rsa.PrivateKey, fc fulciopb.CAClient, token string) (*fulciopb.SigningCertificate, error) { t, err := jwt.ParseSigned(token) if err != nil { - return nil, err + return nil, fmt.Errorf("Failed to parse jwt token for fulcio: %w", err) } var claims struct { diff --git a/signer/fulcio/fulcio_test.go b/signer/fulcio/fulcio_test.go index 1a062821..8f5a3dae 100644 --- a/signer/fulcio/fulcio_test.go +++ b/signer/fulcio/fulcio_test.go @@ -24,6 +24,7 @@ import ( "fmt" "log" "net" + "os" "strings" "testing" "time" @@ -33,6 +34,7 @@ import ( fulciopb "github.com/sigstore/fulcio/pkg/generated/protobuf" "github.com/stretchr/testify/require" "go.step.sm/crypto/jose" + "path/filepath" "google.golang.org/grpc" "gopkg.in/square/go-jose.v2/jwt" @@ -198,6 +200,25 @@ func TestSigner(t *testing.T) { _, err = provider.Signer(ctx) //this should be a tranport err since we cant actually test on 443 which is the default require.ErrorContains(t, err, "lookup test") + + // Test signer with token read from file + // NOTE: this function could be refactored to accept a fileSystem or io.Reader so reading the file can be mocked, + // but unsure if this is the way we want to go for now + wd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get working directory: %v", err) + } + rootDir := filepath.Dir(filepath.Dir(wd)) + tp := filepath.Join(rootDir, "hack", "test.token") + + provider = New(WithFulcioURL(fmt.Sprintf("http://%v:%v", hostname, port)), WithTokenPath(tp)) + _, err = provider.Signer(ctx) + require.NoError(t, err) + + // Test signer with both token read from file and raw token + provider = New(WithFulcioURL(fmt.Sprintf("http://%v:%v", hostname, port)), WithTokenPath(tp), WithToken(token)) + _, err = provider.Signer(ctx) + require.ErrorContains(t, err, "only one of --fulcio-token-path or --fulcio-raw-token can be used") } func generateCertChain(t *testing.T) []string {