Skip to content

Add a a 5min tolerance to clock skew #211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Parent: add 'VERIFY_AND_CHANGE_EMAIL' linkType for generateEmailActionLink
merged 1 commit into from
Jan 2, 2019
Merged

Add a a 5min tolerance to clock skew #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ const (
idTokenCertURL = "https://www.googleapis.com/robot/v1/metadata/x509/securetoken@system.gserviceaccount.com"
issuerPrefix = "https://securetoken.google.com/"
tokenExpSeconds = 3600
clockSkewSeconds = 300
)

var reservedClaims = []string{
"acr", "amr", "at_hash", "aud", "auth_time", "azp", "cnf", "c_hash",
"exp", "firebase", "iat", "iss", "jti", "nbf", "nonce", "sub",
}

var clk clock = &systemClock{}

// Token represents a decoded Firebase ID token.
//
// Token provides typed accessors to the common JWT fields such as Audience (aud) and Expiry (exp).
Expand All @@ -67,6 +66,7 @@ type Client struct {
projectID string
signer cryptoSigner
version string
clock clock
}

type signer interface {
Expand Down Expand Up @@ -136,6 +136,7 @@ func NewClient(ctx context.Context, conf *internal.AuthConfig) (*Client, error)
projectID: conf.ProjectID,
signer: signer,
version: "Go/Admin/" + conf.Version,
clock: &systemClock{},
}, nil
}

Expand Down Expand Up @@ -186,7 +187,7 @@ func (c *Client) CustomTokenWithClaims(ctx context.Context, uid string, devClaim
return "", fmt.Errorf("developer claims %q are reserved and cannot be specified", strings.Join(disallowed, ", "))
}

now := clk.Now().Unix()
now := c.clock.Now().Unix()
info := &jwtInfo{
header: jwtHeader{Algorithm: "RS256", Type: "JWT"},
payload: &customToken{
Expand Down Expand Up @@ -262,9 +263,9 @@ func (c *Client) VerifyIDToken(ctx context.Context, idToken string) (*Token, err
} else if payload.Issuer != issuer {
err = fmt.Errorf("ID token has invalid 'iss' (issuer) claim; expected %q but got %q; %s; %s",
issuer, payload.Issuer, projectIDMsg, verifyTokenMsg)
} else if payload.IssuedAt > clk.Now().Unix() {
} else if (payload.IssuedAt - clockSkewSeconds) > c.clock.Now().Unix() {
err = fmt.Errorf("ID token issued at future timestamp: %d", payload.IssuedAt)
} else if payload.Expires < clk.Now().Unix() {
} else if (payload.Expires + clockSkewSeconds) < c.clock.Now().Unix() {
err = fmt.Errorf("ID token has expired at: %d", payload.Expires)
} else if payload.Subject == "" {
err = fmt.Errorf("ID token has empty 'sub' (subject) claim; %s", verifyTokenMsg)
Expand Down
50 changes: 45 additions & 5 deletions auth/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var (
var defaultTestOpts = []option.ClientOption{
option.WithCredentialsFile("../testdata/service_account.json"),
}
var testClock = &mockClock{now: time.Now()}

func TestMain(m *testing.M) {
var (
Expand Down Expand Up @@ -74,6 +75,7 @@ func TestMain(m *testing.M) {

ks = &fileKeySource{FilePath: "../testdata/public_certs.json"}
}

client, err = NewClient(ctx, &internal.AuthConfig{
Creds: creds,
Opts: opts,
Expand All @@ -83,6 +85,7 @@ func TestMain(m *testing.M) {
log.Fatalln(err)
}
client.keySource = ks
client.clock = testClock

testGetUserResponse, err = ioutil.ReadFile("../testdata/get_user.json")
if err != nil {
Expand Down Expand Up @@ -289,6 +292,35 @@ func TestVerifyIDToken(t *testing.T) {
}
}

func TestVerifyIDTokenClockSkew(t *testing.T) {
now := testClock.Now().Unix()
cases := []struct {
name string
token string
}{
{"FutureToken", getIDToken(mockIDTokenPayload{"iat": now + clockSkewSeconds - 1})},
{"ExpiredToken", getIDToken(mockIDTokenPayload{
"iat": now - 1000,
"exp": now - clockSkewSeconds + 1,
})},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ft, err := client.VerifyIDToken(ctx, tc.token)
if err != nil {
t.Errorf("VerifyIDToken(%q) = (%q, %v); want = (token, nil)", tc.name, ft, err)
}
if ft.Claims["admin"] != true {
t.Errorf("Claims['admin'] = %v; want = true", ft.Claims["admin"])
}
if ft.UID != ft.Subject {
t.Errorf("UID = %q; Sub = %q; want UID = Sub", ft.UID, ft.Subject)
}
})
}
}

func TestVerifyIDTokenInvalidSignature(t *testing.T) {
parts := strings.Split(testIDToken, ".")
token := fmt.Sprintf("%s:%s:invalidsignature", parts[0], parts[1])
Expand All @@ -298,7 +330,7 @@ func TestVerifyIDTokenInvalidSignature(t *testing.T) {
}

func TestVerifyIDTokenError(t *testing.T) {
now := time.Now().Unix()
now := testClock.Now().Unix()
cases := []struct {
name string
token string
Expand All @@ -310,10 +342,10 @@ func TestVerifyIDTokenError(t *testing.T) {
{"EmptySubject", getIDToken(mockIDTokenPayload{"sub": ""})},
{"IntSubject", getIDToken(mockIDTokenPayload{"sub": 10})},
{"LongSubject", getIDToken(mockIDTokenPayload{"sub": strings.Repeat("a", 129)})},
{"FutureToken", getIDToken(mockIDTokenPayload{"iat": now + 1000})},
{"FutureToken", getIDToken(mockIDTokenPayload{"iat": now + clockSkewSeconds + 1})},
{"ExpiredToken", getIDToken(mockIDTokenPayload{
"iat": now - 1000,
"exp": now - 100,
"exp": now - clockSkewSeconds - 1,
})},
{"EmptyToken", ""},
{"BadFormatToken", "foobar"},
Expand Down Expand Up @@ -419,6 +451,14 @@ func verifyCustomToken(ctx context.Context, token string, expected map[string]in
t.Errorf("Subject: %q; want: %q", payload.Sub, email)
}

now := testClock.Now().Unix()
if payload.Exp != now+3600 {
t.Errorf("Exp: %d; want: %d", payload.Exp, now+3600)
}
if payload.Iat != now {
t.Errorf("Iat: %d; want: %d", payload.Iat, now)
}

for k, v := range expected {
if payload.Claims[k] != v {
t.Errorf("Claim[%q]: %v; want: %v", k, payload.Claims[k], v)
Expand All @@ -434,8 +474,8 @@ func getIDTokenWithKid(kid string, p mockIDTokenPayload) string {
pCopy := mockIDTokenPayload{
"aud": client.projectID,
"iss": "https://securetoken.google.com/" + client.projectID,
"iat": time.Now().Unix() - 100,
"exp": time.Now().Unix() + 3600,
"iat": testClock.Now().Unix() - 100,
"exp": testClock.Now().Unix() + 3600,
"sub": "1234567890",
"admin": true,
}
Expand Down