Skip to content

Commit

Permalink
Address gosec integer overflow complaints
Browse files Browse the repository at this point in the history
Silence these complaints only in test code.

Signed-off-by: Carlo Teubner <carlo@cteubner.net>
  • Loading branch information
c4rlo committed Jan 18, 2025
1 parent 3027748 commit 41da0f7
Show file tree
Hide file tree
Showing 33 changed files with 160 additions and 192 deletions.
6 changes: 3 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ issues:
- linters: [ staticcheck ]
path: ".*_test\\.go$"
text: "Subjects has been deprecated since Go 1\\.18.*Subjects will not include the system roots"
- linters: [ gosec ]
path: "(.*_test\\.go$)|(^test/.*)"
text: "integer overflow conversion"

linters:
enable:
Expand Down Expand Up @@ -45,6 +48,3 @@ linters-settings:
rules:
- name: unused-parameter
disabled: true
gosec:
excludes:
- G115 # "Potential integer overflow when converting between integer types"; TODO re-enable eventually
18 changes: 15 additions & 3 deletions cmd/spire-server/cli/entry/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"context"
"errors"
"flag"
"fmt"

"github.com/ccoveille/go-safecast"
"github.com/mitchellh/cli"
entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
Expand Down Expand Up @@ -175,6 +177,16 @@ func (c *createCommand) parseConfig() ([]*types.Entry, error) {
return nil, err
}

x509SvidTTL, err := safecast.ToInt32(c.x509SVIDTTL)
if err != nil {
return nil, fmt.Errorf("X509 SVID TTL: %w", err)
}

jwtSvidTTL, err := safecast.ToInt32(c.jwtSVIDTTL)
if err != nil {
return nil, fmt.Errorf("JWT SVID TTL: %w", err)
}

e := &types.Entry{
Id: c.entryID,
ParentId: parentID,
Expand All @@ -183,8 +195,8 @@ func (c *createCommand) parseConfig() ([]*types.Entry, error) {
ExpiresAt: c.entryExpiry,
DnsNames: c.dnsNames,
StoreSvid: c.storeSVID,
X509SvidTtl: int32(c.x509SVIDTTL),
JwtSvidTtl: int32(c.jwtSVIDTTL),
X509SvidTtl: x509SvidTTL,
JwtSvidTtl: jwtSvidTTL,
Hint: c.hint,
}

Expand Down Expand Up @@ -254,7 +266,7 @@ func prettyPrintCreate(env *commoncli.Env, results ...any) error {

for _, r := range failed {
env.ErrPrintf("Failed to create the following entry (code: %s, msg: %q):\n",
codes.Code(r.Status.Code),
codes.Code(safecast.MustConvert[uint32](r.Status.Code)),
r.Status.Message)
printEntry(r.Entry, env.ErrPrintf)
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/spire-server/cli/entry/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"io"
"os"

"github.com/ccoveille/go-safecast"
"github.com/mitchellh/cli"
entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1"
"github.com/spiffe/spire/cmd/spire-server/util"
Expand Down Expand Up @@ -135,7 +136,7 @@ func (c *deleteCommand) prettyPrintDelete(env *commoncli.Env, results ...any) er
for _, result := range failed {
env.ErrPrintf("Failed to delete entry with ID %s (code: %s, msg: %q)\n",
result.Id,
codes.Code(result.Status.Code),
codes.Code(safecast.MustConvert[uint32](result.Status.Code)),
result.Status.Message)
}

Expand Down
18 changes: 15 additions & 3 deletions cmd/spire-server/cli/entry/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"context"
"errors"
"flag"
"fmt"

"github.com/ccoveille/go-safecast"
"github.com/mitchellh/cli"
entryv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/entry/v1"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
Expand Down Expand Up @@ -169,15 +171,25 @@ func (c *updateCommand) parseConfig() ([]*types.Entry, error) {
return nil, err
}

x509SvidTTL, err := safecast.ToInt32(c.x509SvidTTL)
if err != nil {
return nil, fmt.Errorf("X509 SVID TTL: %w", err)
}

jwtSvidTTL, err := safecast.ToInt32(c.jwtSvidTTL)
if err != nil {
return nil, fmt.Errorf("JWT SVID TTL: %w", err)
}

e := &types.Entry{
Id: c.entryID,
ParentId: parentID,
SpiffeId: spiffeID,
Downstream: c.downstream,
ExpiresAt: c.entryExpiry,
DnsNames: c.dnsNames,
X509SvidTtl: int32(c.x509SvidTTL),
JwtSvidTtl: int32(c.jwtSvidTTL),
X509SvidTtl: x509SvidTTL,
JwtSvidTtl: jwtSvidTTL,
Hint: c.hint,
}

Expand Down Expand Up @@ -240,7 +252,7 @@ func prettyPrintUpdate(env *commoncli.Env, results ...any) error {
// Print entries that failed to be updated
for _, r := range failed {
env.ErrPrintf("Failed to update the following entry (code: %s, msg: %q):\n",
codes.Code(r.Status.Code),
codes.Code(safecast.MustConvert[uint32](r.Status.Code)),
r.Status.Message)
printEntry(r.Entry, env.ErrPrintf)
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/spire-server/cli/federation/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"flag"
"fmt"

"github.com/ccoveille/go-safecast"
"github.com/mitchellh/cli"
trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
Expand Down Expand Up @@ -101,7 +102,7 @@ func (c *createCommand) prettyPrintCreate(env *commoncli.Env, results ...any) er
for _, r := range failed {
env.Println()
env.ErrPrintf("Failed to create the following federation relationship (code: %s, msg: %q):\n",
codes.Code(r.Status.Code),
codes.Code(safecast.MustConvert[uint32](r.Status.Code)),
r.Status.Message)
printFederationRelationship(r.FederationRelationship, env.ErrPrintf)
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/spire-server/cli/federation/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"flag"
"fmt"

"github.com/ccoveille/go-safecast"
"github.com/mitchellh/cli"
trustdomainv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/trustdomain/v1"
"github.com/spiffe/spire-api-sdk/proto/spire/api/types"
Expand Down Expand Up @@ -97,7 +98,7 @@ func (c *updateCommand) prettyPrintUpdate(env *commoncli.Env, results ...any) er
for _, r := range failed {
env.Println()
env.ErrPrintf("Failed to update the following federation relationship (code: %s, msg: %q):\n",
codes.Code(r.Status.Code),
codes.Code(safecast.MustConvert[uint32](r.Status.Code)),
r.Status.Message)
printFederationRelationship(r.FederationRelationship, env.ErrPrintf)
}
Expand Down
11 changes: 8 additions & 3 deletions cmd/spire-server/cli/jwt/mint.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"time"

"github.com/ccoveille/go-safecast"
"github.com/go-jose/go-jose/v4/jwt"
"github.com/mitchellh/cli"
"github.com/spiffe/go-spiffe/v2/spiffeid"
Expand Down Expand Up @@ -63,14 +64,18 @@ func (c *mintCommand) Run(ctx context.Context, env *commoncli.Env, serverClient
if err != nil {
return err
}
ttl, err := ttlToSeconds(c.ttl)
if err != nil {
return fmt.Errorf("TTL: %w", err)
}

client := serverClient.NewSVIDClient()
resp, err := client.MintJWTSVID(ctx, &svidv1.MintJWTSVIDRequest{
Id: &types.SPIFFEID{
TrustDomain: spiffeID.TrustDomain().Name(),
Path: spiffeID.Path(),
},
Ttl: ttlToSeconds(c.ttl),
Ttl: ttl,
Audience: c.audience,
})
if err != nil {
Expand Down Expand Up @@ -132,8 +137,8 @@ func getJWTSVIDEndOfLife(token string) (time.Time, error) {

// ttlToSeconds returns the number of seconds in a duration, rounded up to
// the nearest second
func ttlToSeconds(ttl time.Duration) int32 {
return int32((ttl + time.Second - 1) / time.Second)
func ttlToSeconds(ttl time.Duration) (int32, error) {
return safecast.ToInt32((ttl + time.Second - 1) / time.Second)
}

func prettyPrintMint(env *commoncli.Env, results ...any) error {
Expand Down
8 changes: 7 additions & 1 deletion cmd/spire-server/cli/token/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package token
import (
"context"
"flag"
"fmt"

"github.com/ccoveille/go-safecast"
"github.com/mitchellh/cli"
"github.com/spiffe/go-spiffe/v2/spiffeid"
agentv1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/agent/v1"
Expand Down Expand Up @@ -44,11 +46,15 @@ func (g *generateCommand) Run(ctx context.Context, _ *commoncli.Env, serverClien
if err != nil {
return err
}
ttl, err := safecast.ToInt32(g.TTL)
if err != nil {
return fmt.Errorf("TTL: %w", err)
}

c := serverClient.NewAgentClient()
resp, err := c.CreateJoinToken(ctx, &agentv1.CreateJoinTokenRequest{
AgentId: id,
Ttl: int32(g.TTL),
Ttl: ttl,
})
if err != nil {
return err
Expand Down
12 changes: 9 additions & 3 deletions cmd/spire-server/cli/x509/mint.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"net/url"
"time"

"github.com/ccoveille/go-safecast"
"github.com/mitchellh/cli"
"github.com/spiffe/go-spiffe/v2/spiffeid"
bundlev1 "github.com/spiffe/spire-api-sdk/proto/spire/api/server/bundle/v1"
Expand Down Expand Up @@ -80,6 +81,11 @@ func (c *mintCommand) Run(ctx context.Context, env *commoncli.Env, serverClient
return err
}

ttl, err := ttlToSeconds(c.ttl)
if err != nil {
return fmt.Errorf("TTL: %w", err)
}

key, err := c.generateKey()
if err != nil {
return fmt.Errorf("unable to generate key: %w", err)
Expand All @@ -96,7 +102,7 @@ func (c *mintCommand) Run(ctx context.Context, env *commoncli.Env, serverClient
client := serverClient.NewSVIDClient()
resp, err := client.MintX509SVID(ctx, &svidv1.MintX509SVIDRequest{
Csr: csr,
Ttl: ttlToSeconds(c.ttl),
Ttl: ttl,
})
if err != nil {
return fmt.Errorf("unable to mint SVID: %w", err)
Expand Down Expand Up @@ -167,8 +173,8 @@ func (c *mintCommand) Run(ctx context.Context, env *commoncli.Env, serverClient

// ttlToSeconds returns the number of seconds in a duration, rounded up to
// the nearest second
func ttlToSeconds(ttl time.Duration) int32 {
return int32((ttl + time.Second - 1) / time.Second)
func ttlToSeconds(ttl time.Duration) (int32, error) {
return safecast.ToInt32((ttl + time.Second - 1) / time.Second)
}

type mintResult struct {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ require (
github.com/aws/aws-sdk-go-v2/service/sts v1.33.8
github.com/aws/smithy-go v1.22.1
github.com/blang/semver/v4 v4.0.0
github.com/ccoveille/go-safecast v1.5.0
github.com/cenkalti/backoff/v4 v4.3.0
github.com/docker/docker v27.5.0+incompatible
github.com/envoyproxy/go-control-plane/envoy v1.32.3
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,8 @@ github.com/buildkite/roko v1.2.0/go.mod h1:23R9e6nHxgedznkwwfmqZ6+0VJZJZ2Sg/uVcp
github.com/bytecodealliance/wasmtime-go/v3 v3.0.2 h1:3uZCA/BLTIu+DqCfguByNMJa2HVHpXvjfy0Dy7g6fuA=
github.com/bytecodealliance/wasmtime-go/v3 v3.0.2/go.mod h1:RnUjnIXxEJcL6BgCvNyzCCRzZcxCgsZCi+RNlvYor5Q=
github.com/cactus/go-statsd-client/v5 v5.0.0/go.mod h1:COEvJ1E+/E2L4q6QE5CkjWPi4eeDw9maJBMIuMPBZbY=
github.com/ccoveille/go-safecast v1.5.0 h1:cT/3uVQ/i5PTiJvhvkSU81HeKNurtyQtBndXEH3hDg4=
github.com/ccoveille/go-safecast v1.5.0/go.mod h1:QqwNjxQ7DAqY0C721OIO9InMk9zCwcsO7tnRuHytad8=
github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
Expand Down
29 changes: 24 additions & 5 deletions pkg/agent/api/debug/v1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package debug
import (
"context"
"crypto/x509"
"fmt"
"sync"
"time"

"github.com/ccoveille/go-safecast"
"github.com/sirupsen/logrus"
"github.com/spiffe/go-spiffe/v2/bundle/x509bundle"
"github.com/spiffe/go-spiffe/v2/spiffeid"
Expand Down Expand Up @@ -92,15 +94,32 @@ func (s *Service) GetInfo(context.Context, *debugv1.GetInfoRequest) (*debugv1.Ge
})
}

uptime, err := safecast.ToInt32(s.uptime().Seconds())
if err != nil {
return nil, fmt.Errorf("uptime: %w", err)
}
x509SvidsCount, err := safecast.ToInt32(s.m.CountX509SVIDs())
if err != nil {
return nil, fmt.Errorf("X.509 SVIDs count: %w", err)
}
jwtSvidsCount, err := safecast.ToInt32(s.m.CountJWTSVIDs())
if err != nil {
return nil, fmt.Errorf("JWT SVIDs count: %w", err)
}
svidstoreX509SvidsCount, err := safecast.ToInt32(s.m.CountSVIDStoreX509SVIDs())
if err != nil {
return nil, fmt.Errorf("SVIDStore X.509 SVIDs count: %w", err)
}

// Reset clock and set current response
s.getInfoResp.ts = s.clock.Now()
s.getInfoResp.resp = &debugv1.GetInfoResponse{
SvidChain: svidChain,
Uptime: int32(s.uptime().Seconds()),
SvidsCount: int32(s.m.CountX509SVIDs()),
CachedX509SvidsCount: int32(s.m.CountX509SVIDs()),
CachedJwtSvidsCount: int32(s.m.CountJWTSVIDs()),
CachedSvidstoreX509SvidsCount: int32(s.m.CountSVIDStoreX509SVIDs()),
Uptime: uptime,
SvidsCount: x509SvidsCount,
CachedX509SvidsCount: x509SvidsCount,
CachedJwtSvidsCount: jwtSvidsCount,
CachedSvidstoreX509SvidsCount: svidstoreX509SvidsCount,
LastSyncSuccess: s.m.GetLastSync().UTC().Unix(),
}
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/agent/plugin/keymanager/base/keymanagerbase.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"sort"
"sync"

"github.com/ccoveille/go-safecast"
keymanagerv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/plugin/agent/keymanager/v1"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -170,7 +171,7 @@ func (m *Base) signData(req *keymanagerv1.SignDataRequest) (*keymanagerv1.SignDa
if opts.HashAlgorithm == keymanagerv1.HashAlgorithm_UNSPECIFIED_HASH_ALGORITHM {
return nil, status.Error(codes.InvalidArgument, "hash algorithm is required")
}
signerOpts = crypto.Hash(opts.HashAlgorithm)
signerOpts = crypto.Hash(safecast.MustConvert[uint](opts.HashAlgorithm))
case *keymanagerv1.SignDataRequest_PssOptions:
if opts.PssOptions == nil {
return nil, status.Error(codes.InvalidArgument, "PSS options are nil")
Expand All @@ -180,7 +181,7 @@ func (m *Base) signData(req *keymanagerv1.SignDataRequest) (*keymanagerv1.SignDa
}
signerOpts = &rsa.PSSOptions{
SaltLength: int(opts.PssOptions.SaltLength),
Hash: crypto.Hash(opts.PssOptions.HashAlgorithm),
Hash: crypto.Hash(safecast.MustConvert[uint](opts.PssOptions.HashAlgorithm)),
}
default:
return nil, status.Errorf(codes.InvalidArgument, "unsupported signer opts type %T", opts)
Expand Down
5 changes: 3 additions & 2 deletions pkg/agent/plugin/keymanager/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"crypto/x509"
"io"

"github.com/ccoveille/go-safecast"
keymanagerv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/plugin/agent/keymanager/v1"
"github.com/spiffe/spire/pkg/common/plugin"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -118,7 +119,7 @@ func (v1 *V1) convertKeyType(t KeyType) (keymanagerv1.KeyType, error) {

func (v1 *V1) convertHashAlgorithm(h crypto.Hash) keymanagerv1.HashAlgorithm {
// Hash algorithm constants are aligned.
return keymanagerv1.HashAlgorithm(h)
return keymanagerv1.HashAlgorithm(safecast.MustConvert[int32](h))
}

type v1Key struct {
Expand Down Expand Up @@ -155,7 +156,7 @@ func (s *v1Key) signContext(ctx context.Context, digest []byte, opts crypto.Sign
case *rsa.PSSOptions:
req.SignerOpts = &keymanagerv1.SignDataRequest_PssOptions{
PssOptions: &keymanagerv1.SignDataRequest_PSSOptions{
SaltLength: int32(opts.SaltLength),
SaltLength: safecast.MustConvert[int32](opts.SaltLength),
HashAlgorithm: s.v1.convertHashAlgorithm(opts.Hash),
},
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/agent/plugin/workloadattestor/docker/docker_posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (h *containerHelper) getContainerID(pID int32, log hclog.Logger) (string, e
}

extractor := containerinfo.Extractor{RootDir: h.rootDir, VerboseLogging: h.verboseContainerLocatorLogs}
return extractor.GetContainerID(int(pID), log)
return extractor.GetContainerID(pID, log)
}

func getDockerHost(c *dockerPluginConfig) string {
Expand Down
Loading

0 comments on commit 41da0f7

Please sign in to comment.