Move authentication logic out of the daphne crate

[mendess]

Proposed changes:

remove any notion of authentication from crates/daphne

This results in:

• DapAggregator::unauthorized_reason being removed
• the generic parameter in DapRequest to be removed which in turn
• causes the DapAggregator, DapLeader and DapHelper generic parameters to also be removed.
• tests in this crate also no longer need to care about authentication, this can be seen in crates/daphne/src/roles/mod.rs, crates/daphne/src/taskprov.rs and crates/daphne/src/testing/mod.rs

Most of the diff in this PR is just removing the generic parameter from all these types

in crates/daphne-server

Introduce a new method to the DaphneService trait called check_bearer_token which can be used in the DapRequestExtractor to guarantee that all requests that are parsed by it are authenticated. A new UnauthenticatedDapRequestExtractor is also included for the cases where we want a DapRequest but don't want to require authentication. Which is the case for upload requests.

misc changes

handle_hpke_config_req no longer requires a full DapRequest as it only needs the DapVersion

Breaking API change

Unauthorized requests to the helper no longer produce DapAbort::UnauthorizedRequest, this is because this error requires a TaskId but the task_id is an optional in the DapRequest. I could make the task_id optional in UnauthorizedRequest but I believe the task_id in DapRequest should not be optional at all. This will be fixed in a future PR

[cjpatton]

Decoupling authorization logic from the daphne crate is a good idea. But if we're taking the time to rework this code, I think we should try to think ahead to how we'd build a multitenant service.

[mendess]

Decoupling authorization logic from the daphne crate is a good idea. But if we're taking the time to rework this code, I think we should try to think ahead to how we'd build a multitenant service.

designing for multi-tenancy is not an easy task and depends a lot on how the system is deployed. I believe the best way to design for multi-tenancy in this case is to not design at all. Our internal implementation already has some semblance of multi tenancy going on and it doesn't depend on this implementation to implement it

[mendess]

One of the intentions of this PR is to distance taskprov from authentication. There are two methods by which an aggregator can determine if the request it receives is authorized:

• static: defined in configuration, this allows for just one token (see peer_auth)
• dynamic: stored in kv mapping (Role, TaskId) pairs to a token (see bearer_tokens)

[cjpatton]

One of the intentions of this PR is to distance taskprov from authentication. There are two methods by which an aggregator can determine if the request it receives is authorized:

* static: defined in configuration, this allows for just one token (see [peer_auth](https://github.com/cloudflare/daphne/blob/62036ff253606d661ad35e04c54ea28f1e50f90d/crates/daphne-service-utils/src/config.rs#L58))

* dynamic: stored in kv mapping (Role, TaskId) pairs to a token (see [bearer_tokens](https://github.com/cloudflare/daphne/blob/62036ff253606d661ad35e04c54ea28f1e50f90d/crates/daphne-server/src/roles/mod.rs#L42))

It's a good goal to decouple authentication logic from protocol logic, including taskprov, but I don't think this is the correct way to do it. What this is PR is missing is a notion of "task ownership". Right now whoever knows the peer_auth token has access to all tasks, not necessarily those it has configured. For example, a task we configure manually can be executed by the peer_auth token holder. I don't think this is what we want.

The current code has a notion of a "taskprov owner", who has access to all tasks (and only those tasks) configured via taskprov. On the other hand, it can't access a manually configured task unless it knows the bearer token for that task.

[cjpatton]

Looking good overall. I believe there's one last authentication bug: if the peer presents a valid TLS certificate, it gets access to any task. Mutual TLS is only meant to be available for the taskprov owner at the moment.

[cjpatton]

Looks good! Just a few more small things, so here's an early 🦆 .

[cjpatton]

Suggestion: If we don't match the first arm because the tokens don't match, then that's an authentication failure. If we don't match because we have a PeerBearerToken::Leader { .. } but a DapSender::Collector, then that's a bug, correct? It might be useful to handle the bug as a fatal error rather than an authentication failure.

[mendess]

that makes sense. It's likely a configuration error