Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A network without any entries in public:ccf.gov.nodes.snp.uvm_endorsements does not accept SNP nodes on the basis of hardcoded roots of trust #6482

Open
achamayou opened this issue Sep 16, 2024 · 7 comments
Assignees

Comments

@achamayou
Copy link
Member

achamayou commented Sep 16, 2024

verify_uvm_endorsements() checks endorsements against both internal hardcoded roots of trust, and entries in the public:ccf.gov.nodes.snp.uvm_endorsements map. Only attestations endorsed collateral meeting both requirements are currently allowed through.

This means that a network being updated to SGX requires a governance action to set at least one entry in public:ccf.gov.nodes.snp.uvm_endorsements before nodes can join.

@achamayou
Copy link
Member Author

@andpiccione public:ccf.gov.nodes.snp.uvm_endorsements is populated with the UVM endorsements of the Start node (

InternalTablesAccess::trust_node_uvm_endorsements(
), with did -> (feed, svn) mapping.

It is not updated again automatically, expecting instead that the governance will add to the table with add_snp_uvm_endorsement.

Roots of trust for UVMs have been pinned in the code since #5867, so the following two changes are possible:

  1. Retain the map, to augment valid roots of trust dynamically (but only up to a point, because of the startup self-verification), and expand the policy description endpoint to match
  2. Drop usage of the map altogether, only roots pinned in the code are used

@andpiccione
Copy link
Member

@achamayou It makes sense to keep the map for future flexibility, to allow users to add more trusted roots in the future. However, I would still expect a node to be able to join the network if its UVM endorsements successfully verify against the roots of trust pinned in the code. I'm not sure if it's possible, but we could try verifying the endorsements against both the hardcoded roots of trust and the UVM endorsements in the map; if either check passes, the node should be allowed to join. This way, we can support both new SNP nodes joining a SGX network without having to add the UVM roots of trust and the scenario where custom roots of trust can be used. Nonetheless, if dropping the map is easier, I'm fine with that too.

@achamayou
Copy link
Member Author

However, I would still expect a node to be able to join the network if its UVM endorsements successfully verify against the roots of trust pinned in the code.

Agreed, both options proposed would meet this requirement

I'm not sure if it's possible, but we could try verifying the endorsements against both the hardcoded roots of trust and the UVM endorsements in the map; if either check passes, the node should be allowed to join.

Yes, that's the first option - The only area of doubt is that we currently "self-check" the attestation on node start, before we join, and so that means the table could allow extending the set but only if intersects with what is being started. I don't think that's an issue, just want to make it clear.

This way, we can support both new SNP nodes joining a SGX network without having to add the UVM roots of trust

Adding a did -> (feed, svn) entry via governance prior to the SGX migration should not be major blocker, it's quite a stable value, but I agree it'd be better not to have to do that.

@achamayou achamayou self-assigned this Sep 17, 2024
@andpiccione
Copy link
Member

Yes, that's the first option - The only area of doubt is that we currently "self-check" the attestation on node start, before we join, and so that means the table could allow extending the set but only if intersects with what is being started. I don't think that's an issue, just want to make it clear.

That should be fine for now, we can revisit this later on if needed.

Adding a did -> (feed, svn) entry via governance prior to the SGX migration should not be major blocker, it's quite a stable value, but I agree it'd be better not to have to do that.

It's not a blocker of course and it's definitely doable, but it would be better if we can avoid it just to remove one extra step to perform during code upgrades.

@achamayou
Copy link
Member Author

@andpiccione changed in #6486, but I think this puts us in an awkward spot audit-wise: we record the root of trust for a Start node (with the idea that roots of trust would be managed through governance and kept up to date in the future).

That will need to be removed through governance, when the SVN revs up. To avoid this corner case, we could stop populating it, and treat the map as entirely optional additions to the built-ins roots.

Assuming we do this, the map is there purely optionally, to enable potential rollover to new did/feeds, but remains similarly error prone (you have to not forget to remove the old did -> (feed, svn) once the upgrade has passed).

Aside from the questionable usability, this weakens our audit story, because the UVM roots of trust are no longer explicitly captured in the ledger, except on (did, feed) change, and are instead implicitly captured by the code of the primary.

I think it may be more consistent to use the following logic:

  1. Nodes start, and check their own attestation against hardcoded values in code
  2. When in Start, the relevant value is captured to the ledger
  3. Acceptable values are managed via governance and are auditable
  4. During join, values from the KV alone are used to decide what is acceptable

This also requires a change (at the moment, it's effectively the intersection of what's hardcoded in the primary and what the KV allows), and does require a manual governance action on SGX systems being upgraded, but has the benefit of being regular (every root of trust upgrade requires the same action), and easily auditable.

What do you think?

@achamayou achamayou changed the title Nodes should be able to join a network that has no UVM endorsements in its KV A network without any entries in public:ccf.gov.nodes.snp.uvm_endorsements should still accept SNP nodes on the basis of hardcoded roots of trust Sep 18, 2024
@achamayou achamayou added enhancement and removed bug labels Sep 18, 2024
@achamayou
Copy link
Member Author

achamayou commented Sep 18, 2024

To add some detail on the expected flow for SVN upgrades:

  1. Network: did -> (feed, svn=1)
  2. New joiner: did -> (feed, svn=2) => ALLOWED
  3. Governance: add did -> (feed, svn=2), remove did -> (feed, svn=1)
  4. Network: did -> (feed, svn=2)
  5. New joiner: did -> (feed, svn=1) => DENIED

DID/FEED upgrades

  1. Network: did1 -> (feed1, svn=1)
  2. New joiner: did2 -> (feed2, svn=1) => DENIED
  3. Governance: add did2 -> (feed2, svn=1)
  4. Network: did1 -> (feed1, svn=1), did2 -> (feed2, svn=1)
  5. New joiner: did2 -> (feed2, svn=1) => ALLOWED
  6. Governance: remove did1 -> (feed1, svn=1)
  7. Network: did2 -> (feed2, svn=1)
  8. New joiner: did1 -> (feed1, svn=1) => DENIED

Q: Why does the second scenario require a two-phase add and removal?
A: Because we assume the orchestration switches at some point to "new/replacement nodes are started from this config". In the SVN upgrade scenario, this can happen before we tighten the roots of trust, because the new nodes are acceptable under the old root. But with disjoint conditions, we must temporarily allow both, and can only proscribe the old root when we are sure no nodes relying on it can start.

Q: Can scenario 1 be done implicitly, without governance, if we use built-in roots for validation?
A: Yes. One can switch the orchestration to start nodes that have svn=2, and replace all the nodes gradually. There is no way to tell in ledger when this happened though, without also looking at the code. If the code is not captured directly in the CCEPolicy (because a fragment is used for example), it may be impossible to tell for sure when only new UVMs were accepted.

Q: Can scenario 1 be done implicitly, without governance, if we use built-in roots for validation?
A: No, it is necessary to add the new root, and delete the old.

@achamayou
Copy link
Member Author

achamayou commented Sep 19, 2024

Summary of further discussion, @andpiccione please correct any mistake.

We want to maintain auditability of UVM roots of trusts, nodes should never be accepted in the network without the root of trust for their UVM being present in the ledger/KV at the time. #6482 would go against this and is therefore closed.

  • Nodes do not need to match the roots of trusts hardcoded in the binary running the primary at the time that they join though, they just need to match the current set defined in KV (and auditable through the ledger). A PR is necessary to change this. Only the KV-defined set of UVM roots of trust should be used to accept joining nodes #6489

  • It seems that on network Start, enforcing that the UVM root of trust for the first node matches hardcoded values is unnecessary: it is effectively endorsed by members on open. A separate PR will change that, assuming this is right.

The built-in TS/JS API will continue to use hardcoded roots of trust for now, for compatibility reasons.

@achamayou achamayou changed the title A network without any entries in public:ccf.gov.nodes.snp.uvm_endorsements should still accept SNP nodes on the basis of hardcoded roots of trust A network without any entries in public:ccf.gov.nodes.snp.uvm_endorsements does not accept SNP nodes on the basis of hardcoded roots of trust Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants