Skip to content

Commit

Permalink
Allow leeway for timestamps in reva tokens
Browse files Browse the repository at this point in the history
This bumps golang-jwt/jwt to v5 and uses the new leeway feature to
allow for some clock skew between the reva service, which might be
running on different machines.

Fixes: #4952
  • Loading branch information
rhafer committed Nov 18, 2024
1 parent 4ce61d4 commit 96f5a9e
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 41 deletions.
7 changes: 7 additions & 0 deletions changelog/unreleased/fix-allow-token-clock-skew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Bugfix: Allow small clock skew in reva token validation

Allow for a small clock skew (3 seconds by default) when validating reva tokens
as the different services might be running on different machines.

https://github.com/cs3org/reva/pull/49xx
https://github.com/cs3org/reva/issues/4952
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ require (
github.com/go-redis/redis/v8 v8.11.5
github.com/go-sql-driver/mysql v1.8.1
github.com/gofrs/flock v0.12.1
github.com/golang-jwt/jwt v3.2.2+incompatible
github.com/golang-jwt/jwt/v5 v5.2.1
github.com/golang/protobuf v1.5.4
github.com/gomodule/redigo v1.9.2
github.com/google/go-cmp v0.6.0
Expand Down
6 changes: 2 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46t
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
github.com/cs3org/go-cs3apis v0.0.0-20241105082517-48ba3368a5bd h1:Ji7OTOGVAOynkkiro1Rgv/0sXh1GEWV+4hmGiKmKV3A=
github.com/cs3org/go-cs3apis v0.0.0-20241105082517-48ba3368a5bd/go.mod h1:DedpcqXl193qF/08Y04IO0PpxyyMu8+GrkD6kWK2MEQ=
github.com/cs3org/go-cs3apis v0.0.0-20241105092511-3ad35d174fc1 h1:RU6LT6mkD16xZs011+8foU7T3LrPvTTSWeTQ9OgfhkA=
github.com/cs3org/go-cs3apis v0.0.0-20241105092511-3ad35d174fc1/go.mod h1:DedpcqXl193qF/08Y04IO0PpxyyMu8+GrkD6kWK2MEQ=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down Expand Up @@ -288,8 +286,8 @@ github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7a
github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4=
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY=
github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I=
github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=
github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
Expand Down
12 changes: 6 additions & 6 deletions internal/grpc/services/gateway/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import (
"github.com/cs3org/reva/v2/pkg/share"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
"github.com/golang-jwt/jwt"
"github.com/golang-jwt/jwt/v5"
"github.com/pkg/errors"
gstatus "google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -78,18 +78,18 @@ import (

// transferClaims are custom claims for a JWT token to be used between the metadata and data gateways.
type transferClaims struct {
jwt.StandardClaims
jwt.RegisteredClaims
Target string `json:"target"`
}

func (s *svc) sign(_ context.Context, target string, expiresAt int64) (string, error) {
// Tus sends a separate request to the datagateway service for every chunk.
// For large files, this can take a long time, so we extend the expiration
claims := transferClaims{
StandardClaims: jwt.StandardClaims{
ExpiresAt: expiresAt,
Audience: "reva",
IssuedAt: time.Now().Unix(),
RegisteredClaims: jwt.RegisteredClaims{
ExpiresAt: jwt.NewNumericDate(time.Unix(expiresAt, 0)),
Audience: jwt.ClaimStrings{"reva"},
IssuedAt: jwt.NewNumericDate(time.Now()),
},
Target: target,
}
Expand Down
4 changes: 2 additions & 2 deletions internal/http/services/datagateway/datagateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
"github.com/cs3org/reva/v2/pkg/rhttp"
"github.com/cs3org/reva/v2/pkg/rhttp/global"
"github.com/cs3org/reva/v2/pkg/sharedconf"
"github.com/golang-jwt/jwt"
"github.com/golang-jwt/jwt/v5"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"github.com/rs/zerolog"
Expand All @@ -58,7 +58,7 @@ func init() {

// transferClaims are custom claims for a JWT token to be used between the metadata and data gateways.
type transferClaims struct {
jwt.StandardClaims
jwt.RegisteredClaims
Target string `json:"target"`
}
type config struct {
Expand Down
8 changes: 4 additions & 4 deletions pkg/app/provider/wopi/wopi.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import (
"github.com/cs3org/reva/v2/pkg/sharedconf"
"github.com/cs3org/reva/v2/pkg/storage/utils/templates"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/golang-jwt/jwt"
"github.com/golang-jwt/jwt/v5"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -379,16 +379,16 @@ func getAppURLs(c *config) (map[string]map[string]string, error) {

func (p *wopiProvider) getAccessTokenTTL(ctx context.Context) (string, error) {
tkn := ctxpkg.ContextMustGetToken(ctx)
token, err := jwt.ParseWithClaims(tkn, &jwt.StandardClaims{}, func(token *jwt.Token) (interface{}, error) {
token, err := jwt.ParseWithClaims(tkn, &jwt.RegisteredClaims{}, func(token *jwt.Token) (interface{}, error) {
return []byte(p.conf.JWTSecret), nil
})
if err != nil {
return "", err
}

if claims, ok := token.Claims.(*jwt.StandardClaims); ok && token.Valid {
if claims, ok := token.Claims.(*jwt.RegisteredClaims); ok && token.Valid {
// milliseconds since Jan 1, 1970 UTC as required in https://wopi.readthedocs.io/projects/wopirest/en/latest/concepts.html?highlight=access_token_ttl#term-access-token-ttl
return strconv.FormatInt(claims.ExpiresAt*1000, 10), nil
return strconv.FormatInt(claims.ExpiresAt.Unix()*1000, 10), nil
}

return "", errtypes.InvalidCredentials("wopi: invalid token present in ctx")
Expand Down
10 changes: 5 additions & 5 deletions pkg/siteacc/manager/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ package manager
import (
"time"

"github.com/golang-jwt/jwt"
"github.com/golang-jwt/jwt/v5"
"github.com/pkg/errors"
"github.com/sethvargo/go-password/password"
)

type userToken struct {
jwt.StandardClaims
jwt.RegisteredClaims

User string `json:"user"`
Scope string `json:"scope"`
Expand All @@ -45,10 +45,10 @@ var (
func generateUserToken(user string, scope string, timeout int) (string, error) {
// Create a JWT as the user token
claims := userToken{
StandardClaims: jwt.StandardClaims{
ExpiresAt: time.Now().Add(time.Duration(timeout) * time.Second).Unix(),
RegisteredClaims: jwt.RegisteredClaims{
ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Duration(timeout) * time.Second)),
Issuer: tokenIssuer,
IssuedAt: time.Now().Unix(),
IssuedAt: jwt.NewNumericDate(time.Now()),
},
User: user,
Scope: scope,
Expand Down
12 changes: 6 additions & 6 deletions pkg/storage/utils/decomposedfs/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (

userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/golang-jwt/jwt"
"github.com/golang-jwt/jwt/v5"
"github.com/pkg/errors"
tusd "github.com/tus/tusd/v2/pkg/handler"
"go.opentelemetry.io/otel"
Expand Down Expand Up @@ -372,17 +372,17 @@ func (session *OcisSession) Cleanup(revertNodeMetadata, cleanBin, cleanInfo bool
// URL returns a url to download an upload
func (session *OcisSession) URL(_ context.Context) (string, error) {
type transferClaims struct {
jwt.StandardClaims
jwt.RegisteredClaims
Target string `json:"target"`
}

u := joinurl(session.store.tknopts.DownloadEndpoint, "tus/", session.ID())
ttl := time.Duration(session.store.tknopts.TransferExpires) * time.Second
claims := transferClaims{
StandardClaims: jwt.StandardClaims{
ExpiresAt: time.Now().Add(ttl).Unix(),
Audience: "reva",
IssuedAt: time.Now().Unix(),
RegisteredClaims: jwt.RegisteredClaims{
ExpiresAt: jwt.NewNumericDate(time.Now().Add(ttl)),
Audience: jwt.ClaimStrings{"reva"},
IssuedAt: jwt.NewNumericDate(time.Now()),
},
Target: u,
}
Expand Down
33 changes: 20 additions & 13 deletions pkg/token/manager/jwt/jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,22 @@ import (
"github.com/cs3org/reva/v2/pkg/sharedconf"
"github.com/cs3org/reva/v2/pkg/token"
"github.com/cs3org/reva/v2/pkg/token/manager/registry"
"github.com/golang-jwt/jwt"
"github.com/golang-jwt/jwt/v5"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
)

const defaultExpiration int64 = 86400 // 1 day
const defaultLeeway int64 = 10

func init() {
registry.Register("jwt", New)
}

type config struct {
Secret string `mapstructure:"secret"`
Expires int64 `mapstructure:"expires"`
Secret string `mapstructure:"secret"`
Expires int64 `mapstructure:"expires"`
tokenTimeLeeway int64 `mapstructure:"token_leeway"`
}

type manager struct {
Expand All @@ -50,7 +52,7 @@ type manager struct {

// claims are custom claims for the JWT token.
type claims struct {
jwt.StandardClaims
jwt.RegisteredClaims
User *user.User `json:"user"`
Scope map[string]*auth.Scope `json:"scope"`
}
Expand All @@ -75,6 +77,10 @@ func New(value map[string]interface{}) (token.Manager, error) {
c.Expires = defaultExpiration
}

if c.tokenTimeLeeway == 0 {
c.tokenTimeLeeway = defaultLeeway
}

c.Secret = sharedconf.GetJWTSecret(c.Secret)

if c.Secret == "" {
Expand All @@ -87,31 +93,32 @@ func New(value map[string]interface{}) (token.Manager, error) {

func (m *manager) MintToken(ctx context.Context, u *user.User, scope map[string]*auth.Scope) (string, error) {
ttl := time.Duration(m.conf.Expires) * time.Second
claims := claims{
StandardClaims: jwt.StandardClaims{
ExpiresAt: time.Now().Add(ttl).Unix(),
newClaims := claims{
RegisteredClaims: jwt.RegisteredClaims{
ExpiresAt: jwt.NewNumericDate(time.Now().Add(ttl)),
Issuer: u.Id.Idp,
Audience: "reva",
IssuedAt: time.Now().Unix(),
Audience: jwt.ClaimStrings{"reva"},
IssuedAt: jwt.NewNumericDate(time.Now()),
},
User: u,
Scope: scope,
}

t := jwt.NewWithClaims(jwt.GetSigningMethod("HS256"), claims)
t := jwt.NewWithClaims(jwt.GetSigningMethod("HS256"), newClaims)

tkn, err := t.SignedString([]byte(m.conf.Secret))
if err != nil {
return "", errors.Wrapf(err, "error signing token with claims %+v", claims)
return "", errors.Wrapf(err, "error signing token with claims %+v", newClaims)
}

return tkn, nil
}

func (m *manager) DismantleToken(ctx context.Context, tkn string) (*user.User, map[string]*auth.Scope, error) {
token, err := jwt.ParseWithClaims(tkn, &claims{}, func(token *jwt.Token) (interface{}, error) {
keyfunc := func(token *jwt.Token) (interface{}, error) {
return []byte(m.conf.Secret), nil
})
}
token, err := jwt.ParseWithClaims(tkn, &claims{}, keyfunc, jwt.WithLeeway(time.Duration(m.conf.tokenTimeLeeway)*time.Second))

if err != nil {
return nil, nil, errors.Wrap(err, "error parsing token")
Expand Down

0 comments on commit 96f5a9e

Please sign in to comment.