Skip to content

Commit 9fbb7c9

Browse files
committed
fix: audit trustd code for security
There are no security issues fixed. Drop username/password creds - they were not used. Improve security of token interceptor. Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
1 parent 986e97f commit 9fbb7c9

6 files changed

Lines changed: 114 additions & 103 deletions

File tree

internal/app/trustd/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ func trustdMain() error {
104104
&reg.Registrator{Resources: resources},
105105
factory.WithDefaultLog(),
106106
factory.WithUnaryInterceptor(creds.UnaryInterceptor()),
107+
factory.WithStreamInterceptor(creds.StreamInterceptor()),
107108
factory.ServerOptions(
108109
grpc.Creds(
109110
credentials.NewTLS(serverTLSConfig),

pkg/grpc/middleware/auth/basic/basic.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type Credentials interface {
2323
credentials.PerRPCCredentials
2424

2525
UnaryInterceptor() grpc.UnaryServerInterceptor
26+
StreamInterceptor() grpc.StreamServerInterceptor
2627
}
2728

2829
// NewConnection initializes a grpc.ClientConn configured for basic

pkg/grpc/middleware/auth/basic/token.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ package basic
66

77
import (
88
"context"
9-
"fmt"
9+
"crypto/sha256"
10+
"crypto/subtle"
1011

1112
"google.golang.org/grpc"
1213
"google.golang.org/grpc/codes"
1314
"google.golang.org/grpc/metadata"
15+
"google.golang.org/grpc/status"
1416
)
1517

1618
// TokenGetterFunc is the function to dynamically retrieve the token.
@@ -66,13 +68,23 @@ func (b *TokenCredentials) authenticate(ctx context.Context) error {
6668
return err
6769
}
6870

69-
if md, ok := metadata.FromIncomingContext(ctx); ok {
70-
if len(md["token"]) > 0 && md["token"][0] == token {
71-
return nil
72-
}
71+
md, ok := metadata.FromIncomingContext(ctx)
72+
if !ok {
73+
return status.Error(codes.Unauthenticated, "missing token")
74+
}
75+
76+
if len(md["token"]) == 0 {
77+
return status.Error(codes.Unauthenticated, "missing token")
78+
}
79+
80+
incomingTokenHash := sha256.Sum256([]byte(md["token"][0]))
81+
expectedTokenHash := sha256.Sum256([]byte(token))
82+
83+
if subtle.ConstantTimeCompare(incomingTokenHash[:], expectedTokenHash[:]) != 1 {
84+
return status.Error(codes.Unauthenticated, "invalid token")
7385
}
7486

75-
return fmt.Errorf("%s", codes.Unauthenticated.String())
87+
return nil
7688
}
7789

7890
// UnaryInterceptor sets the UnaryServerInterceptor for the server and enforces
@@ -86,3 +98,14 @@ func (b *TokenCredentials) UnaryInterceptor() grpc.UnaryServerInterceptor {
8698
return handler(ctx, req)
8799
}
88100
}
101+
102+
// StreamInterceptor sets the StreamServerInterceptor for the server and enforces
103+
// basic authentication.
104+
//
105+
// For now, it rejects any API, as we don't have any streaming APIs in trustd component.
106+
// This is to prevent accidentally allowing unauthenticated access to streaming APIs in the future without realizing it.
107+
func (b *TokenCredentials) StreamInterceptor() grpc.StreamServerInterceptor {
108+
return func(any, grpc.ServerStream, *grpc.StreamServerInfo, grpc.StreamHandler) error {
109+
return status.Error(codes.Unimplemented, "streaming APIs are not supported")
110+
}
111+
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
4+
5+
package basic_test
6+
7+
import (
8+
"context"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
"google.golang.org/grpc/codes"
14+
"google.golang.org/grpc/metadata"
15+
"google.golang.org/grpc/status"
16+
17+
"github.com/siderolabs/talos/pkg/grpc/middleware/auth/basic"
18+
)
19+
20+
func TestTokenInterceptor(t *testing.T) {
21+
t.Parallel()
22+
23+
const validToken = "valid-token"
24+
25+
creds := basic.NewTokenCredentials(validToken)
26+
27+
interceptor := creds.UnaryInterceptor()
28+
29+
for _, test := range []struct {
30+
name string
31+
32+
md metadata.MD
33+
wantErr bool
34+
}{
35+
{
36+
name: "valid token",
37+
38+
md: metadata.MD{
39+
"token": []string{validToken},
40+
},
41+
wantErr: false,
42+
},
43+
{
44+
name: "invalid token",
45+
46+
md: metadata.MD{
47+
"token": []string{"invalid-token"},
48+
},
49+
wantErr: true,
50+
},
51+
{
52+
name: "missing token",
53+
md: metadata.MD{},
54+
wantErr: true,
55+
},
56+
{
57+
name: "no metadata",
58+
md: nil,
59+
wantErr: true,
60+
},
61+
} {
62+
t.Run(test.name, func(t *testing.T) {
63+
t.Parallel()
64+
65+
ctx := t.Context()
66+
67+
if test.md != nil {
68+
ctx = metadata.NewIncomingContext(ctx, test.md)
69+
}
70+
71+
_, err := interceptor(ctx, nil, nil, func(context.Context, any) (any, error) {
72+
return nil, nil
73+
})
74+
75+
if test.wantErr {
76+
require.Error(t, err)
77+
assert.Equal(t, codes.Unauthenticated, status.Code(err))
78+
} else {
79+
require.NoError(t, err)
80+
}
81+
})
82+
}
83+
}

pkg/grpc/middleware/auth/basic/username_and_password.go

Lines changed: 0 additions & 83 deletions
This file was deleted.

pkg/grpc/middleware/auth/basic/username_and_password_test.go

Lines changed: 0 additions & 14 deletions
This file was deleted.

0 commit comments

Comments
 (0)