Limit secret() call to pub(crate) visibility, add wrapper fn for the functionality in CaSec.

Shift database transactions from ca_secret.rs to ca.rs

6 files changed, 94 insertions, 84 deletions

diff --git a/src/bin.rs b/src/bin.rs
index 1ba0c6d..338a6f5 100644
--- a/src/bin.rs
+++ b/src/bin.rs
@@ -94,18 +94,18 @@ fn main() -> Result<()> {
         },
         Command::Ca { cmd } => match cmd {
             CaCommand::Init { domain, name } => {
-                ca.secret().ca_init(&domain, name.as_deref())?;
+                ca.ca_init(&domain, name.as_deref())?;
             }
             CaCommand::Export => {
                 println!("{}", ca.ca_get_pubkey_armored()?);
             }
             CaCommand::Revocations { output } => {
-                ca.secret().ca_generate_revocations(output)?;
+                ca.ca_generate_revocations(output)?;
                 println!("Wrote a set of revocations to the output file");
             }
             CaCommand::ImportTsig { cert_file } => {
                 let cert = std::fs::read_to_string(cert_file)?;
-                ca.secret().ca_import_tsig(&cert)?;
+                ca.ca_import_tsig(&cert)?;
             }
             CaCommand::Show => ca.ca_show()?,
         },
diff --git a/src/ca.rs b/src/ca.rs
index 20d8bf2..42db47d 100644
--- a/src/ca.rs
+++ b/src/ca.rs
@@ -22,7 +22,7 @@
 //! let openpgp_ca = OpenpgpCa::new(Some(db_filename)).expect("Failed to set up CA");
 //!
 //! // initialize the CA Admin (with domainname and a symbolic name)
-//! openpgp_ca.secret().ca_init("example.org", Some("Example Org OpenPGP CA Key")).unwrap();
+//! openpgp_ca.ca_init("example.org", Some("Example Org OpenPGP CA Key")).unwrap();
 //!
 //! // create a new user, with all signatures
 //! // (the private key is printed to stdout and needs to be manually
@@ -133,10 +133,34 @@ impl OpenpgpCa {
 
     /// Get the CaSec implementation to run operations that need CA
     /// private key material.
-    pub fn secret(&self) -> &Rc<dyn CaSec> {
+    pub(crate) fn secret(&self) -> &Rc<dyn CaSec> {
         &self.ca_secret
     }
 
+    pub fn ca_init(&self, domainname: &str, name: Option<&str>) -> Result<()> {
+        self.db()
+            .transaction(|| self.ca_secret.ca_init(domainname, name))
+    }
+
+    /// Getting private key material is not possible for all backends and
+    /// may result in panics.
+    ///
+    /// This fn is only intended for internal integration tests!\
+    ///
+    /// FIXME: different visibility?
+    pub fn ca_get_priv_key(&self) -> Result<Cert> {
+        self.ca_secret.ca_get_priv_key()
+    }
+
+    pub fn ca_generate_revocations(&self, output: PathBuf) -> Result<()> {
+        self.ca_secret.ca_generate_revocations(output)
+    }
+
+    pub fn ca_import_tsig(&self, cert: &str) -> Result<()> {
+        self.db()
+            .transaction(|| self.ca_secret.ca_import_tsig(cert))
+    }
+
     /// Get the Cert of the CA (without private key material).
     pub fn ca_get_cert_pub(&self) -> Result<Cert> {
         self.ca_public.ca_get_cert_pub()
diff --git a/src/ca_secret.rs b/src/ca_secret.rs
index 12fc570..85b045b 100644
--- a/src/ca_secret.rs
+++ b/src/ca_secret.rs
@@ -126,13 +126,11 @@ impl CaSec for DbCa {
 
         let ca_key = &Pgp::cert_to_armored_private_key(&cert)?;
 
-        self.db().transaction(|| {
-            self.db().ca_insert(
-                models::NewCa { domainname },
-                ca_key,
-                &cert.fingerprint().to_hex(),
-            )
-        })
+        self.db().ca_insert(
+            models::NewCa { domainname },
+            ca_key,
+            &cert.fingerprint().to_hex(),
+        )
     }
 
     fn ca_generate_revocations(&self, output: PathBuf) -> Result<()> {
@@ -239,42 +237,40 @@ adversaries."#;
     /// signatures from third parties. Take those third party trust
     /// signatures and merge them into our local copy of the CA key.
     fn ca_import_tsig(&self, cert: &str) -> Result<()> {
-        self.db().transaction(|| {
-            let ca_cert = self.ca_get_priv_key()?;
+        let ca_cert = self.ca_get_priv_key()?;
 
-            let cert_import = Pgp::armored_to_cert(cert)?;
+        let cert_import = Pgp::armored_to_cert(cert)?;
 
-            // The imported cert must have the same Fingerprint as the CA cert
-            if ca_cert.fingerprint() != cert_import.fingerprint() {
-                return Err(anyhow::anyhow!(
-                    "The imported cert has an unexpected Fingerprint",
-                ));
-            }
+        // The imported cert must have the same Fingerprint as the CA cert
+        if ca_cert.fingerprint() != cert_import.fingerprint() {
+            return Err(anyhow::anyhow!(
+                "The imported cert has an unexpected Fingerprint",
+            ));
+        }
 
-            // Get the third party tsig(s) from the imported cert
-            let tsigs = Pgp::get_trust_sigs(&cert_import)?;
+        // Get the third party tsig(s) from the imported cert
+        let tsigs = Pgp::get_trust_sigs(&cert_import)?;
 
-            // add tsig(s) to our "own" version of the CA key
-            let mut packets: Vec<Packet> = Vec::new();
-            tsigs.iter().for_each(|s| packets.push(s.clone().into()));
+        // add tsig(s) to our "own" version of the CA key
+        let mut packets: Vec<Packet> = Vec::new();
+        tsigs.iter().for_each(|s| packets.push(s.clone().into()));
 
-            let signed = ca_cert
-                .insert_packets(packets)
-                .context("Merging tsigs into CA Key failed")?;
+        let signed = ca_cert
+            .insert_packets(packets)
+            .context("Merging tsigs into CA Key failed")?;
 
-            // update in DB
-            let (_, mut cacert) = self
-                .db()
-                .get_ca()
-                .context("Failed to load CA cert from database")?;
+        // update in DB
+        let (_, mut cacert) = self
+            .db()
+            .get_ca()
+            .context("Failed to load CA cert from database")?;
 
-            cacert.priv_cert = Pgp::cert_to_armored_private_key(&signed)
-                .context("Failed to re-armor CA Cert")?;
+        cacert.priv_cert = Pgp::cert_to_armored_private_key(&signed)
+            .context("Failed to re-armor CA Cert")?;
 
-            self.db()
-                .cacert_update(&cacert)
-                .context("Update of CA Cert in DB failed")
-        })
+        self.db()
+            .cacert_update(&cacert)
+            .context("Update of CA Cert in DB failed")
     }
 
     fn sign_detached(&self, text: &str) -> Result<String> {
diff --git a/tests/test_gpg.rs b/tests/test_gpg.rs
index 1b6b767..cb63488 100644
--- a/tests/test_gpg.rs
+++ b/tests/test_gpg.rs
@@ -35,7 +35,7 @@ fn test_alice_authenticates_bob_centralized() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // make CA users
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, false, false)?;
@@ -44,7 +44,7 @@ fn test_alice_authenticates_bob_centralized() -> Result<()> {
     // ---- import keys from OpenPGP CA into GnuPG ----
 
     // get Cert for CA
-    let ca_cert = ca.secret().ca_get_priv_key()?;
+    let ca_cert = ca.ca_get_priv_key()?;
 
     // import CA key into GnuPG
     let mut buf = Vec::new();
@@ -115,7 +115,7 @@ fn test_alice_authenticates_bob_decentralized() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     let ca_key = ca.ca_get_pubkey_armored()?;
 
@@ -124,7 +124,7 @@ fn test_alice_authenticates_bob_decentralized() -> Result<()> {
     gnupg::import(&ctx_bob, ca_key.as_bytes());
 
     // get Cert for CA
-    let ca_cert = ca.secret().ca_get_priv_key()?;
+    let ca_cert = ca.ca_get_priv_key()?;
 
     let ca_keyid = format!("{:X}", ca_cert.keyid());
 
@@ -140,11 +140,9 @@ fn test_alice_authenticates_bob_decentralized() -> Result<()> {
     let alice_ca_key = gnupg::export(&ctx_alice, &"openpgp-ca@example.org");
     let bob_ca_key = gnupg::export(&ctx_bob, &"openpgp-ca@example.org");
 
-    ca.secret()
-        .ca_import_tsig(&alice_ca_key)
+    ca.ca_import_tsig(&alice_ca_key)
         .context("import CA tsig from Alice failed")?;
-    ca.secret()
-        .ca_import_tsig(&bob_ca_key)
+    ca.ca_import_tsig(&bob_ca_key)
         .context("import CA tsig from Bob failed")?;
 
     // get public keys for alice and bob from their gnupg contexts
@@ -235,7 +233,7 @@ fn test_bridge() -> Result<()> {
     // ---- populate first OpenPGP CA instance ----
 
     // make new CA key
-    ca1.secret().ca_init("some.org", None)?;
+    ca1.ca_init("some.org", None)?;
 
     // make CA user
     assert!(ca1
@@ -245,7 +243,7 @@ fn test_bridge() -> Result<()> {
     // ---- populate second OpenPGP CA instance ----
 
     // make new CA key
-    ca2.secret().ca_init("other.org", None)?;
+    ca2.ca_init("other.org", None)?;
 
     // make CA user
     ca2.user_new(Some(&"Bob"), &["bob@other.org"], None, false, false)?;
@@ -371,12 +369,12 @@ fn test_multi_bridge() -> Result<()> {
 
     // ---- populate OpenPGP CA instances ----
 
-    ca1.secret().ca_init("alpha.org", None)?;
+    ca1.ca_init("alpha.org", None)?;
     ca1.user_new(Some(&"Alice"), &["alice@alpha.org"], None, false, false)?;
 
-    ca2.secret().ca_init("beta.org", None)?;
+    ca2.ca_init("beta.org", None)?;
 
-    ca3.secret().ca_init("gamma.org", None)?;
+    ca3.ca_init("gamma.org", None)?;
     ca3.user_new(Some(&"Carol"), &["carol@gamma.org"], None, false, false)?;
     ca3.user_new(Some(&"Bob"), &["bob@beta.org"], None, false, false)?;
 
@@ -496,12 +494,12 @@ fn test_scoping() -> Result<()> {
     let ca3 = OpenpgpCa::new(Some(&db3))?;
 
     // ---- populate OpenPGP CA instances ----
-    ca1.secret().ca_init("alpha.org", None)?;
+    ca1.ca_init("alpha.org", None)?;
     ca1.user_new(Some(&"Alice"), &["alice@alpha.org"], None, false, false)?;
 
-    ca2.secret().ca_init("beta.org", None)?;
+    ca2.ca_init("beta.org", None)?;
 
-    ca3.secret().ca_init("other.org", None)?;
+    ca3.ca_init("other.org", None)?;
     ca3.user_new(Some(&"Bob"), &["bob@beta.org"], None, false, false)?;
 
     // ---- set up bridges: scoped trust between alpha<->beta and beta<->gamma ---
@@ -514,19 +512,13 @@ fn test_scoping() -> Result<()> {
 
     // create unscoped trust signature from beta.org CA to other.org CA
     // ---- openpgp-ca@beta.org ---tsign---> openpgp-ca@other.org ----
-    let tsigned_ca3 = Pgp::tsign(
-        ca3.secret().ca_get_priv_key()?,
-        &ca2.secret().ca_get_priv_key()?,
-        None,
-    )?;
+    let tsigned_ca3 =
+        Pgp::tsign(ca3.ca_get_priv_key()?, &ca2.ca_get_priv_key()?, None)?;
 
     // ---- import all keys from OpenPGP CA into one GnuPG instance ----
 
     // get Cert for ca1
-    let ca1_cert = ca1
-        .secret()
-        .ca_get_priv_key()
-        .expect("failed to get CA1 cert");
+    let ca1_cert = ca1.ca_get_priv_key().expect("failed to get CA1 cert");
 
     // get Cert for ca2 from ca1 bridge
     // (this has the signed version of the ca2 pubkey)
diff --git a/tests/test_oca.rs b/tests/test_oca.rs
index b21b445..8132e68 100644
--- a/tests/test_oca.rs
+++ b/tests/test_oca.rs
@@ -41,8 +41,7 @@ fn test_ca() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret()
-        .ca_init("example.org", Some("Example Org OpenPGP CA Key"))?;
+    ca.ca_init("example.org", Some("Example Org OpenPGP CA Key"))?;
 
     // make CA user
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, false, false)?;
@@ -90,8 +89,7 @@ fn test_expiring_certification() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret()
-        .ca_init("example.org", Some("Example Org OpenPGP CA Key"))?;
+    ca.ca_init("example.org", Some("Example Org OpenPGP CA Key"))?;
 
     let ca_cert = ca.ca_get_cert_pub()?;
     let ca_fp = ca_cert.fingerprint();
@@ -171,7 +169,7 @@ fn test_update_cert_key() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // import key as new user
     gnupg::create_user(&ctx, "alice@example.org");
@@ -251,7 +249,7 @@ fn test_ca_import() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // import key as new user
     gnupg::create_user(&ctx, "alice@example.org");
@@ -313,7 +311,7 @@ fn test_ca_insert_duplicate_email() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    assert!(ca.secret().ca_init("example.org", None).is_ok());
+    assert!(ca.ca_init("example.org", None).is_ok());
 
     // make CA user
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, false, false)?;
@@ -351,7 +349,7 @@ fn test_ca_export_wkd() -> Result<()> {
 
     let ca = OpenpgpCa::new(Some(&db))?;
 
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, false, false)?;
     ca.user_new(
         Some(&"Bob"),
@@ -439,7 +437,7 @@ fn test_ca_export_wkd_sequoia() -> Result<()> {
 
     let ca = OpenpgpCa::new(Some(&db))?;
 
-    ca.secret().ca_init("sequoia-pgp.org", None)?;
+    ca.ca_init("sequoia-pgp.org", None)?;
 
     ca.cert_import_new(
         &justus_key,
@@ -482,7 +480,7 @@ fn test_ca_multiple_revocations() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // gpg: make key for Alice
     gnupg::create_user(&ctx, "Alice <alice@example.org>");
@@ -549,7 +547,7 @@ fn test_ca_signatures() -> Result<()> {
     let db = format!("{}/ca.sqlite", home_path);
 
     let ca = OpenpgpCa::new(Some(&db))?;
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // create/import alice, CA signs alice's key
     gnupg::create_user(&ctx, "alice@example.org");
@@ -619,7 +617,7 @@ fn test_apply_revocation() -> Result<()> {
     let db = format!("{}/ca.sqlite", home_path);
 
     let ca = OpenpgpCa::new(Some(&db))?;
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // make CA user
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, false, false)?;
@@ -657,10 +655,10 @@ fn test_import_signed_cert() -> Result<()> {
     let db = format!("{}/ca.sqlite", home_path);
 
     let ca = OpenpgpCa::new(Some(&db))?;
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // import CA key into GnuPG
-    let ca_cert = ca.secret().ca_get_priv_key()?;
+    let ca_cert = ca.ca_get_priv_key()?;
     let mut buf = Vec::new();
     ca_cert.as_tsk().serialize(&mut buf)?;
     gnupg::import(&ctx, &buf);
@@ -738,7 +736,7 @@ fn test_revocation_no_fingerprint() -> Result<()> {
     let ca = OpenpgpCa::new(Some(&db))?;
 
     // make new CA key
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // create Alice
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, false, false)?;
@@ -866,7 +864,7 @@ fn test_create_user_with_pw() -> Result<()> {
     let db = format!("{}/ca.sqlite", home_path);
 
     let ca = OpenpgpCa::new(Some(&db))?;
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // make CA user
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, true, false)?;
@@ -896,7 +894,7 @@ fn test_refresh() -> Result<()> {
     let db = format!("{}/ca.sqlite", home_path);
 
     let ca = OpenpgpCa::new(Some(&db))?;
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     let ca_cert = ca.ca_get_cert_pub()?;
     let ca_fp = ca_cert.fingerprint();
@@ -983,7 +981,7 @@ fn test_wkd_delist() -> Result<()> {
     let db = format!("{}/ca.sqlite", home_path);
 
     let ca = OpenpgpCa::new(Some(&db))?;
-    ca.secret().ca_init("example.org", None)?;
+    ca.ca_init("example.org", None)?;
 
     // make CA users
     ca.user_new(Some(&"Alice"), &["alice@example.org"], None, true, false)?;
diff --git a/tests/test_restd.rs b/tests/test_restd.rs
index 0578a0b..d978df6 100644
--- a/tests/test_restd.rs
+++ b/tests/test_restd.rs
@@ -238,7 +238,7 @@ async fn test_restd() {
 
     // -- init OpenPGP CA --
     let ca = OpenpgpCa::new(Some(&db)).unwrap();
-    ca.secret().ca_init("example.org", None).unwrap();
+    ca.ca_init("example.org", None).unwrap();
 
     // -- start restd --
     let abort_handle = start_restd(db);