-
Notifications
You must be signed in to change notification settings - Fork 2
KMS #22
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
KMS #22
Changes from all commits
ceaa854
509e657
6bcbafc
799a1a8
638fce6
b8c90b8
e016194
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,71 @@ | ||
use alloy::primitives::ChainId; | ||
use alloy::signers::local::PrivateKeySigner; | ||
use alloy_signer_aws::AwsSigner; | ||
use aws_config::{BehaviorVersion, Region}; | ||
use aws_credential_types::provider::future::ProvideCredentials as ProvideCredentialsFuture; | ||
use aws_sdk_kms::config::{Credentials, ProvideCredentials}; | ||
use serde::{Deserialize, Serialize}; | ||
use thirdweb_core::auth::ThirdwebAuth; | ||
use thirdweb_core::iaw::AuthToken; | ||
use vault_types::enclave::auth::Auth; | ||
use vault_types::enclave::auth::Auth as VaultAuth; | ||
|
||
use crate::error::EngineError; | ||
|
||
impl SigningCredential { | ||
/// Create a random private key credential for testing | ||
pub fn random_local() -> Self { | ||
SigningCredential::PrivateKey(PrivateKeySigner::random()) | ||
} | ||
} | ||
|
||
#[derive(Debug, Clone, Serialize, Deserialize)] | ||
pub enum SigningCredential { | ||
Vault(Auth), | ||
Iaw { | ||
auth_token: AuthToken, | ||
thirdweb_auth: ThirdwebAuth | ||
Vault(VaultAuth), | ||
Iaw { | ||
auth_token: AuthToken, | ||
thirdweb_auth: ThirdwebAuth, | ||
}, | ||
AwsKms(AwsKmsCredential), | ||
/// Private key signer for testing and development | ||
/// Note: This should only be used in test environments | ||
#[serde(skip)] | ||
PrivateKey(PrivateKeySigner), | ||
} | ||
|
||
#[derive(Debug, Clone, Serialize, Deserialize)] | ||
pub struct AwsKmsCredential { | ||
pub access_key_id: String, | ||
pub secret_access_key: String, | ||
pub key_id: String, | ||
pub region: String, | ||
} | ||
Comment on lines
+35
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security concern: Storing AWS credentials in memory. Storing AWS access keys directly in the struct poses security risks. Consider using AWS STS temporary credentials or IAM roles instead of long-lived access keys. Consider implementing support for:
🤖 Prompt for AI Agents
|
||
|
||
impl ProvideCredentials for AwsKmsCredential { | ||
fn provide_credentials<'a>(&'a self) -> ProvideCredentialsFuture<'a> | ||
where | ||
Self: 'a, | ||
{ | ||
let credentials = Credentials::new( | ||
self.access_key_id.clone(), | ||
self.secret_access_key.clone(), | ||
None, | ||
None, | ||
"engine-core", | ||
); | ||
ProvideCredentialsFuture::ready(Ok(credentials)) | ||
} | ||
} | ||
|
||
impl AwsKmsCredential { | ||
pub async fn get_signer(&self, chain_id: Option<ChainId>) -> Result<AwsSigner, EngineError> { | ||
let config = aws_config::defaults(BehaviorVersion::latest()) | ||
.credentials_provider(self.clone()) | ||
.region(Region::new(self.region.clone())) | ||
.load() | ||
.await; | ||
let client = aws_sdk_kms::Client::new(&config); | ||
|
||
let signer = AwsSigner::new(client, self.key_id.clone(), chain_id).await?; | ||
Ok(signer) | ||
} | ||
Comment on lines
+60
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add timeout configuration for AWS operations. AWS SDK operations should have timeout configurations to prevent hanging requests. pub async fn get_signer(&self, chain_id: Option<ChainId>) -> Result<AwsSigner, EngineError> {
let config = aws_config::defaults(BehaviorVersion::latest())
.credentials_provider(self.clone())
.region(Region::new(self.region.clone()))
+ .timeout_config(
+ aws_config::timeout::TimeoutConfig::builder()
+ .operation_timeout(std::time::Duration::from_secs(30))
+ .build()
+ )
.load()
.await;
let client = aws_sdk_kms::Client::new(&config);
let signer = AwsSigner::new(client, self.key_id.clone(), chain_id).await?;
Ok(signer)
} 🤖 Prompt for AI Agents
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Document security implications of PrivateKey variant.
The
#[serde(skip)]
attribute prevents serialization, but consider adding runtime validation to ensure this variant is only used in development/test environments.🤖 Prompt for AI Agents