Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: gen-cli generates device-bundle on-demand with tests and refactoring #132

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

steveej
Copy link
Contributor

@steveej steveej commented Aug 16, 2024

includes #133

@steveej steveej force-pushed the gen-cli-with-random-seed branch 2 times, most recently from a6dab1e to 19b3c59 Compare August 21, 2024 14:09
@steveej steveej changed the title [WIP] add device-bundle generation functionality with tests and some refactoring feat: gen-cli generates device-bundle on-demand with tests and refactoring Aug 21, 2024
@steveej steveej marked this pull request as ready for review August 21, 2024 14:15
@steveej steveej requested a review from zo-el as a code owner August 21, 2024 14:15
@steveej steveej force-pushed the gen-cli-with-random-seed branch 2 times, most recently from 580c1b9 to 25545cb Compare August 22, 2024 21:05
Comment on lines -70 to -72
assert_eq!(revocation_pub_key, revocation_pub_key);
assert_eq!(holoport_id, holoport_id);
assert_eq!(registration_code, registration_code);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these assertions don't give any signal as the identical binding is passed twice.

the other assertions are also questionable. in the file where i moved these tests to i added a comment as a reminder to double-check these.

async fn convert_v3_to_v2() {
let (config, _) = generate_test_hpos_config().await.unwrap();

// ensure we start with a v3 config
Copy link
Contributor Author

@steveej steveej Aug 22, 2024

Choose a reason for hiding this comment

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

this could be ensured via a matching let binding instead of an identical conversion

@steveej steveej force-pushed the gen-cli-with-random-seed branch 3 times, most recently from 3d0b535 to b82bebd Compare September 4, 2024 18:24
Copy link
Contributor Author

@steveej steveej left a comment

Choose a reason for hiding this comment

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

some remaining questions and needs a cleanup before it can be merged

Comment on lines 71 to 124
pub async fn get_seed_from_locked_device_bundle(
locked_device_bundle: &[u8],
passphrase: &str,
) -> Result<Seed, failure::Error> {
let passphrase = sodoken::BufRead::from(passphrase.as_bytes());
let unlocked_bundle =
match hc_seed_bundle::UnlockedSeedBundle::from_locked(locked_device_bundle)
.await?
.remove(0)
{
hc_seed_bundle::LockedSeedCipher::PwHash(cipher) => cipher.unlock(passphrase).await,
oth => bail!("unexpected cipher: {:?}", oth),
}?;

let seed = get_seed_from_bundle(&unlocked_bundle)?;

Ok(seed)
}

/// unlock seed_bundles to access the pub-key and seed
pub async fn unlock(device_bundle: &str, passphrase: &str) -> SeedExplorerResult<SigningKey> {
if device_bundle.is_empty() {
return Err(SeedExplorerError::Generic(
"called with device_bundle".into(),
));
}

let cipher = base64::decode_config(device_bundle, base64::URL_SAFE_NO_PAD)?;
match UnlockedSeedBundle::from_locked(&cipher).await?.remove(0) {
LockedSeedCipher::PwHash(cipher) => {
let passphrase = sodoken::BufRead::from(passphrase.as_bytes().to_vec());
let seed = cipher.unlock(passphrase).await?;

let seed_bytes: [u8; 32] = match (&*seed.get_seed().read_lock())[0..32].try_into() {
Ok(b) => b,
Err(_) => {
return Err(SeedExplorerError::Generic(
"Seed buffer is not 32 bytes long".into(),
))
}
};

Ok(SigningKey::from_bytes(&seed_bytes))
}
_ => Err(SeedExplorerError::UnsupportedCipher),
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIU these two are almost equivalent with two exceptions

  • different return type
  • unlock assumes a base64 encoded bundle

// TODO: what should this be?
pub const DEFAULT_DERIVATION_PATH_V3: u32 = 3;

pub fn get_seed_from_bundle(device_bundle: &UnlockedSeedBundle) -> Result<Seed, failure::Error> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think there's no use-case for this considering fn unlock also gets the seed from the locked bundle

Comment on lines +68 to +70
ConfigDiscriminants::V1 => 2,
ConfigDiscriminants::V2 => 2,
ConfigDiscriminants::V3 => 2,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i really don't know what we need or want these to be

@JettTech @mattgeddes

any insight?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant