Add WIT bundle functionality

[jsnctl]

implements #365

Opening draft PR ahead of merge of upstream spiffe/spiffe#327 (per discussion here)

[strideynet]

The constraint that a keypair must have a kid is interesting - I have raised this for discussion on spiffe/spiffe#327 (comment)

Today, the JWT-SVID/WIT-SVID profiles of the Workload API do not make the kid member of the JWKS mandatory - but it looks like go-spiffe treats it as such. I do think that the behaviour of go-spiffe here is reasonable, but, we should consider at least modifying the WIP WIT-SVID profile to express that the kid member is mandatory.

[jsnctl]

Looking at the existing jwtsvid implementation, it also concretely relies on the kid in the FindJWTAuthority call for verification (the corresponding witsvid changes will be in a separate merge)

go-spiffe/svid/jwtsvid/svid.go

Lines 53 to 69 in c03307b

	// Obtain the key ID from the header
	keyID := tok.Headers[0].KeyID
	if keyID == "" {
	return nil, wrapJwtsvidErr(errors.New("token header missing key id"))
	}
	// Get JWT Bundle
	bundle, err := bundles.GetJWTBundleForTrustDomain(trustDomain)
	if err != nil {
	return nil, wrapJwtsvidErr(fmt.Errorf("no bundle found for trust domain %q", trustDomain))
	}
	// Find JWT authority using the key ID from the token header
	authority, ok := bundle.FindJWTAuthority(keyID)
	if !ok {
	return nil, wrapJwtsvidErr(fmt.Errorf("no JWT authority %q found for trust domain %q", keyID, trustDomain))
	}

Presumably JWKS usage without a kid in each authority would just require iterating over all available JWKs, so this seems like a nice pattern to maintain here.

As for the spec, you can imagine (less tidy) downstream implementations which use valid JWKS without the kid? Perhaps the opinion of go-spiffe here isn't the only valid consideration?

[strideynet]

Perhaps the opinion of go-spiffe here isn't the only valid consideration?

Perhaps so - although it definitely carries a fair bit of weight in terms of the "real-world" usage of the specification. Today, if a Workload API implementation does not provide the kid member of the trust bundle JWKS, then it looks like go-spiffe/jwtbundle would fail to unmarshal it. Fortunately, I think all "real-world" implementations of the SPIFFE specification are likely to include the kid member.

However, as we're introducing a new profile for WIT, it does seem like a good opportunity to make the spec a little more restrictive if it means that all proper implementations of the spec will be compatible with the SDKs. At the moment the gray area where an implementation could be spec-compliant but not work with go-spiffe seems a little problematic.

[arndt-s]

I started a review last week but got distracted. Publishing my comments so far.

[arndt-s]

nit: you could use Read() here and supply the file as io.Reader.

[jsnctl]

I agree this would be a nicer implementation - do we want to keep things consistent with the methods in the other *bundle implementations though?

go-spiffe/bundle/jwtbundle/bundle.go

Lines 41 to 49 in c03307b

	// Load loads a bundle from a file on disk. The file must contain a standard RFC 7517 JWKS document.
	func Load(trustDomain spiffeid.TrustDomain, path string) (*Bundle, error) {
	bundleBytes, err := os.ReadFile(path)
	if err != nil {
	return nil, wrapJwtbundleErr(fmt.Errorf("unable to read JWT bundle: %w", err))
	}
	return Parse(trustDomain, bundleBytes)
	}

go-spiffe/bundle/x509bundle/bundle.go

Lines 40 to 47 in c03307b

	func Load(trustDomain spiffeid.TrustDomain, path string) (*Bundle, error) {
	fileBytes, err := os.ReadFile(path)
	if err != nil {
	return nil, wrapX509bundleErr(fmt.Errorf("unable to load X.509 bundle file: %w", err))
	}
	return Parse(trustDomain, fileBytes)
	}

go-spiffe/bundle/spiffebundle/bundle.go

Lines 55 to 64 in c03307b

	// Load loads a bundle from a file on disk. The file must contain a JWKS
	// document following the SPIFFE Trust Domain and Bundle specification.
	func Load(trustDomain spiffeid.TrustDomain, path string) (*Bundle, error) {
	bundleBytes, err := os.ReadFile(path)
	if err != nil {
	return nil, wrapSpiffebundleErr(fmt.Errorf("unable to read SPIFFE bundle: %w", err))
	}
	return Parse(trustDomain, bundleBytes)
	}

[strideynet]

I think consistency is likely more important here - that being said - it'd be nice if we did extend the API of all the profiles to include LoadFromReader or similar? and perhaps refactor Load to leverage that under the hood.

[arndt-s]

This should ideally be a generic. Curious to get the maintainers POV on this. But maybe something for another day.

[jsnctl]

We'd originally said to hold off a merge until spiffe/spiffe#327 was merged (which it is now), do with think that's still applicable here?

[jsnctl]

Not sure of the best way to stack this, but I've also got a branch based off this diff for the corresponding witsvid changes too jsnctl#1