fix(native-certgen): address production audit findings

Make logging init idempotent (swallow SetGlobalDefaultError on repeat
call), remove unused dumpLogs JNI params that violated the API contract,
and strip dead public_key_spki field + build_ec_spki() that were
computed on every keygen but never consumed by the cert builder.
This commit is contained in:
Enginex0
2026-03-09 18:16:50 +01:00
parent 32cfcb3ece
commit 6df266b688
5 changed files with 5 additions and 80 deletions
@@ -50,7 +50,6 @@ data class CertGenConfig(
object NativeCertGen { object NativeCertGen {
private const val LOG_DIR = "/data/adb/tricky_store/logs" private const val LOG_DIR = "/data/adb/tricky_store/logs"
private const val BASE_DIR = "/data/adb/tricky_store"
@Volatile @Volatile
var isAvailable: Boolean = false var isAvailable: Boolean = false
@@ -71,9 +70,9 @@ object NativeCertGen {
private external fun initLogging(verbose: Boolean, logDir: String): Boolean private external fun initLogging(verbose: Boolean, logDir: String): Boolean
private external fun dumpLogs(logDir: String, baseDir: String): String? private external fun dumpLogs(): String?
fun dump(): String? = if (isAvailable) dumpLogs(LOG_DIR, BASE_DIR) else null fun dump(): String? = if (isAvailable) dumpLogs() else null
fun parseNativeResult(bytes: ByteArray): Pair<KeyPair, List<Certificate>> { fun parseNativeResult(bytes: ByteArray): Pair<KeyPair, List<Certificate>> {
val buf = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN) val buf = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN)
-72
View File
@@ -17,8 +17,6 @@ pub fn generate_key_pair(
} }
fn generate_ec_key_pair(curve: EcCurve) -> Result<GeneratedKeyPair> { fn generate_ec_key_pair(curve: EcCurve) -> Result<GeneratedKeyPair> {
use ring::signature::KeyPair;
let alg = match curve { let alg = match curve {
EcCurve::P256 => &ring::signature::ECDSA_P256_SHA256_ASN1_SIGNING, EcCurve::P256 => &ring::signature::ECDSA_P256_SHA256_ASN1_SIGNING,
EcCurve::P384 => &ring::signature::ECDSA_P384_SHA384_ASN1_SIGNING, EcCurve::P384 => &ring::signature::ECDSA_P384_SHA384_ASN1_SIGNING,
@@ -27,79 +25,14 @@ fn generate_ec_key_pair(curve: EcCurve) -> Result<GeneratedKeyPair> {
let rng = ring::rand::SystemRandom::new(); let rng = ring::rand::SystemRandom::new();
let pkcs8_doc = ring::signature::EcdsaKeyPair::generate_pkcs8(alg, &rng)?; let pkcs8_doc = ring::signature::EcdsaKeyPair::generate_pkcs8(alg, &rng)?;
let key_pair = ring::signature::EcdsaKeyPair::from_pkcs8(alg, pkcs8_doc.as_ref(), &rng)?;
let raw_point = key_pair.public_key().as_ref();
let spki = build_ec_spki(curve, raw_point)?;
Ok(GeneratedKeyPair { Ok(GeneratedKeyPair {
private_key_pkcs8: pkcs8_doc.as_ref().to_vec(), private_key_pkcs8: pkcs8_doc.as_ref().to_vec(),
public_key_spki: spki,
}) })
} }
/// Build SubjectPublicKeyInfo DER from a raw EC uncompressed point.
fn build_ec_spki(curve: EcCurve, raw_point: &[u8]) -> Result<Vec<u8>> {
// SPKI = SEQUENCE { AlgorithmIdentifier, BIT STRING(public key) }
// AlgorithmIdentifier = SEQUENCE { OID(ecPublicKey), OID(curve) }
//
// DER-encode manually — the prefix is fixed per curve, only the point varies.
// OID 1.2.840.10045.2.1 (id-ecPublicKey)
const EC_PUBLIC_KEY_OID: &[u8] = &[0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01];
let curve_oid: &[u8] = match curve {
// OID 1.2.840.10045.3.1.7 (prime256v1 / P-256)
EcCurve::P256 => &[0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07],
// OID 1.3.132.0.34 (secp384r1 / P-384)
EcCurve::P384 => &[0x06, 0x05, 0x2b, 0x81, 0x04, 0x00, 0x22],
_ => return Err(CertGenError::UnsupportedEcCurve(curve as i32)),
};
let alg_id_seq = der_sequence(EC_PUBLIC_KEY_OID, curve_oid);
// BIT STRING: 0x03, length, 0x00 (unused bits), raw_point
let bit_string_content_len = 1 + raw_point.len(); // 0x00 byte + point
let mut bit_string = vec![0x03];
encode_der_length(&mut bit_string, bit_string_content_len);
bit_string.push(0x00); // zero unused bits
bit_string.extend_from_slice(raw_point);
// Outer SEQUENCE
let inner_len = alg_id_seq.len() + bit_string.len();
let mut spki = vec![0x30];
encode_der_length(&mut spki, inner_len);
spki.extend_from_slice(&alg_id_seq);
spki.extend_from_slice(&bit_string);
Ok(spki)
}
fn der_sequence(a: &[u8], b: &[u8]) -> Vec<u8> {
let content_len = a.len() + b.len();
let mut seq = vec![0x30];
encode_der_length(&mut seq, content_len);
seq.extend_from_slice(a);
seq.extend_from_slice(b);
seq
}
fn encode_der_length(buf: &mut Vec<u8>, len: usize) {
if len < 0x80 {
buf.push(len as u8);
} else if len < 0x100 {
buf.push(0x81);
buf.push(len as u8);
} else {
buf.push(0x82);
buf.push((len >> 8) as u8);
buf.push(len as u8);
}
}
fn generate_rsa_key_pair(key_size: u32, rsa_public_exponent: u64) -> Result<GeneratedKeyPair> { fn generate_rsa_key_pair(key_size: u32, rsa_public_exponent: u64) -> Result<GeneratedKeyPair> {
use pkcs8::EncodePrivateKey; use pkcs8::EncodePrivateKey;
use rsa::pkcs8::EncodePublicKey;
if !matches!(key_size, 2048 | 3072 | 4096) { if !matches!(key_size, 2048 | 3072 | 4096) {
return Err(CertGenError::InvalidParameter( return Err(CertGenError::InvalidParameter(
@@ -120,12 +53,7 @@ fn generate_rsa_key_pair(key_size: u32, rsa_public_exponent: u64) -> Result<Gene
let pkcs8_der = private_key.to_pkcs8_der() let pkcs8_der = private_key.to_pkcs8_der()
.map_err(|e| CertGenError::SerializationFailed(e.to_string()))?; .map_err(|e| CertGenError::SerializationFailed(e.to_string()))?;
let public_key = private_key.to_public_key();
let pub_der = public_key.to_public_key_der()
.map_err(|e| CertGenError::SerializationFailed(e.to_string()))?;
Ok(GeneratedKeyPair { Ok(GeneratedKeyPair {
private_key_pkcs8: pkcs8_der.as_bytes().to_vec(), private_key_pkcs8: pkcs8_der.as_bytes().to_vec(),
public_key_spki: pub_der.as_ref().to_vec(),
}) })
} }
-2
View File
@@ -126,8 +126,6 @@ fn init_logging_inner(env: &mut JNIEnv, verbose: jboolean, log_dir: &JString) ->
pub extern "system" fn Java_org_matrix_TEESimulator_pki_NativeCertGen_dumpLogs( pub extern "system" fn Java_org_matrix_TEESimulator_pki_NativeCertGen_dumpLogs(
mut env: JNIEnv, mut env: JNIEnv,
_class: JClass, _class: JClass,
_log_dir: JString,
_base_dir: JString,
) -> jstring { ) -> jstring {
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
dump_logs_inner(&mut env) dump_logs_inner(&mut env)
+3 -2
View File
@@ -29,12 +29,13 @@ pub fn init(
let rotating_layer = rotating::RotatingFileLayer::new(log_dir, max_size, max_files); let rotating_layer = rotating::RotatingFileLayer::new(log_dir, max_size, max_files);
let stderr_layer = tracing_subscriber::fmt::layer().with_writer(std::io::stderr); let stderr_layer = tracing_subscriber::fmt::layer().with_writer(std::io::stderr);
tracing_subscriber::registry() // Idempotent — second call returns Ok instead of propagating SetGlobalDefaultError
let _ = tracing_subscriber::registry()
.with(filter) .with(filter)
.with(kmsg_layer) .with(kmsg_layer)
.with(rotating_layer) .with(rotating_layer)
.with(stderr_layer) .with(stderr_layer)
.try_init()?; .try_init();
Ok(()) Ok(())
} }
-1
View File
@@ -118,5 +118,4 @@ pub struct CertGenParams {
pub struct GeneratedKeyPair { pub struct GeneratedKeyPair {
pub private_key_pkcs8: Vec<u8>, pub private_key_pkcs8: Vec<u8>,
pub public_key_spki: Vec<u8>,
} }