[DAP-17] TaskConf AAD Part 4: bind TaskConfiguration into HPKE AADs

[jcjones]

I'm afraid this is another draw the rest of the owl that can't split up worth a darn and have green tests. Except for the ones I'm marking to ignore!

This is broken into sub-commits which deal with these specific headings... but I feel like this one is just as easy to review as a big blob of crazy high quality code

Bind each task's TaskConfiguration into the input-share and aggregate-share HPKE AADs.

Both ends recompute each AAD independently, so byte-identical encoding across every party is mandatory or everything errors out.

• InputShareAad gains task_configuration as field 2
• AggregateShareAad becomes {task_id, task_configuration, collection_job_req}, dropping aggregation_parameter + batch_selector, now matching DAP-18
  • This requires the Helper to do more validations (they became MUST) which I add before the datastore transaction.
• AggregateShareReq becomes {collection_job_req, batch_selector, report_count, checksum}:
  • collection_job_req replaces the standalone aggregation_parameter
  • The Leader forwards its stored CollectionJobReq verbatim and the Helper binds it directly (no reconstruction)
  • the aggregation parameter is now reached via req.collection_job_req().aggregation_parameter()

Important

§4.6.4 defines consistency per batch mode, and for time-interval permits any selector within the queried interval (containment):
all batch buckets whose batch bucket identifiers are contained within the batch interval specified in the CollectionJobReq's query
Here I implement the stricter requirement that the selector reconstruct the query exactly (equality). The reason is that the PUT path stores batch_selector().batch_identifier(), and the poll/GET path rebuilds the AAD's query from that stored identifier alone via query_for_collection_identifier. That reconstruction is lossy for anything narrower than the query, so a strict sub-interval would decrypt on the PUT path and silently fail on the poll path. Equality fails with batchInvalid at PUT; containment would accept and fail silently later. (invalidMessage vs batchInvalid is a judgment call imo, I'm open to changing. I've already changed my mind once.)
The consequence is that Janus rejects some spec-conformant sub-interval requests -- if we want true containment, the Helper needs to persist the query rather than re-derive it, which yet another big expansion of scope for this already big change. I'm erroring on YAGNI here, but if we think we want that, I want to file an issue and take it as a follow-on. I have a test for this, noting the choice.
For leader-selected the check is trivially satisfied, since the query body is empty. (phew)

Note

core/src/vdaf.rs: to_vdaf_config() (which is #[cfg(test-util)]) maps FakeFailsVerifyInit and FakeFailsVerifyStep to VdafConfig::Fake, so the Helper can build an InputShareAad before the intentional verify failure.

Update the interop binaries to build a byte-identical TaskConfiguration.

• Flip AggregatorAddTaskRequest.leader/.helper to janus_messages::Url so endpoint bytes survive interop provisioning verbatim (no url::Url re-encoding); the interop aggregator binds them directly.
• Add a shared INTEROP_TASK_INFO placeholder (the interop test API has no task_info concept) and an interop_batch_config() helper, used identically by the interop aggregator, client, and collector.
• Carry min_batch_size + batch_mode in the interop upload request, and the helper endpoint + min_batch_size in the collector add_task request; new fields are serde-optional so a non-Janus runner still parses.
• Test harness: source real task_info/task_interval/helper endpoint into the collector (collect_aggregate_result_generic now takes a helper_port) and send min_batch_size + batch_mode from the container client.
• Ignore the janus_janus_* container tests: a follow-up will re-enable them.... somehow.

A Stake in tools/collect (see #4713)

new_collector fabricates its TaskConfiguration values to make things work. But now they're actively wrong. I've made the two AAD-consuming paths (run_collection and run_poll_job) now fail fast with an error naming the missing parameters. #4713 (David's suggested --task-config <blob>) is probably The Way.

new-job is unaffected and still works as it never computes an AAD. Success through laziness!

[divergentdave]

§4.6.4 defines consistency per batch mode, and for time-interval permits any selector within the queried interval (containment):
all batch buckets whose batch bucket identifiers are contained within the batch interval specified in the CollectionJobReq's query

I interpret this spec text as describing how the query relates to internal state, not the batch selector. Later on, 5.1.2 says that for the time interval mode, "The payload of BatchSelector.config is struct { Interval batch_interval; } TimeIntervalBatchSelectorConfig; where batch_interval is the batch interval requested by the Collector." This is a straightforward requirement that the same batch interval appear inside the batch_selector field and the nested query field.

Edit: Nevermind, the next paragraph gives the more flexible rules for when the batch selector is consistent query, so I'm now getting mixed messages from this section.

[tgeoghegan]

I think we should correct the spec text to resolve some ambiguities here about whether the query and (full) batch selector need to match exactly. IMO no, they do not, but David is right that the text is unclear at best, if not contradictory here. I wrote up github.com/ietf-wg-ppm/draft-ietf-ppm-dap/issues/791 to track draft-ietf-ppm-dap fixes.

Returning to this PR: I'm not comfortable with the decision to require the aggregate share job's batch selector to exactly match the collection job request's query in the time interval case. It seems reasonable to me to allow leaders to emit batch selectors that are narrower than the query and I think Janus should handle this since we're the reference implementation of DAP. We went to the trouble of making this possible in the spec by including the collection job req in the aggregate share req, so I think we should wire this up properly.

I'm sensitive to the point that persisting the collection job req on the helper side expands the scope of this PR, though, so I'd be OK leaving that as a TODO and doing it in a later PR. Or perhaps resequencing the stack of changes so that you first make a change to persist the collection job req on the helper side and then rebase this change to start folding that into the AAD. I think the latter would be better, but you're doing the actual typing, J.C., so I defer to your preference.

[tgeoghegan]

The code changes in here look fine, though I think it needs re-review after rebasing on #4707 and potentially the change to persist collection job req on the Helper side.

[tgeoghegan]

As discussed in another comment, we should either take a TODO to persist the collection job req and use it in the AAD here, or rebase this change on another that persists the collect req.

[jcjones]

I've opened #4743 for this.

[jcjones]

The code changes in here look fine, though I think it needs re-review after rebasing on #4707 and potentially the change to persist collection job req on the Helper side.

This is already based on #4707. I've opened #4743 for the CollectionJobReq persistence, which I want to take as a follow-on for this rather than make this big change even bigger.