[accless] Add C++ Wrappers For abe4 Library#41
Conversation
|
/gemini review |
3285467 to
332a268
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces C++ wrappers for the abe4 Rust library, which replaces the previous rabe library for attribute-based encryption. The changes are extensive, including new FFI in Rust, C++ wrapper code, a new base64 utility library, and comprehensive unit and API tests. Additionally, there are significant improvements to the Docker-based development environment and the accli command-line tool, making them more robust and user-friendly. My review highlights a critical issue regarding potential panics across the FFI boundary in the Rust code, a high-severity issue where core logic in accless.cpp has been commented out, and several medium-severity suggestions to improve build script debuggability and security. I have also pointed out some minor typos in the documentation.
| #[allow(clippy::missing_safety_doc)] | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn setup_abe4(auths_json: *const c_char) -> *mut c_char { | ||
| let auths_cstr = unsafe { CStr::from_ptr(auths_json) }; | ||
|
|
||
| let auths: Vec<String> = serde_json::from_str(auths_cstr.to_str().unwrap()).unwrap(); | ||
| let auths_ref: Vec<&str> = auths.iter().map(|s| s.as_str()).collect(); | ||
| let mut rng = ark_std::rand::thread_rng(); | ||
|
|
||
| let (msk, mpk) = setup(&mut rng, &auths_ref); | ||
|
|
||
| let mut msk_bytes = Vec::new(); | ||
| msk.serialize_compressed(&mut msk_bytes).unwrap(); | ||
|
|
||
| let mut mpk_bytes = Vec::new(); | ||
| mpk.serialize_compressed(&mut mpk_bytes).unwrap(); | ||
|
|
||
| let output = SetupOutput { | ||
| msk: general_purpose::STANDARD.encode(&msk_bytes), | ||
| mpk: general_purpose::STANDARD.encode(&mpk_bytes), | ||
| }; | ||
|
|
||
| let output_json = serde_json::to_string(&output).unwrap(); | ||
| CString::new(output_json).unwrap().into_raw() | ||
| } | ||
|
|
||
| #[allow(clippy::missing_safety_doc)] | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn keygen_abe4( | ||
| gid: *const c_char, | ||
| msk_b64: *const c_char, | ||
| user_attrs_json: *const c_char, | ||
| ) -> *mut c_char { | ||
| let gid_cstr = unsafe { CStr::from_ptr(gid) }; | ||
| let msk_b64_cstr = unsafe { CStr::from_ptr(msk_b64) }; | ||
| let user_attrs_cstr = unsafe { CStr::from_ptr(user_attrs_json) }; | ||
|
|
||
| let msk_bytes = general_purpose::STANDARD | ||
| .decode(msk_b64_cstr.to_str().unwrap()) | ||
| .unwrap(); | ||
| let msk: MSK = MSK::deserialize_compressed(&msk_bytes[..]).unwrap(); | ||
|
|
||
| let user_attrs: Vec<UserAttribute> = | ||
| serde_json::from_str(user_attrs_cstr.to_str().unwrap()).unwrap(); | ||
|
|
||
| let iota = Iota::new(&user_attrs); | ||
| let mut rng = ark_std::rand::thread_rng(); | ||
|
|
||
| let usk = keygen( | ||
| &mut rng, | ||
| gid_cstr.to_str().unwrap(), | ||
| &msk, | ||
| &user_attrs, | ||
| &iota, | ||
| ); | ||
|
|
||
| let mut usk_bytes = Vec::new(); | ||
| usk.serialize_compressed(&mut usk_bytes).unwrap(); | ||
|
|
||
| let usk_b64 = general_purpose::STANDARD.encode(&usk_bytes); | ||
| CString::new(usk_b64).unwrap().into_raw() | ||
| } | ||
|
|
||
| #[derive(Serialize, Deserialize)] | ||
| struct EncryptOutput { | ||
| gt: String, | ||
| ciphertext: String, | ||
| } | ||
|
|
||
| #[allow(clippy::missing_safety_doc)] | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn encrypt_abe4( | ||
| mpk_b64: *const c_char, | ||
| policy_str: *const c_char, | ||
| ) -> *mut c_char { | ||
| let mpk_b64_cstr = unsafe { CStr::from_ptr(mpk_b64) }; | ||
| let policy_cstr = unsafe { CStr::from_ptr(policy_str) }; | ||
|
|
||
| let mpk_bytes = general_purpose::STANDARD | ||
| .decode(mpk_b64_cstr.to_str().unwrap()) | ||
| .unwrap(); | ||
| let mpk: MPK = MPK::deserialize_compressed(&mpk_bytes[..]).unwrap(); | ||
|
|
||
| let policy = Policy::parse(policy_cstr.to_str().unwrap()).unwrap(); | ||
| let tau = Tau::new(&policy); | ||
| let mut rng = ark_std::rand::thread_rng(); | ||
|
|
||
| let (gt, ct) = encrypt(&mut rng, &mpk, &policy, &tau); | ||
|
|
||
| let mut gt_bytes = Vec::new(); | ||
| gt.serialize_compressed(&mut gt_bytes).unwrap(); | ||
|
|
||
| let mut ct_bytes = Vec::new(); | ||
| ct.serialize_compressed(&mut ct_bytes).unwrap(); | ||
|
|
||
| let output = EncryptOutput { | ||
| gt: general_purpose::STANDARD.encode(>_bytes), | ||
| ciphertext: general_purpose::STANDARD.encode(&ct_bytes), | ||
| }; | ||
|
|
||
| let output_json = serde_json::to_string(&output).unwrap(); | ||
| CString::new(output_json).unwrap().into_raw() | ||
| } | ||
|
|
||
| #[allow(clippy::missing_safety_doc)] | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn decrypt_abe4( | ||
| usk_b64: *const c_char, | ||
| gid: *const c_char, | ||
| policy_str: *const c_char, | ||
| ct_b64: *const c_char, | ||
| ) -> *mut c_char { | ||
| let usk_b64_cstr = unsafe { CStr::from_ptr(usk_b64) }; | ||
| let gid_cstr = unsafe { CStr::from_ptr(gid) }; | ||
| let policy_cstr = unsafe { CStr::from_ptr(policy_str) }; | ||
| let ct_b64_cstr = unsafe { CStr::from_ptr(ct_b64) }; | ||
|
|
||
| let usk_bytes = general_purpose::STANDARD | ||
| .decode(usk_b64_cstr.to_str().unwrap()) | ||
| .unwrap(); | ||
| let usk: USK = USK::deserialize_compressed(&usk_bytes[..]).unwrap(); | ||
|
|
||
| let policy = Policy::parse(policy_cstr.to_str().unwrap()).unwrap(); | ||
| let tau = Tau::new(&policy); | ||
|
|
||
| let user_attrs = usk.get_user_attributes(); | ||
| let iota = Iota::new(&user_attrs); | ||
|
|
||
| let ct_bytes = general_purpose::STANDARD | ||
| .decode(ct_b64_cstr.to_str().unwrap()) | ||
| .unwrap(); | ||
| let ct: Ciphertext = Ciphertext::deserialize_compressed(&ct_bytes[..]).unwrap(); | ||
|
|
||
| let result = decrypt(&usk, gid_cstr.to_str().unwrap(), &iota, &tau, &policy, &ct); | ||
|
|
||
| match result { | ||
| Some(gt) => { | ||
| let mut gt_bytes = Vec::new(); | ||
| gt.serialize_compressed(&mut gt_bytes).unwrap(); | ||
| let gt_b64 = general_purpose::STANDARD.encode(>_bytes); | ||
| CString::new(gt_b64).unwrap().into_raw() | ||
| } | ||
| None => std::ptr::null_mut(), | ||
| } | ||
| } |
There was a problem hiding this comment.
The FFI functions in this file make extensive use of .unwrap(). A panic across an FFI boundary is undefined behavior and can crash the entire application. All unwrap() calls should be replaced with proper error handling. For example, if parsing or decoding fails, the function should return a null pointer and ideally log the error.
Here are some of the locations with .unwrap():
auths_cstr.to_str().unwrap()(line 46 and others)serde_json::from_str(...).unwrap()(line 46 and others)msk.serialize_compressed(...).unwrap()(line 53 and others)CString::new(...).unwrap()(line 64 and others)general_purpose::STANDARD.decode(...).unwrap()(line 79 and others)MSK::deserialize_compressed(...).unwrap()(line 81 and others)Policy::parse(...).unwrap()(line 124 and others)
A match or if let block should be used to handle the Result or Option returned by these functions, returning std::ptr::null_mut() on failure.
| run a commadn in `accli`. All commands and subcommands in `accli` take an | ||
| optional `--help` flag: `./scrips/accli_wrapper.sh --help`. | ||
|
|
||
| ```bash | ||
| cargo build | ||
| ``` | ||
|
|
||
| ## Code Style | ||
| ## Coding Guideleins |
There was a problem hiding this comment.
There are a few typos in this section. commadn should be command, scrips should be scripts, and Guideleins should be Guidelines.
| run a commadn in `accli`. All commands and subcommands in `accli` take an | |
| optional `--help` flag: `./scrips/accli_wrapper.sh --help`. | |
| ```bash | |
| cargo build | |
| ``` | |
| ## Code Style | |
| ## Coding Guideleins | |
| run a commadn in `accli`. All commands and subcommands in `accli` take an | |
| optional `--help` flag: `./scripts/accli_wrapper.sh --help`. | |
| ## Coding Guidelines |
No description provided.