Add splitapi resource plugin #585
Conversation
A generic question. Would it make sense to have a generic plugin to create key-pair for a sandbox and not club it with splitapi? |
Some kind of PKI plugin would be very useful. |
We are good. I think it would be a good idea to have the key generation as a basic service needed by other plugins. We can definitely consider refactoring and making it as a basic service. In that case, the splitapi will utilize this basic service. |
|
@salmanyam a potential advantage of a generic PKI approach is that it wouldn't depend on any external patches in kata or wherever to be usable. I haven't look v closely at this PR yet but that's something to consider. |
|
A generic PKI approach will be very helpful for our peer pod use cases. I I understand correctly, the current proposal is to run a key generation service in a trustee, but a PKI approach sounds to me a signing service in a trustee. Trustee has a private key for CA, and accepts key signing requests from clients. A client in a sandbox generates a key pair, and send a key signing request with the generated public key. The CA in Trustee issues a certificate by singing the received public key using the CA private key, and distribute the issued certificate. I think this is how the current PKI for TLS certificates works, and the sandbox does not need to reveal its private key to Trustee. I think this will work If the purpose of the certificate generation is TLS secure tunneling. This is just out of my curiosity, and a key generation service will still very helpful for our peer pod use case. Another point is that the current proposal generates RSA keys. To support other encryption mechanisms such as WireGuard and SSH, it is helpful to support different types of keys such as Ed25529. WireGuard uses Curve25529 keys, and Ed25529 is also preferable for SSH these days. |
|
|
||
| match self.generate_private_key(&self.ca_key, self.key_size) { | ||
| Ok(_) => println!("CA key generation succeeded and saved to {}.", self.ca_key.display()), | ||
| Err(e) => eprintln!("CA key generation failed: {}", e), |
There was a problem hiding this comment.
Please use info! and error! instead of printing.
I don't really like using a match here either. Are these errors fatal? Currently you continue with the function even if it fails. It might make more sense to call each generate function and pass the error up with a question mark. If you need to log something in the success case, you can just add that afterwards.
| let state = "Default State"; | ||
| let locality = "Default City"; | ||
| let organization = "Default Organization"; | ||
| let org_unit = "Default Unit"; |
There was a problem hiding this comment.
These should be updated presumably with something that says that it's coming from a Trustee plugin or you could have this be configurable.
| impl SplitAPIBackend for CertManager { | ||
| async fn get_server_credential(&self, params: &SandboxParams) -> Result<Vec<u8>> { | ||
| // Try locking the sandbox directory mapper | ||
| let mut mapper_guard = self.mapper.lock().map_err(|e| { |
There was a problem hiding this comment.
Having a separate variable for the mapper_guard is a bit c-like. I think you can just use the mapper itself to hold the lock.
| sandbox_dir_info = existing_dir.clone(); | ||
|
|
||
| //TODO: check if the credentails are already in there | ||
| // send the existing credentials if they are not expired |
There was a problem hiding this comment.
When you create the certs, just store the expiration time alongside them so you can easily see if they are expired.
There was a problem hiding this comment.
I think it's a mistake to store all this stuff on the filesystem.
Here's what I would do. Create a struct that contains all the state that represents a keypair. OpenSSL has structs for all the things you need. Store this struct in a dictionary keyed by the connection id. Wrap the struct in a RwLock. If you need some kind of persistence you can serialize/desrialize the dictionary as needed.
I don't see any reason to be writing things to the filesystem and I think this entire mapper concept can be replaced by something from std, which decreases your complexity significantly.
| if let Some(mapper) = mapper_guard.as_mut() { | ||
| let sandbox_dir_info: SandboxDirectoryInfo; | ||
|
|
||
| if let Some(existing_dir) = mapper.get_directory(¶ms.name) { |
There was a problem hiding this comment.
These params are sent in other the network in plaintext. Would it be a problem if someone tampered with the name value in flight?
More fundamentally, why does this need to be stateful at all?
| pub enum SplitAPIConfig { | ||
| CertManager(manager::SplitAPIRepoDesc), | ||
| } |
There was a problem hiding this comment.
If we use enum here, we are hinting that potentially we could have other types of SplitAPIConfig besides CertManager. Do we? If not, I suggest to change SplitAPIConfig into struct and move members of SplitAPIRepoDesc directly into SplitAPIConfig
| lazy_static! { | ||
| static ref SANDBOX_DIRECTORY_MAPPER: Arc<Mutex<Option<SandboxDirectoryMapper>>> = Arc::new(Mutex::new(None)); | ||
| } |
There was a problem hiding this comment.
Could we move this static variable directly into CertManager as a member? s.t.
pub struct CertManager {
pub plugin_dir: String,
pub mapping_filename: String,
mapper: Arc<Mutex<SandboxDirectoryMapper>>,
}Also, could we use tokio::sync::Mutex rather than std::sync::Mutex? The async version would promote CPU efficiency.
| // Try locking the sandbox directory mapper | ||
| let mut mapper_guard = self.mapper.lock().map_err(|e| { | ||
| anyhow!("Failed to lock sandbox directory mapper: {}", e) | ||
| })?; |
There was a problem hiding this comment.
anyhow provides an easier way for map_err(|e| anyhow!(..., e)) like things
xxx.context("Failed to lock sandbox directory mapper")?;
This plugin generates credentials (keys and certificates) for both the API proxy server (required for kata-containers/kata-containers#9159 and kata-containers/kata-containers#9752) and the workload owner. This plugin also delivers the credentials to a sandbox (i.e., confidential PODs or VMs), specifically to the kata agent to initiate the SplitAPI proxy server so that a workload owner can communicate with the proxy server using a secure tunnel. The IPv4 address, name, and the ID of the sandbox must be provided in the query string to obtain the credential resources from the kbs. After receiving the credential request, the splitapi plugin will create a key pair for the server and client and sign them using the self-signed CA. The generated ca.crt, server.crt, and server.key are stored in a directory specific to the sandbox (the caller) and returned to the caller. In addition, ca.key, client.key, and client.crt are also generated and stored to that particular directory specific to the sandbox (i.e., caller). During the credential generation, a sandbox directory mapper creates a unique directory specific to the sandbox (i.e., the caller). The mapper creates the unique directory using the sandbox parameters passed in the query string. A mapping file is also maintained to store the mapping between the sandbox name and the unique directory created for the sandbox. The splitapi plugin itself is not initialized by default. To initialize it, you need to add 'splitapi' in the kbs-config.toml. Signed-off-by: Salman Ahmed <[email protected]>
salmanyam
force-pushed
the
splitapi-plugin
branch
from
20e17e9 to
3951960
Compare
This plugin generates credentials (keys and certificates) for both the API proxy server (required for
kata-containers/kata-containers#9159 and
kata-containers/kata-containers#9752) and the workload owner. This plugin also delivers the credentials to a sandbox (i.e., confidential PODs or VMs), specifically to the kata agent to initiate the SplitAPI proxy server so that a workload owner can communicate with the proxy server using a secure tunnel.
The IPv4 address, name, and the ID of the sandbox must be provided in the query string to obtain the credential resources from the kbs.
After receiving the credential request, the splitapi plugin will create a key pair for the server and client and sign them using the self-signed CA. The generated ca.crt, server.crt, and server.key are stored in a directory specific to the sandbox (the caller) and returned to the caller. In addition, ca.key, client.key, and client.crt are also generated and stored to that particular directory specific to the sandbox (i.e., caller).
During the credential generation, a sandbox directory mapper creates a unique directory specific to the sandbox (i.e., the caller). The mapper creates the unique directory using the sandbox parameters passed in the query string. A mapping file is also maintained to store the mapping between the sandbox name and the unique directory created for the sandbox.
The splitapi plugin itself is not initialized by default. To initialize it, you need to add 'splitapi' in the kbs-config.toml.