Skip to content

Commit

Permalink
Enable more govet checks and address issues
Browse files Browse the repository at this point in the history
It turns out that the 'govet' linter has a few more tricks up its
sleeve, you just need to enable them.

This find a couple of bugs in the tests which are also being fixed in
this commit:

1. The spire-server tests for BatchCreateFederatedBundle and friends
   were accidentally not including JWT keys in the bundle they were
   testing. This ended up only affecting assertions on log message
   fields, which are being fixed here.
   The fix for this engendered a bit of refactoring to enable access to
   the required JWT struct conversion function.
2. The spire-server tests for the CA journal were _almost_ failing in
   their attempt to list CA journals; it ended up working anyway because
   a conversion between different struct types happened to be
   unnecessary because gorm could work with either one due to matching
   struct field names.

Signed-off-by: Carlo Teubner <carlo@cteubner.net>
  • Loading branch information
c4rlo committed Dec 20, 2024
1 parent 6928ff1 commit 6aff862
Show file tree
Hide file tree
Showing 6 changed files with 312 additions and 222 deletions.
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ linters:
- nolintlint

linters-settings:
govet:
enable:
- nilness
- sortslice
- unusedwrite
revive:
# minimal confidence for issues, default is 0.8
confidence: 0.0
Expand Down
18 changes: 2 additions & 16 deletions cmd/spire-server/cli/bundle/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package bundle

import (
"bytes"
"crypto"
"crypto/x509"
"encoding/json"
"encoding/pem"
Expand All @@ -17,6 +16,7 @@ import (
"github.com/spiffe/go-spiffe/v2/spiffeid"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
"github.com/spiffe/spire/cmd/spire-server/util"
"github.com/spiffe/spire/pkg/common/jwtutil"
"github.com/zeebo/errs"
)

Expand Down Expand Up @@ -103,7 +103,7 @@ func bundleFromProto(bundleProto *types.Bundle) (*spiffebundle.Bundle, error) {
if err != nil {
return nil, err
}
jwtAuthorities, err := jwtKeysFromProto(bundleProto.JwtAuthorities)
jwtAuthorities, err := jwtutil.JWTKeysFromProto(bundleProto.JwtAuthorities)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -132,20 +132,6 @@ func x509CertificatesFromProto(proto []*types.X509Certificate) ([]*x509.Certific
return certs, nil
}

// jwtKeysFromProto converts JWT keys from the given []*types.JWTKey to map[string]crypto.PublicKey.
// The key ID of the public key is used as the key in the returned map.
func jwtKeysFromProto(proto []*types.JWTKey) (map[string]crypto.PublicKey, error) {
keys := make(map[string]crypto.PublicKey)
for i, publicKey := range proto {
jwtSigningKey, err := x509.ParsePKIXPublicKey(publicKey.PublicKey)
if err != nil {
return nil, fmt.Errorf("unable to parse JWT signing key %d: %w", i, err)
}
keys[publicKey.KeyId] = jwtSigningKey
}
return keys, nil
}

func printBundleWithFormat(out io.Writer, bundle *types.Bundle, format string, header bool) error {
if bundle == nil {
return errors.New("no bundle provided")
Expand Down
22 changes: 2 additions & 20 deletions cmd/spire-server/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package util

import (
"context"
"crypto"
"crypto/x509"
"flag"
"fmt"
Expand All @@ -20,6 +19,7 @@ import (
trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1"
api_types "github.com/spiffe/spire-api-sdk/proto/spire/api/types"
common_cli "github.com/spiffe/spire/pkg/common/cli"
"github.com/spiffe/spire/pkg/common/jwtutil"
"github.com/spiffe/spire/pkg/common/pemutil"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -252,7 +252,7 @@ func protoFromSpiffeBundle(bundle *spiffebundle.Bundle) (*api_types.Bundle, erro
X509Authorities: protoFromX509Certificates(bundle.X509Authorities()),
}

jwtAuthorities, err := protoFromJWTKeys(bundle.JWTAuthorities())
jwtAuthorities, err := jwtutil.ProtoFromJWTKeys(bundle.JWTAuthorities())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -280,21 +280,3 @@ func protoFromX509Certificates(certs []*x509.Certificate) []*api_types.X509Certi

return resp
}

// protoFromJWTKeys converts JWT keys from the given map[string]crypto.PublicKey to []*types.JWTKey
func protoFromJWTKeys(keys map[string]crypto.PublicKey) ([]*api_types.JWTKey, error) {
var resp []*api_types.JWTKey

for kid, key := range keys {
pkixBytes, err := x509.MarshalPKIXPublicKey(key)
if err != nil {
return nil, err
}
resp = append(resp, &api_types.JWTKey{
PublicKey: pkixBytes,
KeyId: kid,
})
}

return resp, nil
}
41 changes: 41 additions & 0 deletions pkg/common/jwtutil/jwt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package jwtutil

import (
"crypto"
"crypto/x509"
"fmt"

"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
)

// JWTKeysFromProto converts JWT keys from the given []*types.JWTKey to map[string]crypto.PublicKey.
// The key ID of the public key is used as the key in the returned map.
func JWTKeysFromProto(proto []*types.JWTKey) (map[string]crypto.PublicKey, error) {
keys := make(map[string]crypto.PublicKey)
for i, publicKey := range proto {
jwtSigningKey, err := x509.ParsePKIXPublicKey(publicKey.PublicKey)
if err != nil {
return nil, fmt.Errorf("unable to parse JWT signing key %d: %w", i, err)
}
keys[publicKey.KeyId] = jwtSigningKey
}
return keys, nil
}

// ProtoFromJWTKeys converts JWT keys from the given map[string]crypto.PublicKey to []*types.JWTKey
func ProtoFromJWTKeys(keys map[string]crypto.PublicKey) ([]*types.JWTKey, error) {
var resp []*types.JWTKey

for kid, key := range keys {
pkixBytes, err := x509.MarshalPKIXPublicKey(key)
if err != nil {
return nil, err
}
resp = append(resp, &types.JWTKey{
PublicKey: pkixBytes,
KeyId: kid,
})
}

return resp, nil
}
Loading

0 comments on commit 6aff862

Please sign in to comment.