Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: VerifyNpmPackage API with supplied tuf client #768

Open
wants to merge 49 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
e4c034a
refactor: allow passing in an optional SigstoreTufClient
ramonpetgrave64 May 8, 2024
1f04dce
add Test_VerifyNpmPackage
ramonpetgrave64 May 8, 2024
7d7448b
lint
ramonpetgrave64 May 8, 2024
6c69c5c
use [email protected]
ramonpetgrave64 May 10, 2024
d5d6f6e
make it a new function in the interface
ramonpetgrave64 May 13, 2024
88ba4da
disbale parallel test for runVerify...
ramonpetgrave64 May 13, 2024
ef7384f
feat: workflow to update actions dist (#760)
ramonpetgrave64 May 6, 2024
4da9d12
fix(deps): update dependency @actions/core to v1.10.1 (#717)
renovate-bot May 7, 2024
e208b4a
document the new feature
ramonpetgrave64 May 13, 2024
3fd185e
cleanup
ramonpetgrave64 May 13, 2024
f414666
undo
ramonpetgrave64 May 13, 2024
0356825
add io readfer for attestations
ramonpetgrave64 May 13, 2024
9d0f2b2
add specific test
ramonpetgrave64 May 13, 2024
bec1ad9
typo
ramonpetgrave64 May 13, 2024
96ea870
make attestations an io reader
ramonpetgrave64 May 13, 2024
91173c1
Revert "make attestations an io reader"
ramonpetgrave64 May 17, 2024
aad1c11
builderId to builderID
ramonpetgrave64 May 17, 2024
0629763
no ioreader, better example in docs
ramonpetgrave64 May 17, 2024
f41e1fd
copy testdata
ramonpetgrave64 May 17, 2024
3803a16
chore: fix pr-title-checker (#770)
ianlewis May 15, 2024
efa3fa5
chore: Update Renovate config (#769)
ianlewis May 15, 2024
3ccbce7
readme comments
ramonpetgrave64 May 21, 2024
7a8a94d
fix docstring
ramonpetgrave64 May 21, 2024
e53b528
tuf to TUF initialism
ramonpetgrave64 May 21, 2024
ada0207
add example about utility method for making a client
ramonpetgrave64 May 21, 2024
a2cb9b9
typo
ramonpetgrave64 May 21, 2024
e2dcffc
undo WithSigstoreTUFClient
ramonpetgrave64 Jun 6, 2024
a0c70c8
use VerifierOpts
ramonpetgrave64 Jun 6, 2024
57b3797
cache the verifierOpts
ramonpetgrave64 Jun 6, 2024
c9318ae
use variadic for VefifierOpts
ramonpetgrave64 Jun 10, 2024
f1ee89d
remove NewSigstoreTufClient()
ramonpetgrave64 Jun 10, 2024
5b1c921
better docs for VerifierOpts
ramonpetgrave64 Jun 10, 2024
cde7688
update docs
ramonpetgrave64 Jun 10, 2024
667215a
fix: use pr_number as env variable (#771)
ramonpetgrave64 May 22, 2024
1bd8955
fix: signoff commit (#767)
ramonpetgrave64 May 22, 2024
3a09aac
go mods from main
ramonpetgrave64 Jun 10, 2024
5845e1c
lint
ramonpetgrave64 Jun 10, 2024
5356a7c
Merge branch 'main' into ramonpetgrave64-lib-api
ramonpetgrave64 Jun 10, 2024
65dce19
make GetDefaultSigstoreTUFClient() public
ramonpetgrave64 Jun 11, 2024
8e038e4
cleanup test
ramonpetgrave64 Jun 11, 2024
7268490
update docs
ramonpetgrave64 Jul 1, 2024
6cf042a
use sync.Do instead of atomic.Value
ramonpetgrave64 Jul 1, 2024
4bdc88a
Merge branch 'main' into ramonpetgrave64-lib-api
ramonpetgrave64 Jul 1, 2024
b4381c5
enable print-provenance
ramonpetgrave64 Jul 2, 2024
772f659
golang to Go
ramonpetgrave64 Aug 7, 2024
4ca058b
tabs to spaces
ramonpetgrave64 Aug 7, 2024
8ba939a
Merge branch 'main' into ramonpetgrave64-lib-api
ramonpetgrave64 Aug 7, 2024
a1d45cc
change to ClientOpts, check for more than 1
ramonpetgrave64 Aug 27, 2024
74bf48e
Merge branch 'main' into ramonpetgrave64-lib-api
ramonpetgrave64 Aug 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions cli/slsa-verifier/main_regression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1775,8 +1775,6 @@ func Test_runVerifyNpmPackage(t *testing.T) {
for _, tt := range tests {
tt := tt // Re-initializing variable so it is not changed while executing the closure below
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

artifactPath := filepath.Clean(filepath.Join(TEST_DIR, "npm", "gha", tt.artifact))
attestationsPath := fmt.Sprintf("%s.json", artifactPath)
cmd := verify.VerifyNpmPackageCommand{
Expand Down
77 changes: 77 additions & 0 deletions docs/API-Library.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# Api Library

## Verifers

We have exported functions for using slsa-verifier within your own golang packages
ramonpetgrave64 marked this conversation as resolved.
Show resolved Hide resolved

- slsa-verifier/verifiers/verifier.go

### Npmjs

With `VerifyNpmPackageWithSigstoreTUFClient`, you can pass in your own TUF client with custom options.
Copy link
Contributor

@laurentsimon laurentsimon May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you want to hack on the existing API or create a cleaner / new set of APIs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't call it a hack. But your other comment got me thinking I could expose the Npm struct externally, or make a new NpmVerifier struct. And we could still use that TUFClient to let the user pass in their own TUF root.

type NpmVerifier struct {
	Ctx              context.Context
	AttestationBytes []byte
	PovenanceOpts    *options.ProvenanceOpts
	BuilderOpts      *options.BuilderOpts
	VerifierOpts     *options.NpmVerifierOpts // hypothetical new type

}

type VerifierOpts struct { // hypothetical new type
	Logger  *log.Logger
	offline bool // whether to force offline verification
}

type NpmVerifierOpts struct { // hypothetical new type
	*VerifierOpts
	TUFClient *utils.TUFClient
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another question following from Laurent's - how much leeway do we have to change this API? Does it have many users currently? Would it be possible to make a new release with a breaking change? (Not saying we necessarily have to do that just wondering what the constraints are here).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could have lots of leeway. Personally, I'd rather we get this into v2, and then later on add the breaking changes in a new v3.

For example, use the embedded TUF root with `sigstoreTUF.DefaultOptions().WithForceCache()`.

Example:

```go
package main

import (
"context"
"fmt"
"log"
"os"

sigstoreTUF "github.com/sigstore/sigstore-go/pkg/tuf"
options "github.com/slsa-framework/slsa-verifier/v2/options"
apiVerify "github.com/slsa-framework/slsa-verifier/v2/verifiers"
apiUtils "github.com/slsa-framework/slsa-verifier/v2/verifiers/utils"
)

func main() {
builderID, err := doVerify()
if err != nil {
log.Fatalf("Verifying npm package: FAILED: %w", err)
}
fmt.Printf("builderID: %s\nVerifying npm package: PASSED", builderID.Name())
}

func doVerify() (*apiUtils.TrustedBuilderID, error) {
packageVersion := "0.1.127"
packageName := "@ianlewis/actions-test"
builderID := "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_nodejs_slsa3.yml"
attestations, err := os.ReadFile("attestations.json")
if err != nil {
return nil, fmt.Errorf("reading attestations file: %w", err)
}
tarballHash := "ab786dbef723164a605e55ff0ebe83f8e879159bd411980d4423c9b1646b858a537b4bc4d494fc8f71195db715e5c5e9ab4b8809f8b1b399cd30ac053d180ba7"
provenanceOpts := &options.ProvenanceOpts{
ExpectedSourceURI: "github.com/ianlewis/actions-test",
ExpectedDigest: "ab786dbef723164a605e55ff0ebe83f8e879159bd411980d4423c9b1646b858a537b4bc4d494fc8f71195db715e5c5e9ab4b8809f8b1b399cd30ac053d180ba7",
ExpectedPackageName: &packageName,
ExpectedPackageVersion: &packageVersion,
}
builderOpts := &options.BuilderOpts{
ExpectedID: &builderID,
}
// example: force using the embedded root, without going online for a refresh
// opts := sigstoreTUF.DefaultOptions().WithForceCache()
// example: supply your own root
// opts := sigstoreTUF.DefaultOptions().WithRoot([]byte(`{"signed":{"_type":"root","spec_version":"1.0","version":9,"expires":"2024-09-12T06:53:10Z","keys":{"1e1d65ce98b10 ...`)).WithForceCache()
// example: use our uility method for making a client
// client, err := apiUtils.NewSigstoreTUFClient()
opts := sigstoreTUF.DefaultOptions()
client, err := sigstoreTUF.New(opts)
if err != nil {
return nil, fmt.Errorf("creating SigstoreTUF client: %w", err)
}
verifierOptioners := []options.VerifierOptioner{
options.WithSigstoreTUFClient(client),
}
_, outBuilderID, err := apiVerify.VerifyNpmPackageWithSigstoreTUFClient(context.Background(), attestations, tarballHash, provenanceOpts, builderOpts, verifierOptioners...)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slugclub small change in the interface. We're passing in some variadic arguments, instead of passing in the the client directly.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to using options to set the TUF client. Is there a reason to pass this in as a variadic options.VerifierOptioner rather than something like a *options.VerifierOpts like what we have on lines 332 and 333 for provenanceOpts/builderOpts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only so that we don't break the current API

ramonpetgrave64 marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, fmt.Errorf("Verifying npm package: FAILED: %w", err)
}
return outBuilderID, nil
}
```
1 change: 1 addition & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ var (
ErrorInvalidHash = errors.New("invalid hash")
ErrorNotPresent = errors.New("not present")
ErrorInvalidPublicKey = errors.New("invalid public key")
ErrorCouldNotFindTarget = errors.New("could not get the target from the tuf root")
)
22 changes: 22 additions & 0 deletions options/options.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package options

import (
apiUtils "github.com/slsa-framework/slsa-verifier/v2/verifiers/utils"
)

// ProvenanceOpts are the options for checking provenance information.
type ProvenanceOpts struct {
// ExpectedBranch is the expected branch (github_ref or github_base_ref) in
Expand Down Expand Up @@ -37,3 +41,21 @@ type BuilderOpts struct {
// ExpectedBuilderID is the builderID passed in from the user to be verified
ExpectedID *string
}

// VerifierOpts are the options for the verifier, created with the VerifierOptioner functions.
// In the future, this can include a logger and a rekor client,
// or be a standalone argument to the verifier functions.
type VerifierOpts struct {
// SigstoreTufClient is the Sigstore TUF client, used for retrieving the Npmjs public keys
SigstoreTUFClient apiUtils.SigstoreTUFClient
}

// VerifierOptioner is a function that sets options for the verifier.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason to use this approach for creating options as opposed to something like:

verifierOpts := &options.VerifierOpts{
  SigstoreTUFClient: client,
  ... // Anything else that needs to be set up
}

in the client (and then potentially a DefaultVerifierOptions func if needed - see comment below)? I think that would be more consistent with builderOpts/provenanceOpts and clearer for clients to understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it would be more clear to understand, but my reason is so that we won't me merging in a breaking change at this time.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using variadic seems OK if we need to not make a breaking change to the api. In terms of using an Optioner vs something like options.VerifierOpts, using the latter might be better to keep consistent with the way we provide the other options (possibly including a DefaultVerifierOptions function to make it easy for clients to get the standard set of options). Or are there pros for using an Optioner that I'm missing? (I'm not very familiar with that pattern sorry)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also new to the pattern, but I see it's used a bit in sigstore-go for making optional arguments. (I prefer to call the returned func an "optioner", instead of an "option".)

I could change it to accept a variadic options.VerifierOpts, but then I'd need code to check that the receiver has no more than one of options.VerifierOpts. This seems problematic. And If you recall, that was my original approach, but now it seems we could agree the variadic option funcs seem to be a good compromise to making a whole new VerifyNpmPackageWithSigstoreTUFClient.

func (v *GCBVerifier) VerifyNpmPackage(ctx context.Context,
	attestations []byte, tarballHash string,
	provenanceOpts *options.ProvenanceOpts,
	builderOpts *options.BuilderOpts,
	verifierOpts ...options.VerifierOpts,
) ([]byte, *utils.TrustedBuilderID, error) {
    if len(verifierOpts) > 1 {
        panic("There should be at most one instance of verifierOpts supplied.")
    }
    ...
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah I agree that checking there's at most one options.VerifierOpts isn't the nicest.

Where I'm getting stuck is that ideally the API function and its arguments would provide a "well-lit path" to users i.e. the function signature would make it easy for clients to:
a) understand what the function is doing
b) call the function correctly/with their desired arguments.

I think right now it's more complex than it needs to be. There's three different sets of options you have to pass in and the options themselves are specified two different ways. I know that's a by-product of hacking on the existing API (which I don't believe was necessarily designed to be part of an external library).

I don't want to block this change from merging - I know it's been in the works for a while now and I definitely think this restructuring to allow a user to pass in a TUF client is a big improvement. Maybe for the future it be good to either:

  • Introduce an additional function with a simpler signature (I'm curious to hear Laurent explain why that might be a bad idea)
  • Make a breaking change to this API

We should also make sure the example documentation is super clear.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I agree with @loosebazooka that if we need to be strict about API breakages, the best option would be to accept a variadic VerifierOpts and check for 0 or 1 at runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slugclub Thanks! I've added it and renamed it to ClientOpts, because now we have a separate VerificationOpts. Although, I could fold it into VerificationOpts, instead. That might make more sense. wdyt @loosebazooka ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sounds good. Thanks everyone for the fruitful discussion here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just making sure: you'd prefer it folded into ClientOpts folded into VerificationOpts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel super strongly about this, but I think these configurations as separate structs seems reasonable, as we can grow ClientOpts to include configuration for other service dependencies of slsa-verifier while VerificationOpts is focused on cryptographic verification options.

type VerifierOptioner func(opt *VerifierOpts)

// WithSigstoreTUFClient sets the Sigstore TUF client for the verifier.
func WithSigstoreTUFClient(sigstoreTUFClient apiUtils.SigstoreTUFClient) VerifierOptioner {
return func(opt *VerifierOpts) {
opt.SigstoreTUFClient = sigstoreTUFClient
}
}
1 change: 1 addition & 0 deletions register/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type SLSAVerifier interface {
attestations []byte, tarballHash string,
provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
verifierOptioners ...options.VerifierOptioner,
) ([]byte, *utils.TrustedBuilderID, error)
}

Expand Down
1 change: 1 addition & 0 deletions verifiers/internal/gcb/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func (v *GCBVerifier) VerifyNpmPackage(ctx context.Context,
attestations []byte, tarballHash string,
provenanceOpts *options.ProvenanceOpts,
builderOpts *options.BuilderOpts,
verifierOptioners ...options.VerifierOptioner,
) ([]byte, *utils.TrustedBuilderID, error) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this func is following from VerifyNpmPackage but I wonder if it should return the data in a more structured manner to make it easier for callers to inspect the fields they're interested in. At the moment, you'd have to parse the attestation yourself to get information from it (except builder id which is helpfully extracted). That's quite involved because the structure of the attestation is complex.

I think this is beyond the scope of what this PR is trying to do, so don't worry about it for these changes but maybe something to think about for the future? Even just having slsa-verifier expose some of its functions for parsing an attestation could work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the returned []byte, I think that's what StatementsFromBytes is for, but its not yet implemented for npm.

@laurentsimon, @ianlewis , regarding #493, is it now safe to return the entire provenance, once verified?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so.

if it should return the data in a more structured manner

@ramonpetgrave64 That's maybe what we discussed a few weeks ago, to have a layered approach to verification where the inner layer returns a structured "condense" / summary for callers to use.

Copy link
Contributor Author

@ramonpetgrave64 ramonpetgrave64 Jul 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added printing in a new commit

return nil, nil, serrors.ErrorNotSupported
}
Expand Down
52 changes: 47 additions & 5 deletions verifiers/internal/gha/npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

intoto "github.com/in-toto/in-toto-golang/in_toto"
"github.com/secure-systems-lab/go-securesystemslib/dsse"
sigstoreTUF "github.com/sigstore/sigstore-go/pkg/tuf"
serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
"github.com/slsa-framework/slsa-verifier/v2/options"
"github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance"
Expand All @@ -32,6 +33,9 @@ const (
var errrorInvalidAttestations = errors.New("invalid npm attestations")
var attestationKeyAtomicValue atomic.Value

// cache the default Sigstore TUF client.
var defaultSigstoreTUFClientAtomicValue atomic.Value

type attestationSet struct {
Attestations []attestation `json:"attestations"`
}
Expand All @@ -56,6 +60,7 @@ type Npm struct {
verifiedPublishAtt *SignedAttestation
provenanceAttestation *attestation
publishAttestation *attestation
verifierOpts *options.VerifierOpts
}

func (n *Npm) ProvenanceEnvelope() *dsse.Envelope {
Expand All @@ -66,25 +71,62 @@ func (n *Npm) ProvenanceLeafCertificate() *x509.Certificate {
return n.verifiedProvenanceAtt.SigningCert
}

func NpmNew(ctx context.Context, root *TrustedRoot, attestationBytes []byte) (*Npm, error) {
// NpmNew creates a new Npm verifier.
func NpmNew(ctx context.Context, root *TrustedRoot, attestationBytes []byte, verifierOptioners ...options.VerifierOptioner) (*Npm, error) {
var aSet attestationSet
if err := json.Unmarshal(attestationBytes, &aSet); err != nil {
return nil, fmt.Errorf("%w: json.Unmarshal: %v", errrorInvalidAttestations, err)
}

prov, pub, err := extractAttestations(aSet.Attestations)
if err != nil {
return nil, err
}
verifierOpts, err := getVerifierOpts(verifierOptioners...)
if err != nil {
return nil, err
}
return &Npm{
ctx: ctx,
root: root,

provenanceAttestation: prov,
publishAttestation: pub,
verifierOpts: verifierOpts,
}, nil
}

// getDefaultSigstoreTUFClient returns the default Sigstore TUF client.
ramonpetgrave64 marked this conversation as resolved.
Show resolved Hide resolved
func getDefaultSigstoreTUFClient() (utils.SigstoreTUFClient, error) {
value := defaultSigstoreTUFClientAtomicValue.Load()
if value != nil {
return value.(utils.SigstoreTUFClient), nil
}
sigstoreTUFClient, err := sigstoreTUF.DefaultClient()
if err != nil {
return nil, err
}
defaultSigstoreTUFClientAtomicValue.Store(sigstoreTUFClient)
return sigstoreTUFClient, nil
}

// getVerifierOpts returns the verifier options, adding missing options with default values.
func getVerifierOpts(verifierOptioners ...options.VerifierOptioner) (*options.VerifierOpts, error) {
// Set the verifier options.
verifierOpts := &options.VerifierOpts{}
for _, optioner := range verifierOptioners {
optioner(verifierOpts)
}
// Set the Sigstore TUF client, if not set.
if verifierOpts.SigstoreTUFClient == nil {
sigstoreTUFClient, err := getDefaultSigstoreTUFClient()
if err != nil {
return nil, err
}
verifierOpts.SigstoreTUFClient = sigstoreTUFClient
}
return verifierOpts, nil
}

func extractAttestations(attestations []attestation) (*attestation, *attestation, error) {
if len(attestations) < 2 {
return nil, nil, fmt.Errorf("%w: invalid number of attestations: %v", errrorInvalidAttestations, len(attestations))
Expand All @@ -111,12 +153,12 @@ func extractAttestations(attestations []attestation) (*attestation, *attestation
}

// getAttestationKey retrieves the attestation key and holds it in memory.
func getAttestationKey(npmRegistryPublicKeyID string) (string, error) {
func getAttestationKey(sigstoreTUFClient utils.SigstoreTUFClient, npmRegistryPublicKeyID string) (string, error) {
value := attestationKeyAtomicValue.Load()
if value != nil {
return value.(string), nil
}
npmRegistryPublicKey, err := getKeyDataFromSigstoreTuf(npmRegistryPublicKeyID, attestationKeyUsage)
npmRegistryPublicKey, err := getKeyDataFromSigstoreTUF(sigstoreTUFClient, npmRegistryPublicKeyID, attestationKeyUsage)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -147,7 +189,7 @@ func (n *Npm) verifyPublishAttestationSignature() error {

// Retrieve the key material.
// We found the associated public key in the TUF root, so now we can trust this KeyID.
npmRegistryPublicKey, err := getAttestationKey(npmRegistryPublicKeyID)
npmRegistryPublicKey, err := getAttestationKey(n.verifierOpts.SigstoreTUFClient, npmRegistryPublicKeyID)
if err != nil {
return err
}
Expand Down
35 changes: 9 additions & 26 deletions verifiers/internal/gha/npm_sigstore_tuf.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (
"fmt"
"time"

sigstoreTuf "github.com/sigstore/sigstore-go/pkg/tuf"
serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
"github.com/slsa-framework/slsa-verifier/v2/verifiers/utils"
)

const (
Expand All @@ -16,7 +17,6 @@ const (

var (
errorMissingNpmjsKeyIDKeyUsage = errors.New("could not find a key with the specified 'keyId' and 'keyUsage'")
errorCouldNotFindTarget = errors.New("could not get the target from the tuf root")
errorCouldNotParseKeys = errors.New("could not parse keys file content")
)

Expand All @@ -38,27 +38,13 @@ type validFor struct {
Start time.Time `json:"start"`
}

type sigstoreTufClient interface {
GetTarget(target string) ([]byte, error)
}

// newSigstoreTufClient gets a Sigstore TUF client, which itself is a wrapper around the official TUF client.
func newSigstoreTufClient() (*sigstoreTuf.Client, error) {
opts := sigstoreTuf.DefaultOptions()
client, err := sigstoreTuf.New(opts)
if err != nil {
return nil, fmt.Errorf("creating SigstoreTuf client: %w", err)
}
return client, nil
}

// getNpmjsKeysTarget will fetch and parse the keys.json file in Sigstore's root for npmjs
// The inner TUF client will verify this "blob" is signed with correct delegate TUF roles
// https://github.com/sigstore/root-signing/blob/5fd11f7ec0a993b0f20c335b33e53cfffb986b2e/repository/repository/targets/registry.npmjs.org/7a8ec9678ad824cdccaa7a6dc0961caf8f8df61bc7274189122c123446248426.keys.json#L4
func getNpmjsKeysTarget(client sigstoreTufClient, targetPath string) (*npmjsKeysTarget, error) {
func getNpmjsKeysTarget(client utils.SigstoreTUFClient, targetPath string) (*npmjsKeysTarget, error) {
blob, err := client.GetTarget(targetPath)
if err != nil {
return nil, fmt.Errorf("%w: %w", errorCouldNotFindTarget, err)
return nil, fmt.Errorf("%w: %w", serrors.ErrorCouldNotFindTarget, err)
}
var keys npmjsKeysTarget
if err := json.Unmarshal(blob, &keys); err != nil {
Expand All @@ -79,18 +65,15 @@ func getKeyDataWithNpmjsKeysTarget(keys *npmjsKeysTarget, keyID, keyUsage string
return "", fmt.Errorf("%w: 'keyId': %q, 'keyUsage':%q", errorMissingNpmjsKeyIDKeyUsage, keyID, keyUsage)
}

// getKeyDataFromSigstoreTuf retrieves the keyfile from sigstore's TUF root, parses the file and returns the target key's material.
// getKeyDataFromSigstoreTUF retrieves the keyfile from sigstore's TUF root, parses the file and returns the target key's material.
// See documentation for getNpmjsKeysTarget
//
// example params:
//
// keyID: "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA"
// keyUsage: "npm:attestations"
func getKeyDataFromSigstoreTuf(keyID, keyUsage string) (string, error) {
client, err := newSigstoreTufClient()
if err != nil {
return "", err
}
// client: sigstoreTUFClient
// keyID: "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA"
// keyUsage: "npm:attestations"
func getKeyDataFromSigstoreTUF(client utils.SigstoreTUFClient, keyID, keyUsage string) (string, error) {
keys, err := getNpmjsKeysTarget(client, targetPath)
if err != nil {
return "", err
Expand Down
Loading
Loading