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

kbs/plugins/resource: Add ID_KEY resource backend #698

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions kbs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ aliyun = ["kms/aliyun"]
# Use pkcs11 resource backend to store secrets in an HSM
pkcs11 = ["cryptoki"]

# Use ID_KEY backend
id-key = ["dep:generic-array", "dep:p384", "dep:sha2"]

[dependencies]
actix = "0.13.5"
actix-web = { workspace = true, features = ["openssl"] }
Expand All @@ -46,13 +49,15 @@ clap = { workspace = true, features = ["derive", "env"] }
config.workspace = true
cryptoki = { version = "0.8.0", optional = true }
env_logger.workspace = true
generic-array = { version = "0.14.7", optional = true }
jsonwebtoken = { workspace = true, default-features = false }
jwt-simple.workspace = true
kbs-types.workspace = true
kms = { workspace = true, default-features = false }
lazy_static = "1.4.0"
log.workspace = true
mobc = { version = "0.8.5", optional = true }
p384 = { version = "0.13.1", optional = true, features = ["arithmetic", "ecdh"] }
prost = { workspace = true, optional = true }
rand = "0.8.5"
regex = "1.11.1"
Expand All @@ -63,6 +68,7 @@ scc = "2"
semver = "1.0.16"
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
sha2 = { version = "0.10.8", optional = true }
strum.workspace = true
thiserror.workspace = true
time = { version = "0.3.37", features = ["std"] }
Expand Down
8 changes: 6 additions & 2 deletions kbs/src/plugins/implementations/resource/aliyun_kms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ pub struct AliyunKmsBackend {

#[async_trait::async_trait]
impl StorageBackend for AliyunKmsBackend {
async fn read_secret_resource(&self, resource_desc: ResourceDesc) -> Result<Vec<u8>> {
async fn read_secret_resource(
&self,
resource_desc: ResourceDesc,
_data: &[u8],
) -> Result<Vec<u8>> {
info!(
"Use aliyun KMS backend. Ignore {}/{}",
resource_desc.repository_name, resource_desc.resource_type
Expand All @@ -44,7 +48,7 @@ impl StorageBackend for AliyunKmsBackend {
&self,
_resource_desc: ResourceDesc,
_data: &[u8],
) -> Result<()> {
) -> Result<Option<Vec<u8>>> {
todo!("Does not support!")
}
}
Expand Down
33 changes: 28 additions & 5 deletions kbs/src/plugins/implementations/resource/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,18 @@ type RepositoryInstance = Arc<dyn StorageBackend>;
#[async_trait::async_trait]
pub trait StorageBackend: Send + Sync {
/// Read secret resource from repository.
async fn read_secret_resource(&self, resource_desc: ResourceDesc) -> Result<Vec<u8>>;
async fn read_secret_resource(
&self,
resource_desc: ResourceDesc,
data: &[u8],
tylerfanelli marked this conversation as resolved.
Show resolved Hide resolved
) -> Result<Vec<u8>>;

/// Write secret resource into repository
async fn write_secret_resource(&self, resource_desc: ResourceDesc, data: &[u8]) -> Result<()>;
async fn write_secret_resource(
&self,
resource_desc: ResourceDesc,
data: &[u8],
) -> Result<Option<Vec<u8>>>;
}

#[derive(Debug, Clone, PartialEq)]
Expand Down Expand Up @@ -80,6 +88,10 @@ pub enum RepositoryConfig {
#[cfg(feature = "pkcs11")]
#[serde(alias = "pkcs11")]
Pkcs11(super::pkcs11::Pkcs11Config),

#[cfg(feature = "id-key")]
#[serde(alias = "id-key")]
IdKey(super::id_key::IdKeyConfig),
}

impl Default for RepositoryConfig {
Expand Down Expand Up @@ -119,6 +131,13 @@ impl TryFrom<RepositoryConfig> for ResourceStorage {
backend: Arc::new(client),
})
}
#[cfg(feature = "id-key")]
RepositoryConfig::IdKey(config) => {
let client = super::id_key::IdKeyBackend::new(&config)?;
Ok(Self {
backend: Arc::new(client),
})
}
}
}
}
Expand All @@ -128,14 +147,18 @@ impl ResourceStorage {
&self,
resource_desc: ResourceDesc,
data: &[u8],
) -> Result<()> {
) -> Result<Option<Vec<u8>>> {
self.backend
.write_secret_resource(resource_desc, data)
.await
}

pub(crate) async fn get_secret_resource(&self, resource_desc: ResourceDesc) -> Result<Vec<u8>> {
self.backend.read_secret_resource(resource_desc).await
pub(crate) async fn get_secret_resource(
&self,
resource_desc: ResourceDesc,
data: &[u8],
) -> Result<Vec<u8>> {
self.backend.read_secret_resource(resource_desc, data).await
}
}

Expand Down
213 changes: 213 additions & 0 deletions kbs/src/plugins/implementations/resource/id_key.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
// Copyright (c) 2025 by Red Hat.
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

use super::{ResourceDesc, StorageBackend};
use aes_gcm::{aead::Aead, Aes256Gcm, Key, KeyInit};
use anyhow::{anyhow, Context, Result};
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
use generic_array::GenericArray;
use openssl::{
pkey::Private,
rsa::{Padding, Rsa},
};
use p384::{ecdh, PublicKey, SecretKey};
use serde::{Deserialize, Serialize};
use sha2::Sha256;

#[derive(Debug, Deserialize, Clone, PartialEq)]
pub struct IdKeyConfig;

impl Default for IdKeyConfig {
fn default() -> Self {
Self
}
}

pub struct IdKeyBackend {
idkey_non_hsm: Rsa<Private>,
ecdh_secret: SecretKey,
}

#[async_trait::async_trait]
impl StorageBackend for IdKeyBackend {
async fn read_secret_resource(
&self,
resource_desc: ResourceDesc,
data: &[u8],
) -> Result<Vec<u8>> {
if resource_desc.repository_name != *"id-key" {
return Err(anyhow!("invalid repository name"));
}

if resource_desc.resource_type == *"ecdh-pub-sec1" {
return Ok(self.ecdh_pubkey_sec1());
}

if resource_desc.resource_type != *"nek" {
return Err(anyhow!("invalid resource type"));
}

let ecdh: IdKeyEcdh =
serde_json::from_slice(data).context("unable to deserialize request body")?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is going to work.

When making a resource request, the TEE public key is not in the request body. It's in the attestation token. Check out how we extract the public key here when we use it to encrypt the response.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't using the TEE public key. This is because I'm only supporting ECDH, and not leaving open the use of RSA like JWK's allow. Would you prefer it use JWK?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you're expecting the request to have an additional key in it? That seems like an extension of the protocol.

Copy link
Contributor Author

@tylerfanelli tylerfanelli Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under impression the protocol was a bit flexible with this. I'm using the protocol in an unconventional way anyway w/r/t needing to first fetch the server's ECDH public key before actually requesting the "resource" (unwrapping+decryption of a key I'm providing).

Perhaps that is a good reason for making this its own plugin?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no laws against it, but it's not totally ideal. This would fit better in another plugin. We do support query strings in the request. Any chance you could put the public key in there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that could be done. I'd actually probably prefer that too, as it basically removes the need for the kbs-types PR I previously mentioned.


let wrapped = URL_SAFE_NO_PAD
.decode(&resource_desc.resource_tag)
.context("invalid resource tag")?;

let encrypted = self.ecdh_unwrap(wrapped, ecdh)?;

let decrypted = {
let mut d = [0u8; 256];
let sz = self
.idkey_non_hsm
.private_decrypt(&encrypted, &mut d, Padding::PKCS1)
.context("unable to decrypt key")?;

Vec::from(&d[..sz])
};

Ok(decrypted)
}

async fn write_secret_resource(
&self,
resource_desc: ResourceDesc,
_data: &[u8],
) -> Result<Option<Vec<u8>>> {
if resource_desc.repository_name != *"id-key" {
return Err(anyhow!("invalid repository name"));
}

if resource_desc.resource_type != *"pwnk" {
return Err(anyhow!("invalid resource type"));
}

let decrypted = URL_SAFE_NO_PAD
.decode(&resource_desc.resource_tag)
.context("invalid resource tag")?;

let encrypted = {
let mut d = [0u8; 256];
let sz = self
.idkey_non_hsm
.public_encrypt(&decrypted, &mut d, Padding::PKCS1)
.context("unable to decrypt key")?;

Vec::from(&d[..sz])
};

Ok(Some(URL_SAFE_NO_PAD.encode(encrypted).into()))
}
}

impl IdKeyBackend {
pub fn new(_config: &IdKeyConfig) -> Result<Self> {
Ok(Self {
idkey_non_hsm: Rsa::generate(2048).context("unable to create ID key")?,
ecdh_secret: SecretKey::random(&mut rand::thread_rng()),
})
}

fn ecdh_pubkey_sec1(&self) -> Vec<u8> {
self.ecdh_secret.public_key().to_sec1_bytes().to_vec()
}

fn ecdh_unwrap(&self, wrapped: Vec<u8>, ecdh: IdKeyEcdh) -> Result<Vec<u8>> {
let shared = ecdh::diffie_hellman(
self.ecdh_secret.to_nonzero_scalar(),
ecdh.public_key.as_affine(),
);
let mut sha_bytes = [0u8; 32];

let hkdf = shared.extract::<Sha256>(None);
if hkdf.expand(&[], &mut sha_bytes).is_err() {
return Err(anyhow!("unable to get ECDH shared SHA hash"));
}

let key = Key::<Aes256Gcm>::from_slice(&sha_bytes);
let aes = Aes256Gcm::new(key);

let encrypted = match aes.decrypt(
GenericArray::from_slice(ecdh.iv.as_slice()),
wrapped.as_ref(),
) {
Ok(e) => e,
Err(_) => {
return Err(anyhow!(
"unable to unwrap encrypted secret with shared AES-GCM key"
))
}
};

Ok(encrypted)
}
}

/*
* FIXME
*
* All of this information is included in kbs_types PR#57:
* https://github.com/virtee/kbs-types/pull/57
*
* Remove everything below when PR is merged.
*/
#[derive(Deserialize, Serialize)]
pub struct IdKeyEcdh {
#[serde(
serialize_with = "serialize_ecdh_pubkey_sec1_base64",
deserialize_with = "deserialize_ecdh_pubkey_sec1_base64"
)]
pub public_key: PublicKey,
#[serde(
serialize_with = "serialize_base64",
deserialize_with = "deserialize_base64"
)]
pub iv: Vec<u8>,
}

fn serialize_ecdh_pubkey_sec1_base64<S>(
sub: &PublicKey,
serializer: S,
) -> core::result::Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
let encoded = URL_SAFE_NO_PAD.encode(sub.to_sec1_bytes());
serializer.serialize_str(&encoded)
}

fn deserialize_ecdh_pubkey_sec1_base64<'de, D>(
deserializer: D,
) -> core::result::Result<PublicKey, D::Error>
where
D: serde::Deserializer<'de>,
{
let encoded = String::deserialize(deserializer)?;
let sec1 = URL_SAFE_NO_PAD
.decode(encoded)
.map_err(serde::de::Error::custom)?;
let public_key = PublicKey::from_sec1_bytes(&sec1).map_err(serde::de::Error::custom)?;

Ok(public_key)
}

fn serialize_base64<S>(sub: &Vec<u8>, serializer: S) -> core::result::Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
let encoded = URL_SAFE_NO_PAD.encode(sub);
serializer.serialize_str(&encoded)
}

fn deserialize_base64<'de, D>(deserializer: D) -> core::result::Result<Vec<u8>, D::Error>
where
D: serde::Deserializer<'de>,
{
let encoded = String::deserialize(deserializer)?;
let decoded = URL_SAFE_NO_PAD
.decode(encoded)
.map_err(serde::de::Error::custom)?;

Ok(decoded)
}
Loading
Loading