Refactor the Alphabet & Hsm types into separate modules.

This is easy / possible now that the Hsm type implements RngCore.

Author: flihp

src/alphabet.rs

@@ -0,0 +1,79 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+use anyhow::{Context, Result};
+use rand_core::RngCore;
+use std::collections::HashSet;
+
+pub struct Alphabet {
+    chars: Vec<char>,
+}
+
+impl Default for Alphabet {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl Alphabet {
+    pub fn new() -> Self {
+        let mut chars: HashSet<char> = HashSet::new();
+        chars.extend('a'..='z');
+        chars.extend('A'..='Z');
+        chars.extend('0'..='9');
+
+        // Remove visually similar characters
+        chars = &chars - &HashSet::from(['l', 'I', '1']);
+        chars = &chars - &HashSet::from(['B', '8']);
+        chars = &chars - &HashSet::from(['O', '0']);
+
+        // We generate random passwords from this alphabet by getting a byte
+        // of random data from the HSM and using this value to pick
+        // characters from the alphabet. Our alphabet cannot be larger than
+        // the u8::MAX or it will ignore characters after the u8::MAXth.
+        assert!(usize::from(u8::MAX) > chars.len());
+
+        Alphabet {
+            chars: chars.into_iter().collect(),
+        }
+    }
+
+    pub fn get_char(&self, val: u8) -> Option<char> {
+        let len = self.chars.len() as u8;
+        // let rand = ;
+        // Avoid biasing results by ensuring the random values we use
+        // are a multiple of the length of the alphabet. If they aren't
+        // we just get another.
+        if val < u8::MAX - u8::MAX % len {
+            Some(self.chars[(val % len) as usize])
+        } else {
+            None
+        }
+    }
+
+    pub fn get_random_string<R: RngCore>(
+        &self,
+        rng: &mut R,
+        length: usize,
+    ) -> Result<String> {
+        let mut passwd = String::with_capacity(length + 1);
+        let mut byte = [0u8, 1];
+
+        for i in 0..length {
+            let char = loop {
+                rng.try_fill_bytes(&mut byte).with_context(|| {
+                    format!("failed to get byte {} for password", i)
+                })?;
+
+                if let Some(char) = self.get_char(byte[0]) {
+                    break char;
+                }
+            };
+
+            passwd.push(char);
+        }
+
+        Ok(passwd)
+    }
+}

src/bin/printer-test.rs

@@ -7,8 +7,10 @@ use std::path::PathBuf;
 use anyhow::Result;
 use clap::{Parser, Subcommand};
 use hex::ToHex;
-use oks::{backup::Share, hsm::Alphabet, secret_writer::PrinterSecretWriter};
-use rand::{thread_rng, Rng};
+use oks::{
+    alphabet::Alphabet, backup::Share, secret_writer::PrinterSecretWriter,
+};
+use rand::thread_rng;
 use zeroize::Zeroizing;
 
 #[derive(Parser)]
@@ -56,8 +58,9 @@ fn main() -> Result<()> {
             secret_writer.share(share_idx, share_count, &share)
         }
         Command::HsmPassword { length } => {
-            let password = Alphabet::new()
-                .get_random_string(|| Ok(thread_rng().gen::<u8>()), length)?;
+            let mut rng = thread_rng();
+            let password =
+                Alphabet::new().get_random_string(&mut rng, length)?;
             let password = Zeroizing::new(password);
             secret_writer.password(&password)
         }

src/hsm.rs

@@ -6,7 +6,6 @@ use anyhow::{Context, Result};
 use log::{debug, error, info};
 use pem_rfc7468::LineEnding;
 use rand_core::{impls, CryptoRng, Error as RngError, RngCore};
-use std::collections::HashSet;
 use std::{
     fs,
     io::{self, Write},
@@ -64,81 +63,11 @@ pub enum HsmError {
     NotEnoughShares,
 }
 
-pub struct Alphabet {
-    chars: Vec<char>,
-}
-
-impl Default for Alphabet {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
-impl Alphabet {
-    pub fn new() -> Self {
-        let mut chars: HashSet<char> = HashSet::new();
-        chars.extend('a'..='z');
-        chars.extend('A'..='Z');
-        chars.extend('0'..='9');
-
-        // Remove visually similar characters
-        chars = &chars - &HashSet::from(['l', 'I', '1']);
-        chars = &chars - &HashSet::from(['B', '8']);
-        chars = &chars - &HashSet::from(['O', '0']);
-
-        // We generate random passwords from this alphabet by getting a byte
-        // of random data from the HSM and using this value to pick
-        // characters from the alphabet. Our alphabet cannot be larger than
-        // the u8::MAX or it will ignore characters after the u8::MAXth.
-        assert!(usize::from(u8::MAX) > chars.len());
-
-        Alphabet {
-            chars: chars.into_iter().collect(),
-        }
-    }
-
-    pub fn get_char(&self, val: u8) -> Option<char> {
-        let len = self.chars.len() as u8;
-        // let rand = ;
-        // Avoid biasing results by ensuring the random values we use
-        // are a multiple of the length of the alphabet. If they aren't
-        // we just get another.
-        if val < u8::MAX - u8::MAX % len {
-            Some(self.chars[(val % len) as usize])
-        } else {
-            None
-        }
-    }
-
-    pub fn get_random_string(
-        &self,
-        get_rand_u8: impl Fn() -> Result<u8>,
-        length: usize,
-    ) -> Result<String> {
-        let mut passwd = String::with_capacity(length + 1);
-
-        for _ in 0..length {
-            let char = loop {
-                let rand = get_rand_u8()?;
-
-                if let Some(char) = self.get_char(rand) {
-                    break char;
-                }
-            };
-
-            passwd.push(char);
-        }
-
-        Ok(passwd)
-    }
-}
-
 /// Structure holding common data used by OKS when interacting with the HSM.
 pub struct Hsm {
     pub client: Client,
     pub out_dir: PathBuf,
     pub state_dir: PathBuf,
-    pub alphabet: Alphabet,
     pub backup: bool,
 }
 
@@ -179,18 +108,10 @@ impl Hsm {
             client,
             out_dir: out_dir.to_path_buf(),
             state_dir: state_dir.to_path_buf(),
-            alphabet: Alphabet::new(),
             backup,
         })
     }
 
-    pub fn rand_string(&self, length: usize) -> Result<String> {
-        self.alphabet.get_random_string(
-            || Ok(self.client.get_pseudo_random(1)?[0]),
-            length,
-        )
-    }
-
     /// Create a new wrap key, cut it up into shares, & a Feldman verifier,
     /// then put the key into the YubiHSM. The shares and the verifier are then
     /// returned to the caller. Generally they will then be distributed

src/lib.rs

@@ -2,6 +2,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+pub mod alphabet;
 pub mod backup;
 pub mod ca;
 pub mod config;

src/main.rs

@@ -17,6 +17,7 @@ use yubihsm::object::{Id, Type};
 use zeroize::Zeroizing;
 
 use oks::{
+    alphabet::Alphabet,
     backup::{BackupKey, Share, Verifier, LIMIT, THRESHOLD},
     ca::{Ca, CertOrCsr},
     config::{
@@ -259,7 +260,7 @@ fn get_passwd(
 }
 
 /// get a new password from the environment or by issuing a challenge the user
-fn get_new_passwd(hsm: Option<&Hsm>) -> Result<Zeroizing<String>> {
+fn get_new_passwd(hsm: Option<&mut Hsm>) -> Result<Zeroizing<String>> {
     let passwd = match env::var(ENV_NEW_PASSWORD).ok() {
         // prefer new password from env above all else
         Some(s) => {
@@ -270,7 +271,10 @@ fn get_new_passwd(hsm: Option<&Hsm>) -> Result<Zeroizing<String>> {
             // use the HSM otherwise if available
             Some(hsm) => {
                 info!("Generating random password");
-                Zeroizing::new(hsm.rand_string(GEN_PASSWD_LENGTH)?)
+                let alpha = Alphabet::default();
+                let password =
+                    alpha.get_random_string(&mut *hsm, GEN_PASSWD_LENGTH)?;
+                Zeroizing::new(password)
             }
             // last option: challenge the caller
             None => {
@@ -364,7 +368,7 @@ fn do_ceremony<P: AsRef<Path>>(
         let passwd = if challenge {
             get_new_passwd(None)?
         } else {
-            get_new_passwd(Some(&hsm))?
+            get_new_passwd(Some(&mut hsm))?
         };
 
         secret_writer.password(&passwd)?;
@@ -759,7 +763,7 @@ fn main() -> Result<()> {
                     let passwd_new = if passwd_challenge {
                         get_new_passwd(None)?
                     } else {
-                        get_new_passwd(Some(&hsm))?
+                        get_new_passwd(Some(&mut hsm))?
                     };
 
                     let secret_writer =