-
Notifications
You must be signed in to change notification settings - Fork 0
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
40 implement a secure key storage functionality for the pop server #176
base: main
Are you sure you want to change the base?
Changes from all commits
96e069d
b78ba29
3ca8d7e
0a3aa61
acd5d64
6fd582b
e169c08
f069f43
bcb84be
8fe00a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
use thiserror::Error; | ||
|
||
#[derive(Debug, Error)] | ||
pub enum KeystoreError { | ||
#[error("File error: {0}")] | ||
FileError(std::io::Error), | ||
#[error("JwkConversionError")] | ||
JwkConversionError, | ||
#[error("KeyPairGenerationError")] | ||
KeyPairGenerationError, | ||
#[error("non compliant")] | ||
NonCompliant, | ||
#[error("not found")] | ||
NotFound, | ||
#[error("parse error")] | ||
ParseError(serde_json::Error), | ||
#[error("serde error")] | ||
SerdeError(serde_json::Error), | ||
#[error("Encryption error: {0}")] | ||
EncryptionError(chacha20poly1305::Error), | ||
#[error("Decryption error: {0}")] | ||
DecryptionError(chacha20poly1305::Error), | ||
} | ||
impl From<std::io::Error> for KeystoreError { | ||
fn from(err: std::io::Error) -> Self { | ||
KeystoreError::FileError(err) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,31 +1,15 @@ | ||
pub mod filesystem; | ||
pub mod errors; | ||
|
||
use chrono::Utc; | ||
use did_utils::{ | ||
crypto::{Ed25519KeyPair, Generate, ToPublic, X25519KeyPair}, | ||
jwk::Jwk, | ||
}; | ||
use std::error::Error; | ||
use std::{error::Error, fs::File, io::{Read, Write}}; | ||
|
||
use crate::filesystem::FileSystem; | ||
|
||
#[derive(Debug, thiserror::Error)] | ||
pub enum KeyStoreError { | ||
#[error("failure to convert to JWK format")] | ||
JwkConversionError, | ||
#[error("failure to generate key pair")] | ||
KeyPairGenerationError, | ||
#[error("ioerror: {0}")] | ||
IoError(std::io::Error), | ||
#[error("non compliant")] | ||
NonCompliant, | ||
#[error("not found")] | ||
NotFound, | ||
#[error("parse error")] | ||
ParseError(serde_json::Error), | ||
#[error("serde error")] | ||
SerdeError(serde_json::Error), | ||
} | ||
use crate::errors::KeystoreError; | ||
|
||
pub struct KeyStore<'a> { | ||
fs: &'a mut dyn FileSystem, | ||
|
@@ -34,6 +18,80 @@ | |
keys: Vec<Jwk>, | ||
} | ||
|
||
use chacha20poly1305::{ | ||
aead::{generic_array::GenericArray, Aead, AeadCore, OsRng}, | ||
ChaCha20Poly1305, KeyInit, | ||
}; | ||
|
||
use log::{debug, info}; | ||
use secrecy::{ExposeSecret, SecretString}; | ||
use zeroize::Zeroize; | ||
|
||
struct FileSystemKeystore { | ||
Check warning on line 30 in crates/keystore/src/lib.rs GitHub Actions / Build and Test
|
||
key: SecretString, // Store key securely using secrecy crate | ||
nonce: Vec<u8>, | ||
} | ||
Comment on lines
+30
to
+33
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. What is the purpose of this struct? It is not constructed anywhere. |
||
|
||
impl FileSystemKeystore { | ||
fn encrypt(mut self, secret: KeyStore) -> Result<(), KeystoreError> { | ||
Check warning on line 36 in crates/keystore/src/lib.rs GitHub Actions / Build and Test
|
||
let key = self.key.expose_secret(); // Access key securely | ||
let cipher = ChaCha20Poly1305::new(GenericArray::from_slice(key.as_bytes())); | ||
|
||
let nonce = ChaCha20Poly1305::generate_nonce(&mut OsRng); | ||
let path = secret.path(); | ||
let mut keystorefile = File::open(path.clone())?; // Use Result for error handling | ||
|
||
let mut buffer = Vec::new(); | ||
keystorefile.read_to_end(&mut buffer)?; // Use Result for error handling | ||
|
||
let encrypted_key = cipher | ||
.encrypt(GenericArray::from_slice(&nonce), buffer.as_slice()) | ||
.map_err(|err| KeystoreError::EncryptionError(err))?; // Wrap encryption error | ||
|
||
// Overwrite the file with encrypted keys | ||
keystorefile.write_all(&encrypted_key)?; // Use Result for error handling | ||
|
||
// Store the nonce for decryption | ||
self.nonce = nonce.to_vec(); | ||
|
||
// Overwrite the buffer with zeros to prevent data leakage | ||
buffer.clear(); | ||
buffer.zeroize(); | ||
|
||
// Conditional logging | ||
debug!("Encryption successful for keystore file: {}", path); | ||
|
||
Ok(()) | ||
} | ||
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. I don't really understand what you are trying to do here. Your encrypt function is not used anywhere, I suppose that you created it to encrypt the keys before storing them? If that's the purpose, then how are you going to decrypt them later? |
||
|
||
fn decrypt(self, secret: KeyStore) -> Result<Vec<u8>, KeystoreError> { | ||
let key = self.key.expose_secret(); // Access key securely | ||
let cipher = ChaCha20Poly1305::new(GenericArray::from_slice(key.as_bytes())); | ||
|
||
let path = secret.path(); | ||
let mut keystorefile = File::open(path.clone())?; // Use Result for error handling | ||
|
||
let mut buffer = Vec::new(); | ||
keystorefile.read_to_end(&mut buffer)?; // Use Result for error handling | ||
|
||
let decrypted_key = cipher | ||
.decrypt(GenericArray::from_slice(&self.nonce), buffer.as_slice()) | ||
.map_err(|err| KeystoreError::DecryptionError(err))?; // Wrap decryption error | ||
|
||
// Enhanced redaction: Replace all sensitive characters with asterisks | ||
let redacted_key = decrypted_key.iter().map(|b| if b.is_ascii_graphic() && !b.is_ascii_whitespace() { '*' as u8 } else { *b }).collect::<Vec<u8>>(); | ||
|
||
// Conditional logging with redacted key | ||
info!("Decryption successful for keystore file: {}, redacted key: {:?}", &path, redacted_key); | ||
|
||
buffer.clear(); | ||
buffer.zeroize(); | ||
|
||
Ok(decrypted_key) | ||
Tekum-Emmanuella marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
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. Please also provide tests for your functions |
||
|
||
impl<'a> KeyStore<'a> { | ||
/// Constructs file-based key-value store. | ||
pub fn new(fs: &'a mut dyn FileSystem, storage_dirpath: &str) -> Self { | ||
|
@@ -49,13 +107,13 @@ | |
pub fn latest( | ||
fs: &'a mut dyn FileSystem, | ||
storage_dirpath: &str, | ||
) -> Result<Self, KeyStoreError> { | ||
) -> Result<Self, KeystoreError> { | ||
let dirpath = format!("{storage_dirpath}/keystore"); | ||
|
||
// Read directory | ||
let paths = fs | ||
.read_dir_files(&dirpath) | ||
.map_err(KeyStoreError::IoError)?; | ||
.map_err(KeystoreError::FileError)?; | ||
|
||
// Collect paths and associated timestamps of files inside `dir` | ||
let mut collected: Vec<(String, i32)> = vec![]; | ||
|
@@ -65,7 +123,7 @@ | |
.trim_start_matches(&format!("{}/", &dirpath)) | ||
.trim_end_matches(".json") | ||
.parse() | ||
.map_err(|_| KeyStoreError::NonCompliant)?; | ||
.map_err(|_| KeystoreError::NonCompliant)?; | ||
|
||
collected.push((path, stamp)); | ||
} | ||
|
@@ -77,9 +135,9 @@ | |
.max_by_key(|(_, stamp)| stamp) | ||
.map(|(path, _)| path); | ||
|
||
let path = file.ok_or(KeyStoreError::NotFound)?; | ||
let content = fs.read_to_string(path).map_err(KeyStoreError::IoError)?; | ||
let keys = serde_json::from_str::<Vec<Jwk>>(&content).map_err(KeyStoreError::ParseError)?; | ||
let path = file.ok_or(KeystoreError::NotFound)?; | ||
let content = fs.read_to_string(path).map_err(KeystoreError::FileError)?; | ||
let keys = serde_json::from_str::<Vec<Jwk>>(&content).map_err(KeystoreError::ParseError)?; | ||
|
||
let filename = path | ||
.trim_start_matches(&format!("{}/", &dirpath)) | ||
|
@@ -99,16 +157,16 @@ | |
} | ||
|
||
/// Persists store on disk | ||
fn persist(&mut self) -> Result<(), KeyStoreError> { | ||
fn persist(&mut self) -> Result<(), KeystoreError> { | ||
self.fs | ||
.create_dir_all(&self.dirpath) | ||
.map_err(KeyStoreError::IoError)?; | ||
.map_err(KeystoreError::FileError)?; | ||
self.fs | ||
.write( | ||
&self.path(), | ||
&serde_json::to_string_pretty(&self.keys).map_err(KeyStoreError::SerdeError)?, | ||
&serde_json::to_string_pretty(&self.keys).map_err(KeystoreError::SerdeError)?, | ||
) | ||
.map_err(KeyStoreError::IoError) | ||
.map_err(KeystoreError::FileError) | ||
} | ||
|
||
/// Searches keypair given public key | ||
|
@@ -119,10 +177,10 @@ | |
/// Generates and persists an ed25519 keypair for digital signatures. | ||
/// Returns public Jwk for convenience. | ||
pub fn gen_ed25519_jwk(&mut self) -> Result<Jwk, Box<dyn Error>> { | ||
let keypair = Ed25519KeyPair::new().map_err(|_| KeyStoreError::KeyPairGenerationError)?; | ||
let keypair = Ed25519KeyPair::new().map_err(|_| KeystoreError::KeyPairGenerationError)?; | ||
let jwk: Jwk = keypair | ||
.try_into() | ||
.map_err(|_| KeyStoreError::JwkConversionError)?; | ||
.map_err(|_| KeystoreError::JwkConversionError)?; | ||
let pub_jwk = jwk.to_public(); | ||
|
||
self.keys.push(jwk); | ||
|
@@ -133,11 +191,11 @@ | |
|
||
/// Generates and persists an x25519 keypair for digital signatures. | ||
/// Returns public Jwk for convenience. | ||
pub fn gen_x25519_jwk(&mut self) -> Result<Jwk, KeyStoreError> { | ||
let keypair = X25519KeyPair::new().map_err(|_| KeyStoreError::KeyPairGenerationError)?; | ||
pub fn gen_x25519_jwk(&mut self) -> Result<Jwk, KeystoreError> { | ||
let keypair = X25519KeyPair::new().map_err(|_| KeystoreError::KeyPairGenerationError)?; | ||
let jwk: Jwk = keypair | ||
.try_into() | ||
.map_err(|_| KeyStoreError::JwkConversionError)?; | ||
.map_err(|_| KeystoreError::JwkConversionError)?; | ||
let pub_jwk = jwk.to_public(); | ||
|
||
self.keys.push(jwk); | ||
|
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.
Your errors need to be more descriptive: