From 4bf8588e360bcc05a42dad59e2e9c217cdee8925 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Mon, 2 Nov 2020 15:02:22 -0800 Subject: [PATCH] Add 1 minute clock skew when verifying time constraint Signed-off-by: Wayne Zhang --- BUILD | 4 +- jwt_verify_lib/jwt.h | 16 ++++++ jwt_verify_lib/status.h | 4 +- src/jwt.cc | 17 ++++++ src/jwt_time_test.cc | 111 ++++++++++++++++++++++++++++++++++++++++ src/verify.cc | 9 ++-- src/verify_time_test.cc | 60 ---------------------- 7 files changed, 150 insertions(+), 71 deletions(-) create mode 100644 src/jwt_time_test.cc delete mode 100644 src/verify_time_test.cc diff --git a/BUILD b/BUILD index 04a78ab..31a623a 100644 --- a/BUILD +++ b/BUILD @@ -120,11 +120,11 @@ cc_test( ) cc_test( - name = "verify_time_test", + name = "jwt_time_test", timeout = "short", srcs = [ "src/test_common.h", - "src/verify_time_test.cc", + "src/jwt_time_test.cc", ], linkopts = [ "-lm", diff --git a/jwt_verify_lib/jwt.h b/jwt_verify_lib/jwt.h index e98b415..e24e744 100644 --- a/jwt_verify_lib/jwt.h +++ b/jwt_verify_lib/jwt.h @@ -24,6 +24,9 @@ namespace google { namespace jwt_verify { +// Clock skew defaults to one minute. +constexpr uint64_t kClockSkewInSecond = 60; + /** * struct to hold a JWT data. */ @@ -89,6 +92,19 @@ struct Jwt { * @return the status. */ Status parseFromString(const std::string& jwt); + + /* + * Verify Jwt time constraint if specified + * esp: expiration time, nbf: not before time. + * `now`: is a pass in parameter. + * @return the status. + */ + Status verifyTimeConstraint(uint64_t now) const; + + /* + * Same as above, but `now` is calculated by calling absl::Now. + */ + Status verifyTimeConstraint() const; }; } // namespace jwt_verify diff --git a/jwt_verify_lib/status.h b/jwt_verify_lib/status.h index 85230b5..60f5266 100644 --- a/jwt_verify_lib/status.h +++ b/jwt_verify_lib/status.h @@ -240,9 +240,7 @@ class WithStatus { } } - void resetStatus(Status status) { - status_ = status; - } + void resetStatus(Status status) { status_ = status; } private: // The internal status. diff --git a/src/jwt.cc b/src/jwt.cc index e8b6079..1da8e50 100644 --- a/src/jwt.cc +++ b/src/jwt.cc @@ -19,6 +19,7 @@ #include "absl/container/flat_hash_set.h" #include "absl/strings/escaping.h" #include "absl/strings/str_split.h" +#include "absl/time/clock.h" #include "google/protobuf/util/json_util.h" #include "jwt_verify_lib/struct_utils.h" @@ -136,5 +137,21 @@ Status Jwt::parseFromString(const std::string& jwt) { return Status::Ok; } +Status Jwt::verifyTimeConstraint(uint64_t now) const { + // Check Jwt is active (nbf). + if (now + kClockSkewInSecond < nbf_) { + return Status::JwtNotYetValid; + } + // Check JWT has not expired (exp). + if (exp_ && now > exp_ + kClockSkewInSecond) { + return Status::JwtExpired; + } + return Status::Ok; +} + +Status Jwt::verifyTimeConstraint() const { + return verifyTimeConstraint(absl::ToUnixSeconds(absl::Now())); +} + } // namespace jwt_verify } // namespace google diff --git a/src/jwt_time_test.cc b/src/jwt_time_test.cc new file mode 100644 index 0000000..7053784 --- /dev/null +++ b/src/jwt_time_test.cc @@ -0,0 +1,111 @@ +// Copyright 2020 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "gtest/gtest.h" +#include "jwt_verify_lib/jwt.h" +#include "src/test_common.h" + +namespace google { +namespace jwt_verify { +namespace { + +// Header: {"alg":"RS256","typ":"JWT"} +// Payload: { +// "iss":"https://example.com", +// "sub":"test@example.com", +// "exp": 1605052800, +// "nbf": 1605050800 +// } +const std::string JwtText = + "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9." + "ewogICJpc3MiOiAiaHR0cHM6Ly9leGFtcGxlLmNvbSIsCiAgInN1YiI6ICJ0ZXN0QGV4YW1wbG" + "UuY29tIiwKICAiZXhwIjogMTYwNTA1MjgwMCwKICAibmJmIjogMTYwNTA1MDgwMAp9." + "digk0Fr_IdcWgJNVyeVDw2dC1cQG6LsHwg5pIN93L4"; + +// The exp time for above Jwt +constexpr uint64_t ExpTime = 1605052800U; + +// The nbf time for above Jwt. +constexpr uint64_t NbfTime = 1605050800U; + +TEST(VerifyExpTest, BothNbfExp) { + Jwt jwt; + EXPECT_EQ(jwt.parseFromString(JwtText), Status::Ok); + + // 10s before exp + EXPECT_EQ(jwt.verifyTimeConstraint(ExpTime + kClockSkewInSecond - 10), + Status::Ok); + // 10s after exp + EXPECT_EQ(jwt.verifyTimeConstraint(ExpTime + kClockSkewInSecond + 10), + Status::JwtExpired); + + // 10s after nbf + EXPECT_EQ(jwt.verifyTimeConstraint(NbfTime - kClockSkewInSecond + 10), + Status::Ok); + // 10s befoe nbf + EXPECT_EQ(jwt.verifyTimeConstraint(NbfTime - kClockSkewInSecond - 10), + Status::JwtNotYetValid); +} + +TEST(VerifyExpTest, OnlyExp) { + Jwt jwt; + EXPECT_EQ(jwt.parseFromString(JwtText), Status::Ok); + // Reset nbf + jwt.nbf_ = 0; + + // 10s before exp + EXPECT_EQ(jwt.verifyTimeConstraint(ExpTime + kClockSkewInSecond - 10), + Status::Ok); + // 10s after exp + EXPECT_EQ(jwt.verifyTimeConstraint(ExpTime + kClockSkewInSecond + 10), + Status::JwtExpired); + + // `Now` can be 0, + EXPECT_EQ(jwt.verifyTimeConstraint(0), Status::Ok); +} + +TEST(VerifyExpTest, OnlyNbf) { + Jwt jwt; + EXPECT_EQ(jwt.parseFromString(JwtText), Status::Ok); + // Reset exp + jwt.exp_ = 0; + + // `Now` can be very large + EXPECT_EQ(jwt.verifyTimeConstraint(9223372036854775810U), Status::Ok); + + // 10s after nbf + EXPECT_EQ(jwt.verifyTimeConstraint(NbfTime - kClockSkewInSecond + 10), + Status::Ok); + // 10s befoe nbf + EXPECT_EQ(jwt.verifyTimeConstraint(NbfTime - kClockSkewInSecond - 10), + Status::JwtNotYetValid); +} + +TEST(VerifyExpTest, NotTimeConstraint) { + Jwt jwt; + EXPECT_EQ(jwt.parseFromString(JwtText), Status::Ok); + // Reset both exp and nbf + jwt.exp_ = 0; + jwt.nbf_ = 0; + + // `Now` can be very large + EXPECT_EQ(jwt.verifyTimeConstraint(9223372036854775810U), Status::Ok); + + // `Now` can be 0, + EXPECT_EQ(jwt.verifyTimeConstraint(0), Status::Ok); +} + +} // namespace +} // namespace jwt_verify +} // namespace google diff --git a/src/verify.cc b/src/verify.cc index 5951b15..a31fa9b 100644 --- a/src/verify.cc +++ b/src/verify.cc @@ -304,12 +304,9 @@ Status verifyJwt(const Jwt& jwt, const Jwks& jwks) { } Status verifyJwt(const Jwt& jwt, const Jwks& jwks, uint64_t now) { - // First check that the JWT has not expired (exp) and is active (nbf). - if (now < jwt.nbf_) { - return Status::JwtNotYetValid; - } - if (jwt.exp_ && now > jwt.exp_) { - return Status::JwtExpired; + Status time_status = jwt.verifyTimeConstraint(now); + if (time_status != Status::Ok) { + return time_status; } return verifyJwtWithoutTimeChecking(jwt, jwks); diff --git a/src/verify_time_test.cc b/src/verify_time_test.cc deleted file mode 100644 index d416144..0000000 --- a/src/verify_time_test.cc +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2018 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "gtest/gtest.h" -#include "jwt_verify_lib/verify.h" -#include "src/test_common.h" - -namespace google { -namespace jwt_verify { -namespace { - -// JWT with -// Header: {"alg":"RS256","typ":"JWT"} -// Payload: -// {"iss":"https://example.com","sub":"test@example.com","exp": -// 9223372036854775806,"nbf":10} -const std::string JwtText = - "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9." - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGxlLmNvbSIsIm" - "V4" - "cCI6OTIyMzM3MjAzNjg1NDc3NTgwNiwibmJmIjoxMH0K." - "digk0Fr_IdcWgJNVyeVDw2dC1cQG6LsHwg5pIN93L4_" - "xhEDI3ZFoZ8aE44kvQHWLicnHDlhELqtF-" - "TqxrhfnitpLE7jiyknSu6NVXxtRBcZ3dOTKryVJDvDXcYXOaaP8infnh82loHfhikgg1xmk9" - "rcH50jtc3BkxWNbpNgPyaAAE2tEisIInaxeX0gqkwiNVrLGe1hfwdtdlWFL1WENGlyniQBvB" - "Mwi8DgG_F0eyFKTSRWoaNQQXQruEK0YIcwDj9tkYOXq8cLAnRK9zSYc5-" - "15Hlzfb8eE77pID0HZN-Axeui4IY22I_kYftd0OEqlwXJv_v5p6kNaHsQ9QbtAkw"; - -TEST(VerifyExpTest, Expired) { - Jwt jwt; - Jwks jwks; - EXPECT_EQ(jwt.parseFromString(JwtText), Status::Ok); - // proto.Struct is using "double" for number_value. A big integer has been - // casted to double and back may not be the same. - // In this case, 9223372036854775806 become 9223372036854775808 - // so using 9223372036854775807 for the test will not work. - EXPECT_EQ(verifyJwt(jwt, jwks, 9223372036854775810U), Status::JwtExpired); -} - -TEST(VerifyExpTest, NotBefore) { - Jwt jwt; - Jwks jwks; - EXPECT_EQ(jwt.parseFromString(JwtText), Status::Ok); - EXPECT_EQ(verifyJwt(jwt, jwks, 9), Status::JwtNotYetValid); -} - -} // namespace -} // namespace jwt_verify -} // namespace google