[5/8] coordinator: support insecure manifests behind opt-in

[msanft]

Split from #2337 as part of a stacked review series.

Depends on: #2355

This PR wires insecure manifests into the Coordinator behind explicit opt-in:

• manifest helpers for insecure reference values
• Coordinator opt-in gate
• validator registration for insecure manifests
• user API and tests

Closes CON-212

[charludo]

Thanks! Sorry review took so long. I have a couple of comments, see below; the one architectural one I want to get your opinion on is if we should allow mixed manifests. Is that something you actually require? Because if not, then I'd strongly urge to allow either all-secure, or all-insecure manifests, no mix.

[sespiros]

Thanks, I second Charlotte's comment about needing to decide whether we want to allow mixed manifests. My understanding was that we actually do need to do that for the benchmarking use case that triggered this PR series.

If that's the case we should at least close the impersonation gap by adding a per-policy attestation-type binding (which doesn't exist today). An attacker who controls an insecure node can produce an attestation that authenticates as any CC policy in the manifest, because meshapi.NewMeshCert does Policies[hostData] without checking which validator produced the hostData.

For example like:

type PolicyEntry struct {
    SANs                   []string
    WorkloadSecretID       string
    Role                   Role
    AllowedAttestationOIDs []asn1.ObjectIdentifier `json:",omitempty"`
}

[linear-code]

CON-212

[msanft]

the one architectural one I want to get your opinion on is if we should allow mixed manifests. Is that something you actually require?

I've thought about this for some time now, and couldn't think of a use-case we'd need a mixed manifest for. I second your opinion that it should not be allowed then.

[msanft]

ping, this should be ready for another round of review @burgerdev @sespiros @charludo

[sespiros]

Thanks for the changes. We recently did a significant refactor on our validation logic so this needs to be rebased for a final clean review.

The rest of the changes lgtm.

[sespiros]

Thanks, left a few comments.

I am thinking that recovering an insecure manifest should require CONTRAST_ALLOW_INSECURE. We can check the recovered manifest in ResetState and return ErrInsecureNotAllowed when it is insecure and allowInsecure is false. We can also add a test that simulates a restart: create a Guard, call MakeInsecure, and use UpdateState to set an insecure manifest. Then create a new Guard using the same History, without calling MakeInsecure, and verify that ResetState returns ErrInsecureNotAllowed.

[charludo]

Thanks! My original concerns are all covered, and their implementation LGTM. From my side, and not including @sespiros comments of course, this is good to go.

[burgerdev]

Overall lgtm, thanks!

[burgerdev]

The stateguard's purpose is to store, retrieve and update state, and I tried to avoid putting unrelated business logic in here as much as possible. For example, completeness of the manifest, permission to update a manifest and recovery security are checked in userapi (we added a callback to ResetState for that). Arguably that's also not pretty, but I don't have a good alternative idea right now.

[burgerdev]

Maybe I was not clear enough: let's move this logic out of the stateguard and into the userapi for now. Slicing the userapi into subpackages for request processing, authz and such can then be done later.

[sespiros]

Thanks for the changes, nothing more to add.