Skip to content

Local observation input for the test rig, and make the observation trust model explicit #188

Description

@johanix

Two coupled things, from designing the test rig (#185). The first is a feature; the second is a pre-existing gap that the first one makes relevant.

Background

The rig needs to drive multiple, frequently changing inputs simultaneously so that conflict resolution — pop's actual product — is exercised under sustained churn. Upstream RPZ feeds and local files are straightforward to churn from outside. TAPIR observations are not, because they arrive over MQTT, and MQTT is broker-centric by protocol: a publisher cannot connect to pop, since both publisher and subscriber are clients that dial a broker. Faking that means either embedding a broker or standing one up.

Part 1: a local observation input

Accept observations over a small local input (a REST endpoint) and forward them onto pd.TapirObservations, the same channel the MQTT engine writes to. RefreshEngine (refreshengine.go:91) consumes them identically from there.

It must accept the inner TapirMsg JSON, not a JWS envelope. This is easy to get backwards. In the tapir MQTT engine (mqtt_utils.go:388-401), a validated message has its payload replaced with the verified inner payload before being sent to the subscriber channel:

if validate {
    payload, err := jws.Verify(inbox.Packet.Payload, jws.WithKeySet(me.Keystore))
    if err != nil { ...; continue }   // invalid → dropped, never reaches pop
    mpi.Validated = true
    mpi.Payload = payload             // replaced with the verified inner payload
}
subCh <- mpi

So what the channel carries is decoded JSON, and RefreshEngine calls json.Unmarshal(tpkg.Payload, &tm) on it directly. Handing the endpoint a JWS envelope would simply fail to unmarshal.

What this does and does not exercise

Bypassed broker connection, topic→inbox routing, JWS verification
Exercised payload decode, MsgType routing, ProcessTapirUpdate, conflict resolution, RPZ generation, IXFR chain, downstream NOTIFY

The bypass is narrower than it first appears. Because the engine continues on a verification failure rather than flagging and forwarding, the channel only ever carries verified payloads in production — so an injected message is a faithful representation of what pop actually sees, not a lower-fidelity stand-in.

Deliberately not proposed as an in-process test hook. Keeping it an external interface means the rig can drive the real deployed binary over real sockets, and does not need to live inside pop's process — which also means it does not depend on how #144 resolves.

Containment

It is an input that injects into the policy engine without cryptographic validation, so it should be off by default and explicitly enabled in config. Note that putting it behind the existing API server's X-API-Key is weaker protection than it sounds while #161 is open (non-constant-time comparison, no request body size limit).

Part 2: MqttPkgIn.Validated is never read

grep -rn '\.Validated' *.go in this repo returns nothing. pop sets up the subscription, receives MqttPkgIn values carrying a Validated field, and never consults it.

Today this is safe, but only by accident of configuration. Both subscription call sites — sources.go:203 and configupdater.go:88 — pass validate=true to SubToTopic(topic, ch, mode string, validate bool), and the engine drops what fails. Pass false at any call site, or add a subscription that does, and pop processes unverified observations with nothing in pop positioned to notice.

The trust model is implicit and unstated: "the only writer to this channel is the MQTT engine, and it only writes verified things."

Part 1 makes that assumption false, by adding a second producer to the same channel. So the two belong together: rather than being a reason not to add the input, it is the reason to make the trust model explicit — an observation is processed if it was cryptographically validated or it arrived on an explicitly-configured trusted local input. That way the new path does not have to misrepresent itself in the struct, and the decision lives in config where an operator can see it rather than in an unwritten invariant.

Related to #164 (payload identity not bound to signing key), and worth doing on its own merits regardless of whether the rig input is built.

Notes

  • Part 2 stands alone and could be split into its own issue if that tracks better — happy to do that.
  • Filed while designing the rig for Setup integration tests for POP #185; this is a prerequisite for the observation-churn half of it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Medium priorityenhancementNew feature or requestrobustnessCrash-resistance, error handling, degradationsecurityDoS vectors, auth, trust boundary

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions