Skip to content
Open
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
1 change: 1 addition & 0 deletions Cargo.lock

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

12 changes: 10 additions & 2 deletions aw-datastore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,23 @@ authors = ["Johan Bjäreholt <johan@bjareho.lt>"]
edition = "2021"

[features]
default = [] # no features by default
default = ["bundled"]
# Use bundled SQLite (default, no encryption support)
bundled = ["rusqlite/bundled"]
# Use bundled SQLCipher for encrypted databases (mutually exclusive with 'bundled')
# Build with: cargo build --no-default-features --features encryption
# Requires OpenSSL. Use 'encryption-vendored' to vendor OpenSSL as well.
encryption = ["rusqlite/bundled-sqlcipher"]
# Like 'encryption' but also vendors OpenSSL (fully self-contained)
encryption-vendored = ["rusqlite/bundled-sqlcipher-vendored-openssl"]
legacy_import_tests = []

[dependencies]
dirs = "6"
serde = "1.0"
serde_json = "1.0"
chrono = { version = "0.4", features = ["serde"] }
rusqlite = { version = "0.30", features = ["chrono", "serde_json", "bundled"] }
rusqlite = { version = "0.30", features = ["chrono", "serde_json"] }
mpsc_requests = "0.3"
log = "0.4"

Expand Down
19 changes: 18 additions & 1 deletion aw-datastore/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#[macro_use]
extern crate log;

use std::fmt;

#[macro_export]
macro_rules! json_map {
{ $( $key:literal : $value:expr),* } => {{
Expand All @@ -22,10 +24,25 @@ mod worker;
pub use self::datastore::DatastoreInstance;
pub use self::worker::Datastore;

#[derive(Debug, Clone)]
#[derive(Clone)]
pub enum DatastoreMethod {
Memory(),
File(String),
/// Encrypted SQLite file using SQLCipher. Only available with the
/// `encryption` or `encryption-vendored` feature flags.
#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
FileEncrypted(String, String), // (path, key)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Key material not zeroed on drop

The encryption key is held as a plain String inside FileEncrypted. When the DatastoreMethod value is dropped (after the worker exits), the key bytes remain in process memory until overwritten by the allocator. For an encryption feature this is worth addressing with zeroize::Zeroizing<String>:

// In Cargo.toml (aw-datastore)
zeroize = { version = "1", optional = true }
encryption = ["rusqlite/bundled-sqlcipher", "zeroize"]
encryption-vendored = ["rusqlite/bundled-sqlcipher-vendored-openssl", "zeroize"]

// In lib.rs / worker.rs
#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
use zeroize::Zeroizing;

FileEncrypted(String, Zeroizing<String>)

This ensures the passphrase is securely wiped when the enum is dropped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged — I'll leave the zeroize improvement as a follow-up. The key is stored in DatastoreMethod only until the worker thread starts and extracts it at the top of work_loop, so the exposure window is short. But you're right that Zeroizing<String> would be the clean solution for a hardened build. I'll track it separately rather than block this PR on it.

}

impl fmt::Debug for DatastoreMethod {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
DatastoreMethod::Memory() => write!(f, "Memory()"),
DatastoreMethod::File(p) => write!(f, "File({p:?})"),
#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
DatastoreMethod::FileEncrypted(p, _) => write!(f, "FileEncrypted({p:?}, <redacted>)"),
}
}
}

/* TODO: Implement this as a proper error */
Expand Down
24 changes: 24 additions & 0 deletions aw-datastore/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,20 @@ impl DatastoreWorker {
DatastoreMethod::File(path) => {
Connection::open(path).expect("Failed to create datastore")
}
#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
DatastoreMethod::FileEncrypted(path, key) => {
let conn = Connection::open(path).expect("Failed to create encrypted datastore");
conn.pragma_update(None, "key", key)
.expect("Failed to set SQLCipher encryption key");
// PRAGMA key always succeeds even with a wrong passphrase; the
// first real SQL query is what fails. Read user_version immediately
// to surface an incorrect key as a clear error rather than an
// opaque panic later.
conn.pragma_query_value(None, "user_version", |row| row.get::<_, i64>(0))
.expect("Failed to open encrypted database: wrong passphrase or not an encrypted database");
info!("Opened encrypted database at {}", path);
conn
}
Comment on lines +128 to +141
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Wrong-key failure is a silent worker-thread panic

pragma_update for key succeeds unconditionally — SQLCipher does not validate the passphrase at that point. The first actual read (DatastoreInstance::new(&conn, true).unwrap()) will panic inside the detached worker thread when the key is wrong. The main thread's next request then sees a dead sender and panics separately, with no indication that the cause is a bad passphrase.

Add an explicit verification step immediately after setting the key so the failure is diagnosed clearly:

conn.pragma_update(None, "key", key)
    .expect("Failed to set SQLCipher encryption key");
// Verify the key is correct — PRAGMA user_version is a simple read that
// will return an error if the key cannot decrypt the database header.
conn.pragma_query_value(None, "user_version", |r| r.get::<_, i32>(0))
    .expect("SQLCipher key is incorrect or database is not encrypted");

This produces a human-readable panic message instead of a cryptic SQLite error buried inside a thread.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 67e9062 — added PRAGMA user_version read immediately after PRAGMA key with a clear error message: "Failed to open encrypted database: wrong passphrase or not an encrypted database".

};
let mut ds = DatastoreInstance::new(&conn, true).unwrap();

Expand Down Expand Up @@ -324,6 +338,16 @@ impl Datastore {
Datastore::_new_internal(method, legacy_import)
}

/// Create an encrypted datastore using SQLCipher.
///
/// Requires the `encryption` or `encryption-vendored` feature flag.
/// Build with: `cargo build --no-default-features --features encryption`
#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
pub fn new_encrypted(dbpath: String, key: String, legacy_import: bool) -> Self {
let method = DatastoreMethod::FileEncrypted(dbpath, key);
Datastore::_new_internal(method, legacy_import)
}

fn _new_internal(method: DatastoreMethod, legacy_import: bool) -> Self {
let (requester, responder) =
mpsc_requests::channel::<Command, Result<Response, DatastoreError>>();
Expand Down
43 changes: 43 additions & 0 deletions aw-datastore/tests/datastore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,4 +531,47 @@ mod datastore_tests {
);
}
}

/// Test that an encrypted datastore can be created, written to, and reopened with the same key
/// with data intact.
#[test]
#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
fn test_encrypted_datastore_roundtrip() {
use std::fs;
let dir = get_cache_dir().unwrap();
let db_path = dir.join("test-encrypted.db").to_str().unwrap().to_string();
// Clean up from previous runs
let _ = fs::remove_file(&db_path);

let key = "s3cr3t-p@ssw0rd".to_string();

// Create and populate encrypted datastore
{
let ds = Datastore::new_encrypted(db_path.clone(), key.clone(), false);
let bucket = create_test_bucket(&ds);
let e = Event {
id: None,
timestamp: Utc::now(),
duration: Duration::seconds(1),
data: json_map! { "app": "test-encrypted" },
};
let inserted = ds.insert_events(&bucket.id, &[e]).unwrap();
assert_eq!(inserted.len(), 1);
ds.force_commit().unwrap();
ds.close();
}

// Reopen with correct key — data must survive the roundtrip
{
let ds = Datastore::new_encrypted(db_path.clone(), key.clone(), false);
let events = ds
.get_events("testid", None, None, None)
.expect("should read events from encrypted DB after reopen");
assert_eq!(events.len(), 1, "expected 1 event after encrypted reopen");
assert_eq!(events[0].data["app"], "test-encrypted");
ds.close();
}

let _ = fs::remove_file(&db_path);
}
}
13 changes: 11 additions & 2 deletions aw-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ path = "src/lib.rs"
name = "aw-server"
path = "src/main.rs"

[features]
default = ["bundled"]
# Use bundled SQLite (default, no encryption support)
bundled = ["aw-datastore/bundled"]
# Enable SQLCipher encryption support (requires OpenSSL)
# Build with: cargo build --no-default-features --features encryption
encryption = ["aw-datastore/encryption"]
# Enable SQLCipher encryption with vendored OpenSSL (fully self-contained)
encryption-vendored = ["aw-datastore/encryption-vendored"]

[dependencies]
rocket = { version = "0.5.0", features = ["json"] }
rocket_cors = { version = "0.6.0" }
Expand All @@ -29,8 +39,7 @@ uuid = { version = "1.3", features = ["serde", "v4"] }
clap = { version = "4.1", features = ["derive", "cargo"] }
log-panics = { version = "2", features = ["with-backtrace"]}
rust-embed = { version = "8.0.0", features = ["interpolate-folder-path", "debug-embed"] }

aw-datastore = { path = "../aw-datastore" }
aw-datastore = { path = "../aw-datastore", default-features = false }
aw-models = { path = "../aw-models" }
aw-transform = { path = "../aw-transform" }
aw-query = { path = "../aw-query" }
Expand Down
32 changes: 31 additions & 1 deletion aw-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ struct Opts {
/// Don't import from aw-server-python if no aw-server-rust db found
#[clap(long)]
no_legacy_import: bool,

/// Encryption key for the database (requires 'encryption' feature).
/// Can also be set via the AW_DB_PASSWORD environment variable.
/// WARNING: passing a password on the command line may expose it in process listings.
#[clap(long, env = "AW_DB_PASSWORD")]
#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
db_password: Option<String>,
}

#[rocket::main]
Expand Down Expand Up @@ -141,10 +148,33 @@ async fn main() -> Result<(), rocket::Error> {
device_id::get_device_id()
};

#[cfg(any(feature = "encryption", feature = "encryption-vendored"))]
let datastore = if let Some(key) = opts.db_password {
// Clear the env var immediately so child processes don't inherit the key.
std::env::remove_var("AW_DB_PASSWORD");
info!("Using encrypted database (SQLCipher)");
aw_datastore::Datastore::new_encrypted(db_path, key, legacy_import)
} else {
aw_datastore::Datastore::new(db_path, legacy_import)
};
#[cfg(not(any(feature = "encryption", feature = "encryption-vendored")))]
{
if std::env::var("AW_DB_PASSWORD").is_ok() {
panic!(
"AW_DB_PASSWORD is set but this binary was not compiled with encryption support. \
Refusing to start with an unencrypted database when the user requested encryption. \
Rebuild with the 'encryption' or 'encryption-vendored' feature, or unset \
AW_DB_PASSWORD to use an unencrypted database."
);
}
}
#[cfg(not(any(feature = "encryption", feature = "encryption-vendored")))]
let datastore = aw_datastore::Datastore::new(db_path, legacy_import);
Comment on lines +160 to +172
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security AW_DB_PASSWORD silently ignored in non-encryption builds

When the binary is built without the encryption feature (the default), the db_password field is cfg-gated out of Opts, so clap never registers the AW_DB_PASSWORD env var. A user who sets AW_DB_PASSWORD against a stock binary receives no error, no warning, and no indication that their database is being stored in plaintext — directly contradicting their security expectation.

Add a runtime guard in the non-encryption path:

#[cfg(not(any(feature = "encryption", feature = "encryption-vendored")))]
{
    if std::env::var("AW_DB_PASSWORD").is_ok() {
        warn!(
            "AW_DB_PASSWORD is set but this binary was not compiled with \
             encryption support (missing `encryption` feature). \
             The database will NOT be encrypted and the password is ignored."
        );
    }
    aw_datastore::Datastore::new(db_path, legacy_import)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel like having it set with encryption not built should trigger a panic...

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That's the right call for a security-critical misconfiguration. A warning can be missed; a panic makes the failure impossible to ignore. Here's the updated suggestion:

Suggested change
#[cfg(not(any(feature = "encryption", feature = "encryption-vendored")))]
let datastore = aw_datastore::Datastore::new(db_path, legacy_import);
#[cfg(not(any(feature = "encryption", feature = "encryption-vendored")))]
{
if std::env::var("AW_DB_PASSWORD").is_ok() {
panic!(
"AW_DB_PASSWORD is set but this binary was not compiled with \
encryption support (missing `encryption` or `encryption-vendored` feature). \
Refusing to start to avoid storing data in plaintext against your expectations. \
Rebuild with `--features encryption` or unset AW_DB_PASSWORD."
);
}
aw_datastore::Datastore::new(db_path, legacy_import)
}

The panic message explains both why it's failing and how to fix it, which is the important part — users who hit this will know exactly what to do.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules and .greptile/config.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — addressed in 4597c2c. Changed warn! to panic! so the binary refuses to start when AW_DB_PASSWORD is set but encryption support was not compiled in. The user can either rebuild with the encryption/encryption-vendored feature or unset the env var to use an unencrypted DB knowingly.


let server_state = endpoints::ServerState {
// Even if legacy_import is set to true it is disabled on Android so
// it will not happen there
datastore: Mutex::new(aw_datastore::Datastore::new(db_path, legacy_import)),
datastore: Mutex::new(datastore),
asset_resolver: endpoints::AssetResolver::new(asset_path),
device_id,
};
Expand Down
Loading