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

40 implement a secure key storage functionality for the pop server #176

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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: 7 additions & 0 deletions crates/keystore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,10 @@ chrono.workspace = true
serde_json.workspace = true
thiserror.workspace = true
nix.workspace = true
chacha20poly1305 = "0.10.1"
log = "0.4.22"
openssl = "0.10.66"
jsonwebtoken = "9.3.0"
zeroize.workspace = true
secrecy = "0.10.2"
Tekum-Emmanuella marked this conversation as resolved.
Show resolved Hide resolved

90 changes: 89 additions & 1 deletion crates/keystore/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
pub mod filesystem;

use chacha20poly1305::{aead::generic_array::GenericArray, KeyInit};

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;

Expand Down Expand Up @@ -33,6 +39,87 @@
filename: String,
keys: Vec<Jwk>,
}
use chacha20poly1305::{
aead::{Aead, AeadCore, OsRng},
ChaCha20Poly1305,
};

use log::{debug, info, error}; // Import logging macros
use secrecy::{ExposeSecret, SecretString};
use zeroize::Zeroize;

// Define a custom error type for keystore operations
#[derive(Debug, thiserror::Error)]
pub enum KeystoreError {
#[error("File error: {0}")]
FileError(std::io::Error),
#[error("Encryption error: {0}")]
EncryptionError(chacha20poly1305::Error),
#[error("Decryption error: {0}")]
DecryptionError(chacha20poly1305::Error),
}
Tekum-Emmanuella marked this conversation as resolved.
Show resolved Hide resolved


struct FileSystemKeystore {

Check warning on line 63 in crates/keystore/src/lib.rs

View workflow job for this annotation

GitHub Actions / Build and Test

struct `FileSystemKeystore` is never constructed

Check warning on line 63 in crates/keystore/src/lib.rs

View workflow job for this annotation

GitHub Actions / Build and Test

struct `FileSystemKeystore` is never constructed
key: SecretString, // Store key securely using secrecy crate
nonce: Vec<u8>,
}
Comment on lines +30 to +33
Copy link
Collaborator

Choose a reason for hiding this comment

The 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<(), Box<dyn Error>> {

Check warning on line 69 in crates/keystore/src/lib.rs

View workflow job for this annotation

GitHub Actions / Build and Test

methods `encrypt` and `decrypt` are never used

Check warning on line 69 in crates/keystore/src/lib.rs

View workflow job for this annotation

GitHub Actions / Build and Test

methods `encrypt` and `decrypt` are never used
Tekum-Emmanuella marked this conversation as resolved.
Show resolved Hide resolved
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(&self.nonce), buffer.as_slice())
.map_err(|err| err).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you map the error when not going to propagate it?

// Overwrite the file with encrypted keys
keystorefile.write_all(&encrypted_key)?; // Use Result for error handling

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(())
}
Copy link
Collaborator

@Hermann-Core Hermann-Core Sep 26, 2024

Choose a reason for hiding this comment

The 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>, std::io::Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This functions you created are not used, where do you call them ?

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| err).unwrap();

// 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
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.
Expand Down Expand Up @@ -147,6 +234,7 @@
}
}


#[cfg(test)]
mod tests {
use super::*;
Expand Down
Loading