Make RequestStreams accept empty TenantInfo

+ Make Authz tokens survive Transaction resets
+ Make tokens explicitly resettable by assigning empty value to authz token TR option
+ Fill out missing trace for some failed token verifications
+ Trace non-sensitive token parts upon failed verification
This commit is contained in:
Junhyun Shim 2022-07-18 13:07:00 +02:00
parent 03a496c0e0
commit ca29cd9f41
7 changed files with 39 additions and 33 deletions

View File

@ -6117,7 +6117,6 @@ ACTOR static Future<Void> tryCommit(Reference<TransactionState> trState,
}
req.tenantInfo = trState->getTenantInfo();
startTime = now();
state Optional<UID> commitID = Optional<UID>();
@ -6619,8 +6618,10 @@ void Transaction::setOption(FDBTransactionOptions::Option option, Optional<Strin
break;
case FDBTransactionOptions::AUTHORIZATION_TOKEN:
validateOptionValuePresent(value);
trState->setToken(value.get());
if (value.present())
trState->setToken(value.get());
else
trState->clearToken();
break;
default:

View File

@ -176,7 +176,7 @@ struct CommitTransactionRequest : TimedRequest {
CommitTransactionRequest() : CommitTransactionRequest(SpanContext()) {}
CommitTransactionRequest(SpanContext const& context) : spanContext(context), flags(0) {}
bool verify() const { return tenantInfo.hasAuthorizedTenant(); }
bool verify() const { return tenantInfo.empty() || tenantInfo.hasAuthorizedTenant(); }
template <class Ar>
void serialize(Ar& ar) {
@ -356,7 +356,7 @@ struct GetKeyServerLocationsRequest {
: arena(arena), spanContext(spanContext), tenant(tenant), begin(begin), end(end), limit(limit), reverse(reverse),
minTenantVersion(minTenantVersion) {}
bool verify() const { return tenant.hasAuthorizedTenant(); }
bool verify() const { return tenant.empty() || tenant.hasAuthorizedTenant(); }
template <class Ar>
void serialize(Ar& ar) {

View File

@ -278,6 +278,7 @@ struct TransactionState : ReferenceCounted<TransactionState> {
bool hasTenant() const;
void setToken(Standalone<StringRef> token) { authToken_ = token; }
void clearToken() { authToken_.reset(); }
private:
Optional<TenantName> tenant_;

View File

@ -31,7 +31,7 @@
#include "fdbrpc/LoadBalance.actor.h"
#include "fdbrpc/Stats.h"
#include "fdbrpc/TimedRequest.h"
#include "fdbrpc/TenantAuth.actor.h"
#include "fdbrpc/TenantAuth.h"
#include "fdbrpc/TSSComparison.h"
#include "fdbclient/CommitTransaction.h"
#include "fdbclient/TagThrottle.actor.h"
@ -268,7 +268,7 @@ struct GetValueRequest : TimedRequest {
// to this client, of all storage replicas that
// serve the given key
bool verify() const { return tenantInfo.hasAuthorizedTenant(); }
bool verify() const { return tenantInfo.empty() || tenantInfo.hasAuthorizedTenant(); }
GetValueRequest() {}
GetValueRequest(SpanContext spanContext,
@ -324,7 +324,7 @@ struct WatchValueRequest {
: spanContext(spanContext), tenantInfo(tenantInfo), key(key), value(value), version(ver), tags(tags),
debugID(debugID) {}
bool verify() const { return tenantInfo.hasAuthorizedTenant(); }
bool verify() const { return tenantInfo.empty() || tenantInfo.hasAuthorizedTenant(); }
template <class Ar>
void serialize(Ar& ar) {
@ -369,7 +369,7 @@ struct GetKeyValuesRequest : TimedRequest {
GetKeyValuesRequest() : isFetchKeys(false) {}
bool verify() const { return tenantInfo.hasAuthorizedTenant(); }
bool verify() const { return tenantInfo.empty() || tenantInfo.hasAuthorizedTenant(); }
template <class Ar>
void serialize(Ar& ar) {
@ -428,7 +428,7 @@ struct GetMappedKeyValuesRequest : TimedRequest {
GetMappedKeyValuesRequest() : isFetchKeys(false) {}
bool verify() const { return tenantInfo.hasAuthorizedTenant(); }
bool verify() const { return tenantInfo.empty() || tenantInfo.hasAuthorizedTenant(); }
template <class Ar>
void serialize(Ar& ar) {
@ -496,7 +496,7 @@ struct GetKeyValuesStreamRequest {
GetKeyValuesStreamRequest() : isFetchKeys(false) {}
bool verify() const { return tenantInfo.hasAuthorizedTenant(); }
bool verify() const { return tenantInfo.empty() || tenantInfo.hasAuthorizedTenant(); }
template <class Ar>
void serialize(Ar& ar) {
@ -545,7 +545,7 @@ struct GetKeyRequest : TimedRequest {
// to this client, of all storage replicas that
// serve the given key
bool verify() const { return tenantInfo.hasAuthorizedTenant(); }
bool verify() const { return tenantInfo.empty() || tenantInfo.hasAuthorizedTenant(); }
GetKeyRequest() {}

View File

@ -151,6 +151,12 @@ bool TokenCache::validate(TenantNameRef name, StringRef token) {
return impl->validate(name, token);
}
#define TRACE_INVALID_PARSED_TOKEN(reason, token) \
TraceEvent(SevWarn, "InvalidToken") \
.detail("From", peer) \
.detail("Reason", reason) \
.detail("Token", token.toStringRef(arena).toStringView())
bool TokenCacheImpl::validateAndAdd(double currentTime,
StringRef signature,
StringRef token,
@ -159,40 +165,45 @@ bool TokenCacheImpl::validateAndAdd(double currentTime,
authz::jwt::TokenRef t;
if (!authz::jwt::parseToken(arena, t, token)) {
TEST(true); // Token can't be parsed
TraceEvent(SevWarn, "InvalidToken")
.detail("From", peer)
.detail("Reason", "ParseError")
.detail("Token", token.toString());
return false;
}
auto key = FlowTransport::transport().getPublicKeyByName(t.keyId);
if (!key.present()) {
TEST(true); // Token referencing non-existing key
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "UnknownKey");
TRACE_INVALID_PARSED_TOKEN("UnknownKey", t);
return false;
} else if (!t.expiresAtUnixTime.present()) {
TEST(true); // Token has no expiration time
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "NoExpirationTime");
TRACE_INVALID_PARSED_TOKEN("NoExpirationTime", t);
return false;
} else if (double(t.expiresAtUnixTime.get()) <= currentTime) {
TEST(true); // Expired token
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "Expired");
TRACE_INVALID_PARSED_TOKEN("Expired", t);
return false;
} else if (!t.notBeforeUnixTime.present()) {
TEST(true); // Token has no not-before field
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "NoNotBefore");
TRACE_INVALID_PARSED_TOKEN("NoNotBefore", t);
return false;
} else if (double(t.notBeforeUnixTime.get()) > currentTime) {
TEST(true); // Tokens not-before is in the future
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "TokenNotYetValid");
TRACE_INVALID_PARSED_TOKEN("TokenNotYetValid", t);
return false;
} else if (!t.tenants.present()) {
TEST(true); // Token with no tenants
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "NoTenants");
TRACE_INVALID_PARSED_TOKEN("NoTenants", t);
return false;
} else if (!authz::jwt::verifyToken(token, key.get())) {
TEST(true); // Token with invalid signature
TraceEvent(SevWarn, "InvalidToken").detail("From", peer).detail("Reason", "InvalidSignature");
TRACE_INVALID_PARSED_TOKEN("InvalidSignature", t);
return false;
} else {
CacheEntry c;
c.expirationTime = double(t.expiresAtUnixTime.get());
c.tenants.reserve(c.arena, t.tenants.get().size());
for (auto tenant : t.tenants.get()) {
c.tenants.push_back_deep(c.arena, tenant);
}
@ -203,14 +214,18 @@ bool TokenCacheImpl::validateAndAdd(double currentTime,
}
bool TokenCacheImpl::validate(TenantNameRef name, StringRef token) {
NetworkAddress peer = FlowTransport::transport().currentDeliveryPeerAddress();
auto sig = authz::jwt::signaturePart(token);
if (sig.empty()) {
TEST(true); // Token is ill-formed
TraceEvent(SevWarn, "InvalidToken")
.detail("From", peer)
.detail("Reason", "NoSignaturePart")
.detail("BadToken", token.toString());
return false;
}
auto cachedEntry = cache.get(sig);
double currentTime = g_network->timer();
NetworkAddress peer = FlowTransport::transport().currentDeliveryPeerAddress();
if (!cachedEntry.present()) {
if (validateAndAdd(currentTime, sig, token, peer)) {

View File

@ -37,19 +37,6 @@
#include "flow/actorcompiler.h" // has to be last include
// TODO: receive and validate token instead
struct AuthorizationRequest {
constexpr static FileIdentifier file_identifier = 11499331;
Arena arena;
VectorRef<StringRef> tokens;
template <class Ar>
void serialize(Ar& ar) {
serializer(ar, tokens, arena);
}
};
template <>
struct serializable_traits<TenantInfo> : std::true_type {
template <class Archiver>

View File

@ -45,6 +45,8 @@ struct TenantInfo {
// the client is trying to access data of a tenant it is authorized to use.
bool hasAuthorizedTenant() const { return trusted || (name.present() && verified); }
bool empty() const { return !name.present() && tenantId == INVALID_TENANT; }
TenantInfo() : tenantId(INVALID_TENANT) {}
TenantInfo(Optional<TenantName> const& tenantName, Optional<Standalone<StringRef>> const& token, int64_t tenantId)
: tenantId(tenantId) {