Skip to content

kv: don't Update HLC clock in response to untrusted tenants #72663

@nvb

Description

@nvb

Discovered in #72278.

We currently update the HLC clock in response to BatchRequests from SQL tenant pods here:

// Update our clock with the incoming request timestamp. This advances the
// local node's clock to a high water mark from all nodes with which it has
// interacted.
if baClockTS, ok := ba.Timestamp.TryToClockTimestamp(); ok {
if s.cfg.TestingKnobs.DisableMaxOffsetCheck {
s.cfg.Clock.Update(baClockTS)
} else {
// If the command appears to come from a node with a bad clock,
// reject it instead of updating the local clock and proceeding.
if err := s.cfg.Clock.UpdateAndCheckMaxOffset(ctx, baClockTS); err != nil {
return nil, roachpb.NewError(err)
}
}
}

This seems like a bad idea, given our multi-tenant threat model where any SQL pod can be entirely owned and we still want to protect the KV host cluster. Currently, a malicious tenant could keep pushing the clock forward by max_offset-1 until it crashed a KV node (through (rpc.RemoteClockMonitor).VerifyClockOffset).

Now that we've convinced ourselves that this HLC channel is not necessary for correctness, we can consider removing the call to (hlc.Clock).Update here for requests originating from secondary tenants.

Jira issue: CRDB-11248

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-kv-transactionsRelating to MVCC and the transactional model.A-multitenancyRelated to multi-tenancyA-securityC-enhancementSolution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)T-kvKV Team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions