diff options
author | Pavlo Strokov <pstrokov@gitlab.com> | 2020-04-17 15:33:40 +0300 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2020-04-17 15:33:40 +0300 |
commit | 54465275b27178af0268d1281a7f933585f29b0f (patch) | |
tree | 27af8b79b0b3a01ea2896c5e4dea2e3fc0c20329 /auth | |
parent | 0ef438fb0ab8ad9a49a0c42f9722e5647254a253 (diff) |
Drop support for Gitaly v1 authentication
Removal of `RPCCredentials` as it was completely the same
implementation as `RPCCredentialsV2`.
Fix all places where `RPCCredentials` was used.
Removal of `v1` token representation check and validation.
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/2498
Diffstat (limited to 'auth')
-rw-r--r-- | auth/extract_test.go | 66 | ||||
-rw-r--r-- | auth/rpccredentials.go | 8 | ||||
-rw-r--r-- | auth/token.go | 23 |
3 files changed, 9 insertions, 88 deletions
diff --git a/auth/extract_test.go b/auth/extract_test.go index 4274785c4..510fb1790 100644 --- a/auth/extract_test.go +++ b/auth/extract_test.go @@ -7,51 +7,8 @@ import ( "github.com/grpc-ecosystem/go-grpc-middleware/util/metautils" "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/credentials" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/status" ) -func TestCheckTokenV1(t *testing.T) { - secret := "secret 1234" - - testCases := []struct { - desc string - md metadata.MD - code codes.Code - }{ - { - desc: "ok", - md: credsMD(t, RPCCredentials(secret)), - code: codes.OK, - }, - { - desc: "denied", - md: credsMD(t, RPCCredentials("wrong secret")), - code: codes.PermissionDenied, - }, - { - desc: "invalid, not bearer", - md: credsMD(t, &invalidCreds{"foobar"}), - code: codes.Unauthenticated, - }, - { - desc: "invalid, bearer but not base64", - md: credsMD(t, &invalidCreds{"Bearer foo!!bar"}), - code: codes.Unauthenticated, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - ctx := metadata.NewIncomingContext(context.Background(), tc.md) - err := CheckToken(ctx, secret, time.Now()) - require.Equal(t, tc.code, status.Code(err), "expected grpc code in error %v", err) - }) - } -} - func TestCheckTokenV2(t *testing.T) { targetTime := time.Unix(1535671600, 0) secret := []byte("foo") @@ -97,9 +54,14 @@ func TestCheckTokenV2(t *testing.T) { result: errDenied, }, { + desc: "Invalid token format", + token: "foo.bar", + result: errUnauthenticated, + }, + { desc: "Empty token", token: "", - result: errDenied, + result: errUnauthenticated, }, } @@ -113,19 +75,3 @@ func TestCheckTokenV2(t *testing.T) { }) } } - -func credsMD(t *testing.T, creds credentials.PerRPCCredentials) metadata.MD { - md, err := creds.GetRequestMetadata(context.Background()) - require.NoError(t, err) - return metadata.New(md) -} - -type invalidCreds struct { - authHeader string -} - -func (invalidCreds) RequireTransportSecurity() bool { return false } - -func (ic *invalidCreds) GetRequestMetadata(context.Context, ...string) (map[string]string, error) { - return map[string]string{"authorization": ic.authHeader}, nil -} diff --git a/auth/rpccredentials.go b/auth/rpccredentials.go index 923ee8567..9ebf19d15 100644 --- a/auth/rpccredentials.go +++ b/auth/rpccredentials.go @@ -9,14 +9,6 @@ import ( "google.golang.org/grpc/credentials" ) -// RPCCredentials can be used with grpc.WithPerRPCCredentials to create a -// grpc.DialOption that uses v1 Gitaly authentication for authentication -// with a Gitaly server. The shared secret must match the one used on the -// Gitaly server. -func RPCCredentials(sharedSecret string) credentials.PerRPCCredentials { - return &rpcCredentialsV2{sharedSecret: sharedSecret} -} - // RPCCredentialsV2 can be used with grpc.WithPerRPCCredentials to create // a grpc.DialOption that inserts an V2 (HMAC) token with the current // timestamp for authentication with a Gitaly server. The shared secret diff --git a/auth/token.go b/auth/token.go index 755d68933..dee53227c 100644 --- a/auth/token.go +++ b/auth/token.go @@ -4,9 +4,8 @@ import ( "context" "crypto/hmac" "crypto/sha256" - "crypto/subtle" - "encoding/base64" "encoding/hex" + "fmt" "strconv" "strings" "time" @@ -58,17 +57,7 @@ func CheckToken(ctx context.Context, secret string, targetTime time.Time) error return errUnauthenticated } - switch authInfo.Version { - case "v1": - decodedToken, err := base64.StdEncoding.DecodeString(authInfo.Message) - if err != nil { - return errUnauthenticated - } - - if tokensEqual(decodedToken, []byte(secret)) { - return nil - } - case "v2": + if authInfo.Version == "v2" { if v2HmacInfoValid(authInfo.Message, authInfo.SignedMessage, []byte(secret), targetTime, timestampThreshold) { return nil } @@ -77,10 +66,6 @@ func CheckToken(ctx context.Context, secret string, targetTime time.Time) error return errDenied } -func tokensEqual(tok1, tok2 []byte) bool { - return subtle.ConstantTimeCompare(tok1, tok2) == 1 -} - // ExtractAuthInfo returns an `AuthInfo` with the data extracted from `ctx` func ExtractAuthInfo(ctx context.Context) (*AuthInfo, error) { token, err := grpc_auth.AuthFromMD(ctx, "bearer") @@ -91,10 +76,8 @@ func ExtractAuthInfo(ctx context.Context) (*AuthInfo, error) { split := strings.SplitN(token, ".", 3) - // v1 is base64-encoded using base64.StdEncoding, which cannot contain a ".". - // A v1 token cannot slip through here. if len(split) != 3 { - return &AuthInfo{Version: "v1", Message: token}, nil + return nil, fmt.Errorf("invalid token format") } version, sig, msg := split[0], split[1], split[2] |