Update libuweave/macaroon code Added delegation time stamp into access token to match changed validation logic of macaroons. BUG: 26728665 Change-Id: I131b92b0e0b1b2274d80bdc0b5790a8c05071ec5 Reviewed-on: https://weave-review.googlesource.com/2467 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/src/privet/auth_manager.cc b/src/privet/auth_manager.cc index 0b8a981..c82887e 100644 --- a/src/privet/auth_manager.cc +++ b/src/privet/auth_manager.cc
@@ -321,6 +321,8 @@ std::vector<uint8_t> AuthManager::CreateAccessToken(const UserInfo& user_info, base::TimeDelta ttl) const { + const base::Time now = Now(); + TimestampCaveat issued{now}; ScopeCaveat scope{ToMacaroonScope(user_info.scope())}; // Macaroons have no caveats for auth type. So we just append the type to the // user ID. @@ -328,13 +330,14 @@ id_with_type.push_back(static_cast<uint8_t>(user_info.id().type)); UserIdCaveat user{id_with_type}; AppIdCaveat app{user_info.id().app}; - const base::Time now = Now(); ExpirationCaveat expiration{now + ttl}; - return CreateMacaroonToken(access_secret_, now, - { - &scope.GetCaveat(), &user.GetCaveat(), - &app.GetCaveat(), &expiration.GetCaveat(), - }); + return CreateMacaroonToken( + access_secret_, now, + { + + &issued.GetCaveat(), &scope.GetCaveat(), &user.GetCaveat(), + &app.GetCaveat(), &expiration.GetCaveat(), + }); } bool AuthManager::ParseAccessToken(const std::vector<uint8_t>& token, @@ -346,7 +349,7 @@ UwMacaroonValidationResult result{}; const base::Time now = Now(); if (!LoadMacaroon(token, &buffer, &macaroon, error) || - macaroon.num_caveats != 4 || + macaroon.num_caveats != 5 || !VerifyMacaroon(access_secret_, macaroon, now, &result, error)) { return Error::AddTo(error, FROM_HERE, errors::kInvalidAuthorization, "Invalid token");
diff --git a/src/privet/auth_manager_unittest.cc b/src/privet/auth_manager_unittest.cc index a0a0d01..294aefa 100644 --- a/src/privet/auth_manager_unittest.cc +++ b/src/privet/auth_manager_unittest.cc
@@ -70,18 +70,18 @@ } TEST_F(AuthManagerTest, CreateAccessToken) { - EXPECT_EQ("WCaEQgEURglEMjM0AEIKQEYFGhudoQBQaML7svgzFKDXI+/geUUn0w==", + EXPECT_EQ("WC2FRggaG52hAEIBFEYJRDIzNABCCkBGBRobnaEAUFAF46oQlMmXgnLstt7wU2w=", Base64Encode(auth_.CreateAccessToken( UserInfo{AuthScope::kViewer, TestUserId{"234"}}, {}))); - EXPECT_EQ("WCaEQgEIRglEMjU3AEIKQEYFGhudoQBQ0f4NfEW7KDC1QnbExFbf9w==", + EXPECT_EQ("WC2FRggaG52hAEIBCEYJRDI1NwBCCkBGBRobnaEAUEdWRNHcu/0mA6c3e0tgDrk=", Base64Encode(auth_.CreateAccessToken( UserInfo{AuthScope::kManager, TestUserId{"257"}}, {}))); - EXPECT_EQ("WCaEQgECRglENDU2AEIKQEYFGhudoQBQtgk1ZlsGqs5gF7m+UpwxmQ==", + EXPECT_EQ("WC2FRggaG52hAEIBAkYJRDQ1NgBCCkBGBRobnaEAUH2ZLgUPdTtjNRa+PoDkMW4=", Base64Encode(auth_.CreateAccessToken( UserInfo{AuthScope::kOwner, TestUserId{"456"}}, {}))); auto new_time = clock_.Now() + base::TimeDelta::FromDays(11); EXPECT_CALL(clock_, Now()).WillRepeatedly(Return(new_time)); - EXPECT_EQ("WCaEQgEORglEMzQ1AEIKQEYFGhusIYBQl3SG1De+fl2qTquwTl1uRA==", + EXPECT_EQ("WC2FRggaG6whgEIBDkYJRDM0NQBCCkBGBRobrCGAUDAFptj7bbYmbpaa6Wpb1Wo=", Base64Encode(auth_.CreateAccessToken( UserInfo{AuthScope::kUser, TestUserId{"345"}}, {}))); }
diff --git a/third_party/libuweave/src/macaroon.c b/third_party/libuweave/src/macaroon.c index 80e5933..c823804 100644 --- a/third_party/libuweave/src/macaroon.c +++ b/third_party/libuweave/src/macaroon.c
@@ -124,29 +124,32 @@ static void init_validation_result(UwMacaroonValidationResult* result) { // Start from the largest scope - result->granted_scope = kUwMacaroonCaveatScopeTypeOwner; - result->expiration_time = UINT32_MAX; - result->weave_app_restricted = false; - result->lan_session_id = NULL; - result->lan_session_id_len = 0; - result->num_delegatees = 0; + *result = (UwMacaroonValidationResult){ + .granted_scope = kUwMacaroonCaveatScopeTypeOwner, + .expiration_time = UINT32_MAX, + }; } /** Reset the result object to the lowest scope when encountering errors */ static void reset_validation_result(UwMacaroonValidationResult* result) { - // Start from the largest scope or highest privilege - result->granted_scope = 0; - result->expiration_time = 0; - result->weave_app_restricted = true; - result->lan_session_id = NULL; - result->lan_session_id_len = 0; + *result = (UwMacaroonValidationResult){ + .weave_app_restricted = true, + .granted_scope = UW_MACAROON_CAVEAT_SCOPE_LOWEST_POSSIBLE}; +} - result->num_delegatees = 0; - for (size_t i = 0; i < MAX_NUM_DELEGATEES; i++) { - result->delegatees[i].id = NULL; - result->delegatees[i].id_len = 0; - result->delegatees[i].type = kUwMacaroonDelegateeTypeNone; +/** Get the next closest scope (to the narrower side). */ +static UwMacaroonCaveatScopeType get_closest_scope( + UwMacaroonCaveatScopeType scope) { + if (scope <= kUwMacaroonCaveatScopeTypeOwner) { + return kUwMacaroonCaveatScopeTypeOwner; + } else if (scope <= kUwMacaroonCaveatScopeTypeManager) { + return kUwMacaroonCaveatScopeTypeManager; + } else if (scope <= kUwMacaroonCaveatScopeTypeUser) { + return kUwMacaroonCaveatScopeTypeUser; + } else if (scope <= kUwMacaroonCaveatScopeTypeViewer) { + return kUwMacaroonCaveatScopeTypeViewer; } + return scope; } bool uw_macaroon_validate_(const UwMacaroon* macaroon, @@ -178,6 +181,7 @@ } } + result->granted_scope = get_closest_scope(result->granted_scope); return true; }
diff --git a/third_party/libuweave/src/macaroon.h b/third_party/libuweave/src/macaroon.h index c93bbb2..c739bca 100644 --- a/third_party/libuweave/src/macaroon.h +++ b/third_party/libuweave/src/macaroon.h
@@ -36,6 +36,7 @@ const uint8_t* id; size_t id_len; UwMacaroonDelegateeType type; + uint32_t timestamp; } UwMacaroonDelegateeInfo; #define MAX_NUM_DELEGATEES 10 @@ -67,7 +68,8 @@ /** * Verify and validate the Macaroon, and put relevant information into the - * result object. + * result object. Note that the resulting granted_scope will be the closest + * valid scope type (to the narrower side) defined in macaroon_caveat.h. */ bool uw_macaroon_validate_( const UwMacaroon* macaroon,
diff --git a/third_party/libuweave/src/macaroon_caveat.c b/third_party/libuweave/src/macaroon_caveat.c index 85d8bbb..dc4ee3b 100644 --- a/third_party/libuweave/src/macaroon_caveat.c +++ b/third_party/libuweave/src/macaroon_caveat.c
@@ -12,9 +12,6 @@ #include "src/macaroon_context.h" #include "src/macaroon_encoding.h" -// For security sanity checks -#define UW_MACAROON_CAVEAT_SCOPE_LOWEST_POSSIBLE 127 - static bool is_valid_caveat_type_(UwMacaroonCaveatType type) { switch (type) { case kUwMacaroonCaveatTypeNonce: @@ -403,8 +400,9 @@ static bool update_delegatee_list(UwMacaroonCaveatType caveat_type, const UwMacaroonCaveat* caveat, + uint32_t issued_time, UwMacaroonValidationResult* result) { - if (result->num_delegatees >= MAX_NUM_DELEGATEES) { + if (result->num_delegatees >= MAX_NUM_DELEGATEES || issued_time == 0) { return false; } @@ -441,6 +439,7 @@ return false; } result->delegatees[result->num_delegatees].type = delegatee_type; + result->delegatees[result->num_delegatees].timestamp = issued_time; result->num_delegatees++; return true; } @@ -471,22 +470,22 @@ return true; case kUwMacaroonCaveatTypeDelegationTimestamp: - state->has_issued_time = true; - if (!uw_macaroon_caveat_get_value_uint_(caveat, &issued_time)) { + if (!uw_macaroon_caveat_get_value_uint_(caveat, &issued_time) || + issued_time < state->issued_time) { return false; } state->issued_time = issued_time; return true; case kUwMacaroonCaveatTypeTTL1Hour: - if (!(state->has_issued_time)) { + if (state->issued_time == 0) { return false; } return update_and_check_expiration_time( context->current_time, state->issued_time + 60 * 60, result); case kUwMacaroonCaveatTypeTTL24Hour: - if (!(state->has_issued_time)) { + if (state->issued_time == 0) { return false; } return update_and_check_expiration_time( @@ -496,7 +495,8 @@ case kUwMacaroonCaveatTypeDelegateeUser: case kUwMacaroonCaveatTypeDelegateeApp: case kUwMacaroonCaveatTypeDelegateeService: - return update_delegatee_list(caveat_type, caveat, result); + return update_delegatee_list(caveat_type, caveat, state->issued_time, + result); // Time related caveats case kUwMacaroonCaveatTypeExpirationAbsolute: @@ -596,7 +596,6 @@ return false; } - state->has_issued_time = false; state->issued_time = 0; return true; }
diff --git a/third_party/libuweave/src/macaroon_caveat.h b/third_party/libuweave/src/macaroon_caveat.h index 33f9d24..4905667 100644 --- a/third_party/libuweave/src/macaroon_caveat.h +++ b/third_party/libuweave/src/macaroon_caveat.h
@@ -40,6 +40,9 @@ kUwMacaroonCaveatScopeTypeViewer = 20, } UwMacaroonCaveatScopeType; +// For security sanity checks +#define UW_MACAROON_CAVEAT_SCOPE_LOWEST_POSSIBLE 127 + /** Compute the buffer sizes that are enough for caveat creation functions. */ size_t uw_macaroon_caveat_creation_get_buffsize_(UwMacaroonCaveatType type, size_t str_len);
diff --git a/third_party/libuweave/src/macaroon_caveat_internal.h b/third_party/libuweave/src/macaroon_caveat_internal.h index 46a72fb..d6e7b07 100644 --- a/third_party/libuweave/src/macaroon_caveat_internal.h +++ b/third_party/libuweave/src/macaroon_caveat_internal.h
@@ -20,8 +20,7 @@ size_t mac_tag_size); typedef struct { - bool has_issued_time; - uint32_t issued_time; + uint32_t issued_time; // 0 when invalid or not set. } UwMacaroonValidationState; bool uw_macaroon_caveat_init_validation_state_(