funman300/Ferrous-Solitaire
1
1
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 48 Wiki Activity

refactor(android): forbid unsafe workspace-wide, quarantine JNI in app (#91)

Browse Source 
Addresses #91: the #90 posture (`deny(unsafe_code)` + three scattered
`#![allow(unsafe_code)]` across solitaire_data and solitaire_engine) punched
unsafe holes into otherwise-pure logic crates. Replace it by *reducing* the
unsafe rather than relocating it, then forbidding it everywhere it can be
forbidden.

Changes:
- solitaire_data: new safe `android_jni` bridge owning the cached `JavaVM`
  and activity `GlobalRef`; exposes `with_env` / `with_activity_env` so
  keystore/clipboard/safe-area never touch a raw handle.
- keystore: drop the `JavaVM::from_raw` init path (now in the app) and
  replace the three `unsafe { JByteArray::from_raw(x.into_raw()) }` casts
  with the safe `JByteArray::from(JObject)` conversion jni 0.21 provides.
- engine clipboard + safe_area: route through the bridge; remove their
  `#![allow(unsafe_code)]` and all `from_raw` calls.
- solitaire_app: becomes the single owner of FFI unsafe. `android_main`
  reconstructs the raw `JavaVM` / activity once (it must, as the cdylib that
  exports `#[unsafe(no_mangle)]`) and registers the safe wrappers. It opts to
  its own `deny`-level lints with two scoped `#[allow(unsafe_code)]`.
- workspace: `unsafe_code` is now `forbid`. Every crate except the app entry
  point is fully unsafe-free.

Net: 7 unsafe sites across three crates collapse to 3 at the OS boundary in
one crate. Verified with host `clippy --workspace -- -D warnings` and an
`aarch64-linux-android` clippy build of solitaire_app (transitively engine +
data); also fixed two latent android-only `collapsible_if` warnings surfaced
in the keystore by the cross-target check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
funman300
2026-06-22 11:32:26 -07:00
parent 6a9352cde1
commit 9299176b2d
10 changed files with 190 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files
solitaire_data/src/android_keystore.rs
+35 -72
View File
@@ -1,8 +1,3 @@
// JNI FFI requires `unsafe` to reconstruct `JavaVM` / `JByteArray` handles
// from raw pointers handed over by the Android runtime. Scoped to this
// module so the rest of the workspace stays `deny(unsafe_code)`.
#![allow(unsafe_code)]
/// Android Keystore token storage via JNI.
///
/// Tokens are serialised to JSON, encrypted with AES-256/GCM/NoPadding using a
@@ -19,19 +14,16 @@
///
/// Only compiled and linked on `target_os = "android"`.
use jni::{
JNIEnv, JavaVM,
JNIEnv,
objects::{JByteArray, JObject, JObjectArray, JValue, JValueOwned},
};
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use std::ffi::c_void;
use std::path::PathBuf;
use std::sync::OnceLock;
use crate::auth_tokens::TokenError;
const KEY_ALIAS: &str = "ferrous_solitaire_token_key";
static ANDROID_JVM: OnceLock<JavaVM> = OnceLock::new();
#[derive(Serialize, Deserialize)]
struct TokenBlob {
@@ -44,43 +36,15 @@ struct TokenBlob {
// JVM helper
// ---------------------------------------------------------------------------
/// Initialise Android Keystore access with the process-wide `JavaVM*`.
///
/// This is called by `solitaire_app` from Android startup code. Keeping the
/// raw JVM pointer here avoids making `solitaire_data` depend on the app or
/// engine layer just to reach platform startup state.
pub fn init_android_jvm(vm_ptr: *mut c_void) -> Result<(), TokenError> {
if vm_ptr.is_null() {
return Err(TokenError::KeychainUnavailable(
"JavaVM pointer is null".into(),
));
}
if ANDROID_JVM.get().is_some() {
return Ok(());
}
// SAFETY: `vm_ptr` is supplied by Android startup code and must be the
// process-wide JavaVM* for this app. `OnceLock` keeps the wrapper alive for
// the process lifetime.
let vm = unsafe { JavaVM::from_raw(vm_ptr.cast()) }
.map_err(|e| TokenError::Keyring(format!("JavaVM: {e}")))?;
let _ = ANDROID_JVM.set(vm);
Ok(())
}
/// Run `f` with an attached `JNIEnv`, delegating thread attach and the
/// `JavaVM` handle to the safe [`crate::android_jni`] bridge. The bridge is
/// initialised once from Android startup, so the keystore never touches a raw
/// pointer and this module stays `forbid(unsafe_code)`.
fn with_jvm<F, R>(f: F) -> Result<R, TokenError>
where
F: for<'env> FnOnce(&mut JNIEnv<'env>) -> Result<R, jni::errors::Error>,
{
let vm = ANDROID_JVM
.get()
.ok_or_else(|| TokenError::KeychainUnavailable("Android JavaVM not initialised".into()))?;
let mut env = vm
.attach_current_thread_permanently()
.map_err(|e| TokenError::Keyring(format!("attach: {e}")))?;
f(&mut env).map_err(|e| TokenError::Keyring(format!("JNI: {e}")))
crate::android_jni::with_env(f).map_err(TokenError::Keyring)
}
// ---------------------------------------------------------------------------
@@ -235,9 +199,10 @@ fn encrypt_gcm(
.v()?;
// IV is generated by Android's provider; read it back after init.
// `getIV()` returns `[B`; the safe `From<JObject>` reinterprets the
// returned object reference as a typed byte array.
let iv_jobj = env.call_method(&cipher, "getIV", "()[B", &[])?.l()?;
// SAFETY: the method signature guarantees a byte array return.
let iv_arr = unsafe { JByteArray::from_raw(iv_jobj.into_raw()) };
let iv_arr = JByteArray::from(iv_jobj);
let iv = env.convert_byte_array(&iv_arr)?;
let pt_arr = env.byte_array_from_slice(plaintext)?;
@@ -245,8 +210,7 @@ fn encrypt_gcm(
let ct_jobj = env
.call_method(&cipher, "doFinal", "([B)[B", &[pt_val.borrow()])?
.l()?;
// SAFETY: doFinal([B) returns [B.
let ct_arr = unsafe { JByteArray::from_raw(ct_jobj.into_raw()) };
let ct_arr = JByteArray::from(ct_jobj);
let ciphertext = env.convert_byte_array(&ct_arr)?;
let mut out = Vec::with_capacity(iv.len() + ciphertext.len());
@@ -297,8 +261,7 @@ fn decrypt_gcm(
let pt_jobj = env
.call_method(&cipher, "doFinal", "([B)[B", &[ct_val.borrow()])?
.l()?;
// SAFETY: doFinal([B) returns [B.
let pt_arr = unsafe { JByteArray::from_raw(pt_jobj.into_raw()) };
let pt_arr = JByteArray::from(pt_jobj);
env.convert_byte_array(&pt_arr)
}
@@ -385,29 +348,29 @@ fn read_map() -> Result<HashMap<String, TokenBlob>, TokenError> {
}
// --- 2. Legacy path migration ---
if let Some(ref lpath) = legacy_path {
if lpath.exists() {
let data = read_file_bytes_from(lpath).map_err(|e| match e {
TokenError::NotFound(_) => TokenError::NotFound(String::new()),
other => other,
if let Some(ref lpath) = legacy_path
&& lpath.exists()
{
let data = read_file_bytes_from(lpath).map_err(|e| match e {
TokenError::NotFound(_) => TokenError::NotFound(String::new()),
other => other,
})?;
if data.len() >= 12 {
let plaintext = with_jvm(|env| {
let key = load_or_create_key(env)?;
decrypt_gcm(env, &key, &data)
})?;
if data.len() >= 12 {
let plaintext = with_jvm(|env| {
let key = load_or_create_key(env)?;
decrypt_gcm(env, &key, &data)
})?;
if let Ok(blob) = serde_json::from_slice::<TokenBlob>(&plaintext) {
let mut map = HashMap::new();
map.insert(blob.username.clone(), blob);
// Write to the new location, then remove the legacy file.
if write_map_inner(&map).is_ok() {
let _ = std::fs::remove_file(lpath);
}
return Ok(map);
if let Ok(blob) = serde_json::from_slice::<TokenBlob>(&plaintext) {
let mut map = HashMap::new();
map.insert(blob.username.clone(), blob);
// Write to the new location, then remove the legacy file.
if write_map_inner(&map).is_ok() {
let _ = std::fs::remove_file(lpath);
}
return Ok(map);
}
// Legacy file corrupt or unrecognised — treat as empty.
}
// Legacy file corrupt or unrecognised — treat as empty.
}
// --- 3. No file found ---
@@ -496,11 +459,11 @@ pub fn delete_tokens(username: &str) -> Result<(), TokenError> {
if map.is_empty() {
// No more users — remove the file and the Keystore key.
if let Some(path) = token_file_path() {
if path.exists() {
std::fs::remove_file(&path)
.map_err(|e| TokenError::Keyring(format!("delete auth_tokens.bin: {e}")))?;
}
if let Some(path) = token_file_path()
&& path.exists()
{
std::fs::remove_file(&path)
.map_err(|e| TokenError::Keyring(format!("delete auth_tokens.bin: {e}")))?;
}
// Remove the Keystore key so a future re-login generates a fresh key.