From e5b9a31fbd68d7674341a8f7623085e4aceb7421 Mon Sep 17 00:00:00 2001 From: Max Howell Date: Tue, 26 Nov 2024 12:31:33 -0500 Subject: [PATCH 1/3] Store private key in keychain We store it so it can only be accessed by our developer ID. --- README.md | 46 ++++++------ src/config.rs | 34 +++++---- src/keychain.rs | 181 ++++++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 1 + 4 files changed, 219 insertions(+), 43 deletions(-) create mode 100644 src/keychain.rs diff --git a/README.md b/README.md index 6fa5368..b085945 100644 --- a/README.md +++ b/README.md @@ -1,14 +1,16 @@ -# boats's personal barricade - pkgx updates +# boats's personal barricade This is a tool to automatically sign git commits, replacing gpg for that purpose. It is very opinionated, and only useful if you use gpg the same way I do. -## Updates +## `pkgx` Updates -updated to edition 2021 by pkgx +* Updated to edition 2021 by pkgx +* Stores the private key in the macOS keychain such that only this tool (when + codesigned) can access it. -## How to install +## How to Install ```sh git clone https://github.com/pkgxdev/bpb-pkgx @@ -16,7 +18,7 @@ cd bpb-pkgx cargo install --path . ``` -## How to set up +## How to Set Up Once you've installed this program, you should run the `bpb init` subcommand. This command expects you to pass a userid argument. For example, this is how I @@ -29,11 +31,13 @@ bpb init "withoutboats " You can pass any string you want as your userid, but `"$NAME <$EMAIL>"` is the conventional standard for OpenPGP userids. -This will create a file at ~/.bpb_keys.toml. This file contains your bpb public -and private keys. +This will create a file at ~/.bpb_keys.toml. This file contains your public +key. -It also prints your public key in OpenPGP format, so that you can upload it -again. You can print your public key more times with: +The private and public keys are output as JSON. This is the only time this +tool will expose your private key publicly. + +You can print your public key more times with: ```sh bpb print @@ -43,27 +47,19 @@ If you want to use it to sign git commits, you also need to inform git to call it instead of gpg. You can do this with this command: ```sh -git config --global gpg.program bpb +git config --global gpg.program bpb_pkgx ``` You should also provide the public key to people who want to verify your -commits. Personally, I just upload the public key to GitHub; you may have other -requirements. +commits. Personally, I just upload the public key to GitHub; you may have +other requirements. -## How it replaces gpg +## How it Replaces GPG -If this program receives a `-s` argument, it reads from stdin and then writes a -signature to stdout. If it receives any arguments it doesn't recognize, it +If this program receives a `-s` argument, it reads from stdin and then writes +a signature to stdout. If it receives any arguments it doesn't recognize, it delegates to the gpg binary in your path. This means that this program can be used to replace gpg as a signing tool, but -it does not replace any other functionality. For example, if you want to verify -the signatures on other peoples' git commits, it will shell out to gpg. - -## Storing your private key - -By default, your private key is stored as a hex string in `~/.bpb_keys.toml`. -However, if you are uncomfortable with the possibility of someone reading your -private key from your home directory, you can instead store it somewhere else. -To do this, replace the `key` field with a `program` field, and `bpb` will run -this program, expecting it to print your key to stdout. +it does not replace any other functionality. For example, if you want to +verify the signatures on other peoples' git commits, it will shell out to gpg. diff --git a/src/config.rs b/src/config.rs index e05742e..10fa57d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,6 +4,9 @@ use failure::Error; use crate::key_data::KeyData; +use crate::keychain::get_keychain_item; +use crate::keychain::add_keychain_item; + #[derive(Serialize, Deserialize)] pub struct Config { public: PublicKey, @@ -29,13 +32,17 @@ impl Config { } pub fn load(file: &mut impl Read) -> Result { - let mut buf = vec![]; - file.read_to_end(&mut buf)?; - Ok(toml::from_slice(&buf)?) + let service: &str = "xyz.tea.BASE.bpb"; + let account: &str = "example_account"; + let str = get_keychain_item(service, account).unwrap(); + Ok(toml::from_str(&str)?) } pub fn write(&self, file: &mut impl Write) -> Result<(), Error> { - Ok(file.write_all(&toml::to_vec(self)?)?) + let secret = toml::to_string(self)?; + let service = "xyz.tea.BASE.bpb"; + let account = "example_account"; //self.user_id(); + add_keychain_item(service, account, &secret) } pub fn timestamp(&self) -> u64 { @@ -46,10 +53,6 @@ impl Config { &self.public.userid } - // pub fn public(&self) -> &str { - // &self.public.key - // } - pub fn secret(&self) -> Result<[u8; 32], Error> { self.secret.secret() } @@ -70,16 +73,11 @@ struct SecretKey { impl SecretKey { fn secret(&self) -> Result<[u8; 32], Error> { - if let Some(key) = &self.key { - to_32_bytes(key) - } else if let Some(cmd) = &self.program { - let mut args = cmd.split_whitespace(); - let cmd = args.next().ok_or(failure::err_msg("Missing command"))?; - let output = std::process::Command::new(cmd).args(args).output().unwrap(); - to_32_bytes(&String::from_utf8(output.stdout)?) - } else { - bail!("No secret key or program specified") - } + if let Some(key) = &self.key { + to_32_bytes(key) + } else { + bail!("No secret key or program specified") + } } } diff --git a/src/keychain.rs b/src/keychain.rs new file mode 100644 index 0000000..e5a9984 --- /dev/null +++ b/src/keychain.rs @@ -0,0 +1,181 @@ +use std::ffi::{CString, c_void}; +use failure::Error; +use std::ptr; + +#[link(name = "Security", kind = "framework")] +extern "C" { + fn SecItemAdd(attributes: *const c_void, result: *mut *const c_void) -> i32; + fn SecItemCopyMatching(query: *const c_void, result: *mut *const c_void) -> i32; + fn SecItemDelete(query: *const c_void) -> i32; + + static kSecClass: *const c_void; + static kSecClassGenericPassword: *const c_void; + static kSecAttrService: *const c_void; + static kSecAttrAccount: *const c_void; + static kSecValueData: *const c_void; + static kSecReturnData: *const c_void; + static kCFBooleanTrue: *const c_void; + static kSecAttrAccessGroup: *const c_void; +} + +const ERR_SUCCESS: i32 = 0; + +pub fn add_keychain_item(service: &str, account: &str, secret: &str) -> Result<(), Error> { + let service = CString::new(service).unwrap(); + let account = CString::new(account).unwrap(); + let secret_bytes = secret.as_bytes(); + + let cf_service = unsafe { + CFStringCreateWithCString(ptr::null(), service.as_ptr(), 0x08000100) // UTF-8 encoding + }; + let cf_account = unsafe { + CFStringCreateWithCString(ptr::null(), account.as_ptr(), 0x08000100) // UTF-8 encoding + }; + let cf_data = unsafe { CFDataCreate(ptr::null(), secret_bytes.as_ptr(), secret_bytes.len()) }; + let cf_access_group = unsafe { + CFStringCreateWithCString( + ptr::null(), + "7WV56FL599.keychain_tool".as_ptr() as *const i8, // Fixed pointer type + 0x08000100, + ) + }; + + assert!(!cf_service.is_null(), "Failed to create CFString for service"); + assert!(!cf_account.is_null(), "Failed to create CFString for account"); + assert!(!cf_data.is_null(), "Failed to create CFData for secret"); + assert!(!cf_access_group.is_null(), "Failed to create CFString for access group"); + + let attributes = vec![ + (unsafe { kSecClass }, unsafe { kSecClassGenericPassword }), + (unsafe { kSecAttrService }, cf_service), + (unsafe { kSecAttrAccount }, cf_account), + (unsafe { kSecValueData }, cf_data), + (unsafe { kSecAttrAccessGroup }, cf_access_group), // Added access group + ]; + + let attributes_ptr = attributes_to_dict(&attributes); + assert!(!attributes_ptr.is_null(), "CFDictionary creation failed!"); + + unsafe { + let mut result: *mut c_void = ptr::null_mut(); + let status = SecItemAdd(attributes_ptr, &mut result as *mut *mut c_void as *mut *const c_void); + + CFRelease(attributes_ptr); // Release dictionary + CFRelease(cf_service); // Release service string + CFRelease(cf_account); // Release account string + CFRelease(cf_data); // Release data + CFRelease(cf_access_group); // Release access group string + + if status == ERR_SUCCESS { + Ok(()) + } else { + Err(failure::err_msg(format!("SecItemAdd failed with status: {}", status))) + } + } +} + +pub fn get_keychain_item(service: &str, account: &str) -> Result { + let service = CString::new(service).unwrap(); + let account = CString::new(account).unwrap(); + + let cf_service = unsafe { + CFStringCreateWithCString(ptr::null(), service.as_ptr() as *const i8, 0x08000100) // UTF-8 encoding + }; + let cf_account = unsafe { + CFStringCreateWithCString(ptr::null(), account.as_ptr() as *const i8, 0x08000100) // UTF-8 encoding + }; + + assert!(!cf_service.is_null(), "Failed to create CFString for service"); + assert!(!cf_account.is_null(), "Failed to create CFString for account"); + + let query = vec![ + (unsafe { kSecClass }, unsafe { kSecClassGenericPassword }), + (unsafe { kSecAttrService }, cf_service), + (unsafe { kSecAttrAccount }, cf_account), + (unsafe { kSecReturnData }, unsafe { kCFBooleanTrue }), + ]; + + let query_ptr = attributes_to_dict(&query); + assert!(!query_ptr.is_null(), "CFDictionary creation failed for query!"); + + unsafe { + let mut result: *mut c_void = ptr::null_mut(); + let status = SecItemCopyMatching(query_ptr, &mut result as *mut *mut c_void as *mut *const c_void); + + CFRelease(query_ptr); // Release query dictionary + CFRelease(cf_service); // Release service string + CFRelease(cf_account); // Release account string + + if status == ERR_SUCCESS { + assert!(!result.is_null(), "SecItemCopyMatching returned a null result"); + + // Convert the result to a Rust String + let data_ptr = CFDataGetBytePtr(result); + let data_len = CFDataGetLength(result); + let bytes = std::slice::from_raw_parts(data_ptr, data_len); + let secret = String::from_utf8_lossy(bytes).to_string(); + + CFRelease(result); // Release result data + + Ok(secret) + } else { + Err(format!("SecItemCopyMatching failed with status: {}", status)) + } + } +} + +// Helpers for Keychain Constants and Conversions + +#[link(name = "CoreFoundation", kind = "framework")] +extern "C" { + fn CFRelease(cf: *const c_void); + fn CFDataGetLength(data: *const c_void) -> usize; + fn CFDataGetBytePtr(data: *const c_void) -> *const u8; +} + +extern "C" { + fn CFDictionaryCreate( + allocator: *const c_void, + keys: *const *const c_void, + values: *const *const c_void, + count: usize, + key_callbacks: *const c_void, + value_callbacks: *const c_void, + ) -> *const c_void; + + fn CFStringCreateWithCString( + allocator: *const c_void, + cstr: *const i8, + encoding: u32, + ) -> *const c_void; + + fn CFDataCreate( + allocator: *const c_void, + bytes: *const u8, + length: usize, + ) -> *const c_void; +} + +fn attributes_to_dict(attrs: &[(/* key: */ *const c_void, /* value: */ *const c_void)]) -> *const c_void { + let keys: Vec<*const c_void> = attrs.iter().map(|(key, _)| *key).collect(); + let values: Vec<*const c_void> = attrs.iter().map(|(_, value)| *value).collect(); + + unsafe { + CFDictionaryCreate( + ptr::null(), + keys.as_ptr(), + values.as_ptr(), + attrs.len(), + ptr::null(), + ptr::null(), + ) + } +} + +fn CFDataToString(data: *const c_void) -> String { + unsafe { + let length = CFDataGetLength(data); + let bytes = CFDataGetBytePtr(data); + String::from_utf8(Vec::from_raw_parts(bytes as *mut u8, length, length)).unwrap() + } +} diff --git a/src/main.rs b/src/main.rs index 36d41d7..ccbdffe 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,6 +5,7 @@ extern crate serde_derive; mod config; mod key_data; +mod keychain; mod tests; use std::time::SystemTime; From adca3221c3937c07942fe0dff0d39977f336eea3 Mon Sep 17 00:00:00 2001 From: Max Howell Date: Tue, 26 Nov 2024 13:21:35 -0500 Subject: [PATCH 2/3] Add `upgrade` command to convert file to keychain --- src/config.rs | 18 ++++++++++++------ src/keychain.rs | 4 ++-- src/main.rs | 19 +++++++++++++------ 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/config.rs b/src/config.rs index 10fa57d..d2608fb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -31,14 +31,20 @@ impl Config { }) } - pub fn load(file: &mut impl Read) -> Result { - let service: &str = "xyz.tea.BASE.bpb"; - let account: &str = "example_account"; - let str = get_keychain_item(service, account).unwrap(); - Ok(toml::from_str(&str)?) + pub fn legacy_load(file: &mut impl Read) -> Result { + let mut buf = vec![]; + file.read_to_end(&mut buf)?; + Ok(toml::from_slice(&buf)?) } - pub fn write(&self, file: &mut impl Write) -> Result<(), Error> { + pub fn load() -> Result { + let service = "xyz.tea.BASE.bpb"; + let account = "example_account"; + let str = get_keychain_item(service, account)?; + Ok(toml::from_str::(&str)?) + } + + pub fn write(&self) -> Result<(), Error> { let secret = toml::to_string(self)?; let service = "xyz.tea.BASE.bpb"; let account = "example_account"; //self.user_id(); diff --git a/src/keychain.rs b/src/keychain.rs index e5a9984..43e46e9 100644 --- a/src/keychain.rs +++ b/src/keychain.rs @@ -74,7 +74,7 @@ pub fn add_keychain_item(service: &str, account: &str, secret: &str) -> Result<( } } -pub fn get_keychain_item(service: &str, account: &str) -> Result { +pub fn get_keychain_item(service: &str, account: &str) -> Result { let service = CString::new(service).unwrap(); let account = CString::new(account).unwrap(); @@ -119,7 +119,7 @@ pub fn get_keychain_item(service: &str, account: &str) -> Result Ok(secret) } else { - Err(format!("SecItemCopyMatching failed with status: {}", status)) + Err(failure::err_msg(format!("SecItemCopyMatching failed with status: {}", status))) } } } diff --git a/src/main.rs b/src/main.rs index ccbdffe..d3285ff 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,9 +9,11 @@ mod keychain; mod tests; use std::time::SystemTime; +use std::fs; use ed25519_dalek as ed25519; use failure::Error; +use keychain::add_keychain_item; use rand::RngCore; use crate::config::Config; @@ -27,6 +29,7 @@ fn main() -> Result<(), Error> { bail!("Must specify a userid argument, e.g.: `bpb init \"username \"`") } } + Some("upgrade") => upgrade(), Some("print") => print_public_key(), Some("--help") => print_help_message(), Some(arg) if gpg_sign_arg(arg) => verify_commit(), @@ -73,15 +76,13 @@ fn generate_keypair(userid: String) -> Result<(), Error> { let key_data = KeyData::create(keypair, userid, timestamp); let config = Config::create(&key_data)?; - let mut file = std::fs::File::create(keys_file)?; - config.write(&mut file)?; + config.write()?; println!("{}", key_data.public()); Ok(()) } fn print_public_key() -> Result<(), Error> { - let mut file = std::fs::File::open(keys_file())?; - let config = Config::load(&mut file)?; + let config = Config::load()?; let keypair = KeyData::load(&config)?; println!("{}", keypair.public()); Ok(()) @@ -94,8 +95,7 @@ fn verify_commit() -> Result<(), Error> { let mut stdin = std::io::stdin(); stdin.read_to_string(&mut commit)?; - let mut file = std::fs::File::open(keys_file())?; - let config = Config::load(&mut file)?; + let config = Config::load()?; let keypair = KeyData::load(&config)?; let sig = keypair.sign(commit.as_bytes())?; @@ -114,6 +114,13 @@ fn delegate() -> ! { process::exit(status) } +fn upgrade() -> Result<(), Error> { + let mut file = std::fs::File::open(keys_file())?; + let config = Config::legacy_load(&mut file)?; + config.write()?; + fs::remove_file(keys_file()).map_err(|e| failure::err_msg(e.to_string())) +} + fn keys_file() -> String { std::env::var("BPB_KEYS") .unwrap_or_else(|_| format!("{}/.bpb_keys.toml", std::env::var("HOME").unwrap())) From 90ebbc39d3bcdaa9d9c8baac774ebc9bf001d109 Mon Sep 17 00:00:00 2001 From: Jacob Heider Date: Tue, 26 Nov 2024 14:51:23 -0500 Subject: [PATCH 3/3] style and lint fixes, add test/lint CI --- .github/workflows/ci.yaml | 121 +++++++++++++++ Cargo.lock | 64 ++------ Cargo.toml | 4 +- README.md | 9 +- src/config.rs | 29 ++-- src/keychain.rs | 318 +++++++++++++++++++++----------------- src/main.rs | 11 +- 7 files changed, 344 insertions(+), 212 deletions(-) create mode 100644 .github/workflows/ci.yaml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..1bee0a6 --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,121 @@ +# https://github.com/BamPeers/rust-ci-github-actions-workflow + +on: + pull_request: + push: + branches: + - main + +name: CI + +concurrency: + group: ci/${{ github.event.pull_request.head.ref }} + cancel-in-progress: true + +jobs: + check: + name: Check + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + override: true + - uses: actions-rs/cargo@v1 + with: + command: check + env: + RUSTFLAGS: "-D warnings" + + fmt: + name: Rustfmt + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + override: true + - run: rustup component add rustfmt + - uses: actions-rs/cargo@v1 + with: + command: fmt + args: --all -- --check + + clippy: + name: Clippy + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + toolchain: nightly + components: clippy + override: true + - uses: actions-rs/cargo@v1 + with: + command: clippy + args: --all-features + env: + RUSTFLAGS: "-D warnings" + + markdownlint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: nosborn/github-action-markdown-cli@v3.2.0 + with: + files: . + + test: + name: Test + env: + PROJECT_NAME_UNDERSCORE: bpb_pkgx + CARGO_INCREMENTAL: 0 + RUSTFLAGS: -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort -D warnings + RUSTDOCFLAGS: -Cpanic=abort + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + override: true + - name: Cache dependencies + uses: actions/cache@v2 + env: + cache-name: cache-dependencies + with: + path: | + ~/.cargo/.crates.toml + ~/.cargo/.crates2.json + ~/.cargo/bin + ~/.cargo/registry/index + ~/.cargo/registry/cache + target + key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('Cargo.lock') }} + - uses: actions-rs/cargo@v1 + with: + command: test + args: --all-features + + # coverage: + # name: Coverage + # runs-on: macos-latest + # steps: + # - uses: actions/checkout@v4 + # - uses: actions-rs/toolchain@v1 + # with: + # profile: minimal + # toolchain: nightly + # override: true + # - name: Generate test result and coverage report + # run: | + # cargo install cargo-tarpaulin + # cargo tarpaulin --engine ptrace -o lcov --output-dir coverage --coveralls $COVERALLS_TOKEN + # env: + # COVERALLS_TOKEN: ${{ secrets.COVERALLS_TOKEN }} diff --git a/Cargo.lock b/Cargo.lock index ff14684..2885445 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -164,7 +164,7 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f46882e17999c6cc590af592290432be3bce0428cb0d5f8b6715e4dc7b383eb3" dependencies = [ - "proc-macro2 1.0.92", + "proc-macro2", "quote 1.0.37", "syn 2.0.89", ] @@ -331,15 +331,6 @@ dependencies = [ "zerocopy", ] -[[package]] -name = "proc-macro2" -version = "0.4.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "effdb53b25cdad54f8f48843d67398f7ef2e14f12c1b4cb4effc549a6462a4d6" -dependencies = [ - "unicode-xid 0.1.0", -] - [[package]] name = "proc-macro2" version = "1.0.92" @@ -355,22 +346,13 @@ version = "0.3.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a6e920b65c65f10b2ae65c831a81a073a89edd28c7cce89475bff467ab4167a" -[[package]] -name = "quote" -version = "0.6.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e44651a0dc4cdd99f71c83b561e221f714912d11af1a4dff0631f923d53af035" -dependencies = [ - "proc-macro2 0.4.6", -] - [[package]] name = "quote" version = "1.0.37" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5b9d34b8991d19d98081b46eacdd8eb58c6f2b201139f7c5f643cc155a633af" dependencies = [ - "proc-macro2 1.0.92", + "proc-macro2", ] [[package]] @@ -432,19 +414,22 @@ checksum = "61697e0a1c7e512e84a621326239844a24d8207b4669b41bc18b32ea5cbf988b" [[package]] name = "serde" -version = "1.0.70" +version = "1.0.215" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c3adf19c07af6d186d91dae8927b83b0553d07ca56cbf7f2f32560455c91920" +checksum = "6513c1ad0b11a9376da888e3e0baa0077f1aed55c17f50e7b2397136129fb88f" +dependencies = [ + "serde_derive", +] [[package]] name = "serde_derive" -version = "1.0.70" +version = "1.0.215" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3525a779832b08693031b8ecfb0de81cd71cfd3812088fafe9a7496789572124" +checksum = "ad1e866f866923f252f05c889987993144fb74e722403468a4ebd70c3cd756c0" dependencies = [ - "proc-macro2 0.4.6", - "quote 0.6.3", - "syn 0.14.4", + "proc-macro2", + "quote 1.0.37", + "syn 2.0.89", ] [[package]] @@ -509,18 +494,7 @@ checksum = "d3b891b9015c88c576343b9b3e41c2c11a51c219ef067b264bd9c8aa9b441dad" dependencies = [ "quote 0.3.15", "synom", - "unicode-xid 0.0.4", -] - -[[package]] -name = "syn" -version = "0.14.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2beff8ebc3658f07512a413866875adddd20f4fd47b2a4e6c9da65cd281baaea" -dependencies = [ - "proc-macro2 0.4.6", - "quote 0.6.3", - "unicode-xid 0.1.0", + "unicode-xid", ] [[package]] @@ -529,7 +503,7 @@ version = "2.0.89" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "44d46482f1c1c87acd84dea20c1bf5ebff4c757009ed6bf19cfd36fb10e92c4e" dependencies = [ - "proc-macro2 1.0.92", + "proc-macro2", "quote 1.0.37", "unicode-ident", ] @@ -540,7 +514,7 @@ version = "0.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a393066ed9010ebaed60b9eafa373d4b1baac186dd7e008555b0f702b51945b6" dependencies = [ - "unicode-xid 0.0.4", + "unicode-xid", ] [[package]] @@ -580,12 +554,6 @@ version = "0.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8c1f860d7d29cf02cb2f3f359fd35991af3d30bac52c57d265a3c461074cb4dc" -[[package]] -name = "unicode-xid" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc72304796d0818e357ead4e000d19c9c174ab23dc11093ac919054d20a6a7fc" - [[package]] name = "version_check" version = "0.9.5" @@ -636,7 +604,7 @@ version = "0.7.35" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fa4f8080344d4671fb4e831a13ad1e68092748387dfc4f55e356242fae12ce3e" dependencies = [ - "proc-macro2 1.0.92", + "proc-macro2", "quote 1.0.37", "syn 2.0.89", ] diff --git a/Cargo.toml b/Cargo.toml index a5c87cf..bbba798 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,8 +11,8 @@ authors = ["Without Boats ", "Jacob Heider "] toml = "0.4.6" rand = { version = "0.8.5", features = ["std"] } sha2 = "0.7.1" -serde_derive = "1.0.70" -serde = "1.0.70" +serde_derive = "1.0.215" +serde = "1.0.215" hex = "0.3.2" failure = "0.1.1" diff --git a/README.md b/README.md index b085945..0f18773 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,15 @@ do. ## `pkgx` Updates -* Updated to edition 2021 by pkgx -* Stores the private key in the macOS keychain such that only this tool (when +- Updated to edition 2021 by pkgx +- Stores the private key in the macOS keychain such that only this tool (when codesigned) can access it. +### TODO + +- [ ] Move keychain identifiers out to build variables in `config.rs` +- [ ] Move keychain identifier out to a build variable in `keychain.rs` + ## How to Install ```sh diff --git a/src/config.rs b/src/config.rs index d2608fb..735e233 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,11 +1,13 @@ -use std::io::{Read, Write}; +use std::io::Read; use failure::Error; use crate::key_data::KeyData; -use crate::keychain::get_keychain_item; -use crate::keychain::add_keychain_item; +use crate::keychain::{add_keychain_item, get_keychain_item}; + +const KEYCHAIN_SERVICE: &str = "xyz.tea.BASE.bpb"; +const KEYCHAIN_ACCOUNT: &str = "example_account"; #[derive(Serialize, Deserialize)] pub struct Config { @@ -38,17 +40,14 @@ impl Config { } pub fn load() -> Result { - let service = "xyz.tea.BASE.bpb"; - let account = "example_account"; - let str = get_keychain_item(service, account)?; - Ok(toml::from_str::(&str)?) + let str = get_keychain_item(KEYCHAIN_SERVICE, KEYCHAIN_ACCOUNT)?; + Ok(toml::from_str::(&str)?) } pub fn write(&self) -> Result<(), Error> { let secret = toml::to_string(self)?; - let service = "xyz.tea.BASE.bpb"; - let account = "example_account"; //self.user_id(); - add_keychain_item(service, account, &secret) + // let account = self.user_id(); + add_keychain_item(KEYCHAIN_SERVICE, KEYCHAIN_ACCOUNT, &secret) } pub fn timestamp(&self) -> u64 { @@ -79,11 +78,11 @@ struct SecretKey { impl SecretKey { fn secret(&self) -> Result<[u8; 32], Error> { - if let Some(key) = &self.key { - to_32_bytes(key) - } else { - bail!("No secret key or program specified") - } + if let Some(key) = &self.key { + to_32_bytes(key) + } else { + bail!("No secret key or program specified") + } } } diff --git a/src/keychain.rs b/src/keychain.rs index 43e46e9..31727c2 100644 --- a/src/keychain.rs +++ b/src/keychain.rs @@ -1,12 +1,16 @@ -use std::ffi::{CString, c_void}; use failure::Error; +use std::ffi::{c_void, CString}; use std::ptr; +// Fixed pointer type +const KEYCHAIN_ID: *const i8 = "7WV56FL599.keychain_tool".as_ptr() as *const i8; + #[link(name = "Security", kind = "framework")] extern "C" { fn SecItemAdd(attributes: *const c_void, result: *mut *const c_void) -> i32; fn SecItemCopyMatching(query: *const c_void, result: *mut *const c_void) -> i32; - fn SecItemDelete(query: *const c_void) -> i32; + // unused + // fn SecItemDelete(query: *const c_void) -> i32; static kSecClass: *const c_void; static kSecClassGenericPassword: *const c_void; @@ -21,107 +25,141 @@ extern "C" { const ERR_SUCCESS: i32 = 0; pub fn add_keychain_item(service: &str, account: &str, secret: &str) -> Result<(), Error> { - let service = CString::new(service).unwrap(); - let account = CString::new(account).unwrap(); - let secret_bytes = secret.as_bytes(); - - let cf_service = unsafe { - CFStringCreateWithCString(ptr::null(), service.as_ptr(), 0x08000100) // UTF-8 encoding - }; - let cf_account = unsafe { - CFStringCreateWithCString(ptr::null(), account.as_ptr(), 0x08000100) // UTF-8 encoding - }; - let cf_data = unsafe { CFDataCreate(ptr::null(), secret_bytes.as_ptr(), secret_bytes.len()) }; - let cf_access_group = unsafe { - CFStringCreateWithCString( - ptr::null(), - "7WV56FL599.keychain_tool".as_ptr() as *const i8, // Fixed pointer type - 0x08000100, - ) - }; - - assert!(!cf_service.is_null(), "Failed to create CFString for service"); - assert!(!cf_account.is_null(), "Failed to create CFString for account"); - assert!(!cf_data.is_null(), "Failed to create CFData for secret"); - assert!(!cf_access_group.is_null(), "Failed to create CFString for access group"); - - let attributes = vec![ - (unsafe { kSecClass }, unsafe { kSecClassGenericPassword }), - (unsafe { kSecAttrService }, cf_service), - (unsafe { kSecAttrAccount }, cf_account), - (unsafe { kSecValueData }, cf_data), - (unsafe { kSecAttrAccessGroup }, cf_access_group), // Added access group - ]; - - let attributes_ptr = attributes_to_dict(&attributes); - assert!(!attributes_ptr.is_null(), "CFDictionary creation failed!"); - - unsafe { - let mut result: *mut c_void = ptr::null_mut(); - let status = SecItemAdd(attributes_ptr, &mut result as *mut *mut c_void as *mut *const c_void); - - CFRelease(attributes_ptr); // Release dictionary - CFRelease(cf_service); // Release service string - CFRelease(cf_account); // Release account string - CFRelease(cf_data); // Release data - CFRelease(cf_access_group); // Release access group string - - if status == ERR_SUCCESS { - Ok(()) - } else { - Err(failure::err_msg(format!("SecItemAdd failed with status: {}", status))) - } - } + let service = CString::new(service).unwrap(); + let account = CString::new(account).unwrap(); + let secret_bytes = secret.as_bytes(); + + let cf_service = unsafe { + CFStringCreateWithCString(ptr::null(), service.as_ptr(), 0x08000100) // UTF-8 encoding + }; + let cf_account = unsafe { + CFStringCreateWithCString(ptr::null(), account.as_ptr(), 0x08000100) // UTF-8 encoding + }; + let cf_data = unsafe { CFDataCreate(ptr::null(), secret_bytes.as_ptr(), secret_bytes.len()) }; + let cf_access_group = + unsafe { CFStringCreateWithCString(ptr::null(), KEYCHAIN_ID, 0x08000100) }; + + assert!( + !cf_service.is_null(), + "Failed to create CFString for service" + ); + assert!( + !cf_account.is_null(), + "Failed to create CFString for account" + ); + assert!(!cf_data.is_null(), "Failed to create CFData for secret"); + assert!( + !cf_access_group.is_null(), + "Failed to create CFString for access group" + ); + + let attributes = vec![ + (unsafe { kSecClass }, unsafe { kSecClassGenericPassword }), + (unsafe { kSecAttrService }, cf_service), + (unsafe { kSecAttrAccount }, cf_account), + (unsafe { kSecValueData }, cf_data), + (unsafe { kSecAttrAccessGroup }, cf_access_group), // Added access group + ]; + + let attributes_ptr = attributes_to_dict(&attributes); + assert!(!attributes_ptr.is_null(), "CFDictionary creation failed!"); + + unsafe { + let mut result: *mut c_void = ptr::null_mut(); + let status = SecItemAdd( + attributes_ptr, + &mut result as *mut *mut c_void as *mut *const c_void, + ); + + CFRelease(attributes_ptr); // Release dictionary + CFRelease(cf_service); // Release service string + CFRelease(cf_account); // Release account string + CFRelease(cf_data); // Release data + CFRelease(cf_access_group); // Release access group string + + if status == ERR_SUCCESS { + Ok(()) + } else { + Err(failure::err_msg(format!( + "SecItemAdd failed with status: {}", + status + ))) + } + } } pub fn get_keychain_item(service: &str, account: &str) -> Result { - let service = CString::new(service).unwrap(); - let account = CString::new(account).unwrap(); - - let cf_service = unsafe { - CFStringCreateWithCString(ptr::null(), service.as_ptr() as *const i8, 0x08000100) // UTF-8 encoding - }; - let cf_account = unsafe { - CFStringCreateWithCString(ptr::null(), account.as_ptr() as *const i8, 0x08000100) // UTF-8 encoding - }; - - assert!(!cf_service.is_null(), "Failed to create CFString for service"); - assert!(!cf_account.is_null(), "Failed to create CFString for account"); - - let query = vec![ - (unsafe { kSecClass }, unsafe { kSecClassGenericPassword }), - (unsafe { kSecAttrService }, cf_service), - (unsafe { kSecAttrAccount }, cf_account), - (unsafe { kSecReturnData }, unsafe { kCFBooleanTrue }), - ]; - - let query_ptr = attributes_to_dict(&query); - assert!(!query_ptr.is_null(), "CFDictionary creation failed for query!"); - - unsafe { - let mut result: *mut c_void = ptr::null_mut(); - let status = SecItemCopyMatching(query_ptr, &mut result as *mut *mut c_void as *mut *const c_void); - - CFRelease(query_ptr); // Release query dictionary - CFRelease(cf_service); // Release service string - CFRelease(cf_account); // Release account string - - if status == ERR_SUCCESS { - assert!(!result.is_null(), "SecItemCopyMatching returned a null result"); - - // Convert the result to a Rust String - let data_ptr = CFDataGetBytePtr(result); - let data_len = CFDataGetLength(result); - let bytes = std::slice::from_raw_parts(data_ptr, data_len); - let secret = String::from_utf8_lossy(bytes).to_string(); - - CFRelease(result); // Release result data - - Ok(secret) - } else { - Err(failure::err_msg(format!("SecItemCopyMatching failed with status: {}", status))) - } - } + let service = CString::new(service).unwrap(); + let account = CString::new(account).unwrap(); + + let cf_service = unsafe { + // CString.as_prt() type is platform dependent + #[allow(clippy::unnecessary_cast)] + CFStringCreateWithCString(ptr::null(), service.as_ptr() as *const i8, 0x08000100) + // UTF-8 encoding + }; + let cf_account = unsafe { + // CString.as_prt() type is platform dependent + #[allow(clippy::unnecessary_cast)] + CFStringCreateWithCString(ptr::null(), account.as_ptr() as *const i8, 0x08000100) + // UTF-8 encoding + }; + + assert!( + !cf_service.is_null(), + "Failed to create CFString for service" + ); + assert!( + !cf_account.is_null(), + "Failed to create CFString for account" + ); + + let query = vec![ + (unsafe { kSecClass }, unsafe { kSecClassGenericPassword }), + (unsafe { kSecAttrService }, cf_service), + (unsafe { kSecAttrAccount }, cf_account), + (unsafe { kSecReturnData }, unsafe { kCFBooleanTrue }), + ]; + + let query_ptr = attributes_to_dict(&query); + assert!( + !query_ptr.is_null(), + "CFDictionary creation failed for query!" + ); + + unsafe { + let mut result: *mut c_void = ptr::null_mut(); + let status = SecItemCopyMatching( + query_ptr, + &mut result as *mut *mut c_void as *mut *const c_void, + ); + + CFRelease(query_ptr); // Release query dictionary + CFRelease(cf_service); // Release service string + CFRelease(cf_account); // Release account string + + if status == ERR_SUCCESS { + assert!( + !result.is_null(), + "SecItemCopyMatching returned a null result" + ); + + // Convert the result to a Rust String + let data_ptr = CFDataGetBytePtr(result); + let data_len = CFDataGetLength(result); + let bytes = std::slice::from_raw_parts(data_ptr, data_len); + let secret = String::from_utf8_lossy(bytes).to_string(); + + CFRelease(result); // Release result data + + Ok(secret) + } else { + Err(failure::err_msg(format!( + "SecItemCopyMatching failed with status: {}", + status + ))) + } + } } // Helpers for Keychain Constants and Conversions @@ -134,48 +172,50 @@ extern "C" { } extern "C" { - fn CFDictionaryCreate( - allocator: *const c_void, - keys: *const *const c_void, - values: *const *const c_void, - count: usize, - key_callbacks: *const c_void, - value_callbacks: *const c_void, - ) -> *const c_void; - - fn CFStringCreateWithCString( - allocator: *const c_void, - cstr: *const i8, - encoding: u32, - ) -> *const c_void; - - fn CFDataCreate( - allocator: *const c_void, - bytes: *const u8, - length: usize, - ) -> *const c_void; + fn CFDictionaryCreate( + allocator: *const c_void, + keys: *const *const c_void, + values: *const *const c_void, + count: usize, + key_callbacks: *const c_void, + value_callbacks: *const c_void, + ) -> *const c_void; + + fn CFStringCreateWithCString( + allocator: *const c_void, + cstr: *const i8, + encoding: u32, + ) -> *const c_void; + + fn CFDataCreate(allocator: *const c_void, bytes: *const u8, length: usize) -> *const c_void; } -fn attributes_to_dict(attrs: &[(/* key: */ *const c_void, /* value: */ *const c_void)]) -> *const c_void { - let keys: Vec<*const c_void> = attrs.iter().map(|(key, _)| *key).collect(); - let values: Vec<*const c_void> = attrs.iter().map(|(_, value)| *value).collect(); - - unsafe { - CFDictionaryCreate( - ptr::null(), - keys.as_ptr(), - values.as_ptr(), - attrs.len(), - ptr::null(), - ptr::null(), - ) - } -} +fn attributes_to_dict( + attrs: &[( + /* key: */ *const c_void, + /* value: */ *const c_void, + )], +) -> *const c_void { + let keys: Vec<*const c_void> = attrs.iter().map(|(key, _)| *key).collect(); + let values: Vec<*const c_void> = attrs.iter().map(|(_, value)| *value).collect(); -fn CFDataToString(data: *const c_void) -> String { unsafe { - let length = CFDataGetLength(data); - let bytes = CFDataGetBytePtr(data); - String::from_utf8(Vec::from_raw_parts(bytes as *mut u8, length, length)).unwrap() + CFDictionaryCreate( + ptr::null(), + keys.as_ptr(), + values.as_ptr(), + attrs.len(), + ptr::null(), + ptr::null(), + ) } } + +// unused +// fn CFDataToString(data: *const c_void) -> String { +// unsafe { +// let length = CFDataGetLength(data); +// let bytes = CFDataGetBytePtr(data); +// String::from_utf8(Vec::from_raw_parts(bytes as *mut u8, length, length)).unwrap() +// } +// } diff --git a/src/main.rs b/src/main.rs index d3285ff..59379ce 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,12 +8,11 @@ mod key_data; mod keychain; mod tests; -use std::time::SystemTime; use std::fs; +use std::time::SystemTime; use ed25519_dalek as ed25519; use failure::Error; -use keychain::add_keychain_item; use rand::RngCore; use crate::config::Config; @@ -115,10 +114,10 @@ fn delegate() -> ! { } fn upgrade() -> Result<(), Error> { - let mut file = std::fs::File::open(keys_file())?; - let config = Config::legacy_load(&mut file)?; - config.write()?; - fs::remove_file(keys_file()).map_err(|e| failure::err_msg(e.to_string())) + let mut file = fs::File::open(keys_file())?; + let config = Config::legacy_load(&mut file)?; + config.write()?; + fs::remove_file(keys_file()).map_err(|e| failure::err_msg(e.to_string())) } fn keys_file() -> String {