Make Pccs mandatory but add a remote mode with no internal cache - #88
Make Pccs mandatory but add a remote mode with no internal cache#88ameba23 wants to merge 2 commits into
Conversation
samlaf
left a comment
samlaf
left a comment
There was a problem hiding this comment.
Only did a superficial pass but LGTM. Appreciate the docs :)
| pub enum PccsMode { | ||
| /// Fetch collateral from the configured endpoint for every asynchronous | ||
| /// lookup, without keeping an internal cache. | ||
| /// | ||
| /// Synchronous lookups are unavailable in this mode because fetching | ||
| /// collateral requires asynchronous I/O. | ||
| Remote, | ||
| /// Start pre-warming an internal cache when [`Pccs`] is constructed. | ||
| /// | ||
| /// Call [`Pccs::ready`] to wait for the initial pre-warm to complete. | ||
| Prewarmed, | ||
| /// Start with an empty internal cache and fetch collateral on demand. | ||
| Lazy, | ||
| } |
There was a problem hiding this comment.
curious why you chose to use PccsMode::Remote instead of PccsCache::None. Is it because not having a cache doesnt necessarily imply that you are fetching from a remote..?
Otherwise Prewarmed and Lazy are describing a cache strategy whereas remote doesnt (arguably indirectly). Very possible Im just misunmderstanding because I havent reviewed super closely and dont fully understand the subtleties in this PR.
There was a problem hiding this comment.
Because you can set this to be a remote PCCS instance. That is, rather than having an internal cache, you can run one on a remote server and use this instead.
Why would you want to do this?
- For one-shot verifications where you wont keep an instance of AttestationVerifier running over multiple verifications but still want fast fetch.
- For multiple instances behind load balancer that should share a common cache located on same zone or host.
- Intel PCS is rate limited for anonymous users. We have an open issue for adding the option to add an API key here PCCS - allow configuring an API key for Intel PCS #66 - but in some cases this adds friction as all your users need to create one. Providing a PCCS with API key exclusively for your service speeds things up.
dcap-qvlactually defaults tohttps://pccs.phala.networkrun by Phala.
So 'None' implies no cache, wheres it might actually be a remote cache.
There was a problem hiding this comment.
Right but isn't that just a matter of changing the pccs_url? So you'd just turn the cache off and then point to some local shared pccs endpoint that acts as the cache, and in that case "remote" is kind of wrong right given that it's actually inside your own local network.
2 participants
This addresses a bug found by @samlaf - see #70 (comment)
Previously, with no internal PCCS cache configured, a custom remote one cannot be used - we always default to intel PCS.
In some cases we would want a remote pccs rather than the internal in-memory cache. For example when we have multiple instances of the attested TLS proxy behind a load balancer (as we do on Builderhub) they can share a common cache.
This PR refactors things to make the
PccsonAttestationVerifiermandatory (notOption) but add a 'remote mode' which always fetches from the remote resource.