From 9dd4f3ab56548e1005201c8d3d3072389b5e0932 Mon Sep 17 00:00:00 2001
From: Nora Widdecke <nora@sequoia-pgp.org>
Date: Mon, 24 Oct 2022 23:06:16 +0200
Subject: [PATCH] opgpcard: Make the KeySlots type safe

---
 tools/src/bin/opgpcard/cli.rs  | 54 +++++++++++++++++++++++++++++-----
 tools/src/bin/opgpcard/main.rs | 37 ++++++++---------------
 2 files changed, 60 insertions(+), 31 deletions(-)

diff --git a/tools/src/bin/opgpcard/cli.rs b/tools/src/bin/opgpcard/cli.rs
index cf594ea..fe8d191 100644
--- a/tools/src/bin/opgpcard/cli.rs
+++ b/tools/src/bin/opgpcard/cli.rs
@@ -1,7 +1,7 @@
 // SPDX-FileCopyrightText: 2021-2022 Heiko Schaefer <heiko@schaefer.name>
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use clap::{AppSettings, Parser};
+use clap::{AppSettings, Parser, ValueEnum};
 use std::path::PathBuf;
 
 use crate::{OutputFormat, OutputVersion};
@@ -204,8 +204,8 @@ pub enum AdminCommand {
 
     /// Set touch policy
     Touch {
-        #[clap(name = "Key slot (SIG|DEC|AUT|ATT)", short = 'k', long = "key")]
-        key: String,
+        #[clap(name = "Key slot", short = 'k', long = "key", value_enum)]
+        key: BasePlusAttKeySlot,
 
         #[clap(
             name = "Policy (Off|On|Fixed|Cached|Cached-Fixed)",
@@ -277,8 +277,8 @@ pub enum AttCommand {
         #[clap(name = "card ident", short = 'c', long = "card")]
         ident: String,
 
-        #[clap(name = "Key slot (SIG|DEC|AUT)", short = 'k', long = "key")]
-        key: String,
+        #[clap(name = "Key slot", short = 'k', long = "key", value_enum)]
+        key: BaseKeySlot,
 
         #[clap(name = "User PIN file", short = 'p', long = "user-pin")]
         user_pin: Option<PathBuf>,
@@ -290,7 +290,47 @@ pub enum AttCommand {
         #[clap(name = "card ident", short = 'c', long = "card")]
         ident: Option<String>,
 
-        #[clap(name = "Key slot (SIG|DEC|AUT)", short = 'k', long = "key")]
-        key: String,
+        #[clap(name = "Key slot", short = 'k', long = "key", value_enum)]
+        key: BaseKeySlot,
     },
 }
+
+#[derive(ValueEnum, Debug, Clone)]
+#[clap(rename_all = "UPPER")]
+pub enum BaseKeySlot {
+    Sig,
+    Dec,
+    Aut,
+}
+
+impl From<BaseKeySlot> for openpgp_card_sequoia::types::KeyType {
+    fn from(ks: BaseKeySlot) -> Self {
+        use openpgp_card_sequoia::types::KeyType;
+        match ks {
+            BaseKeySlot::Sig => KeyType::Signing,
+            BaseKeySlot::Dec => KeyType::Decryption,
+            BaseKeySlot::Aut => KeyType::Authentication,
+        }
+    }
+}
+
+#[derive(ValueEnum, Debug, Clone)]
+#[clap(rename_all = "UPPER")]
+pub enum BasePlusAttKeySlot {
+    Sig,
+    Dec,
+    Aut,
+    Att,
+}
+
+impl From<BasePlusAttKeySlot> for openpgp_card_sequoia::types::KeyType {
+    fn from(ks: BasePlusAttKeySlot) -> Self {
+        use openpgp_card_sequoia::types::KeyType;
+        match ks {
+            BasePlusAttKeySlot::Sig => KeyType::Signing,
+            BasePlusAttKeySlot::Dec => KeyType::Decryption,
+            BasePlusAttKeySlot::Aut => KeyType::Authentication,
+            BasePlusAttKeySlot::Att => KeyType::Attestation,
+        }
+    }
+}
diff --git a/tools/src/bin/opgpcard/main.rs b/tools/src/bin/opgpcard/main.rs
index b914697..868fe71 100644
--- a/tools/src/bin/opgpcard/main.rs
+++ b/tools/src/bin/opgpcard/main.rs
@@ -3,6 +3,7 @@
 
 use anyhow::{anyhow, Result};
 use clap::Parser;
+use cli::BaseKeySlot;
 use std::path::{Path, PathBuf};
 
 use sequoia_openpgp::cert::prelude::ValidErasedKeyAmalgamation;
@@ -128,14 +129,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 
                 let mut sign = util::verify_to_sign(&mut open, user_pin.as_deref())?;
 
-                let kt = match key.as_str() {
-                    "SIG" => KeyType::Signing,
-                    "DEC" => KeyType::Decryption,
-                    "AUT" => KeyType::Authentication,
-                    _ => {
-                        return Err(anyhow!("Unexpected Key Type {}", key).into());
-                    }
-                };
+                let kt = KeyType::from(key);
                 sign.generate_attestation(kt, &|| {
                     println!("Touch confirmation needed to generate an attestation")
                 })?;
@@ -160,13 +154,15 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                 }
 
                 // Select cardholder certificate
-                match key.as_str() {
-                    "AUT" => open.select_data(0, &[0x7F, 0x21], select_data_workaround)?,
-                    "DEC" => open.select_data(1, &[0x7F, 0x21], select_data_workaround)?,
-                    "SIG" => open.select_data(2, &[0x7F, 0x21], select_data_workaround)?,
-
-                    _ => {
-                        return Err(anyhow!("Unexpected Key Type {}", key).into());
+                match key {
+                    BaseKeySlot::Aut => {
+                        open.select_data(0, &[0x7F, 0x21], select_data_workaround)?
+                    }
+                    BaseKeySlot::Dec => {
+                        open.select_data(1, &[0x7F, 0x21], select_data_workaround)?
+                    }
+                    BaseKeySlot::Sig => {
+                        open.select_data(2, &[0x7F, 0x21], select_data_workaround)?
                     }
                 };
 
@@ -337,15 +333,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
                     )?;
                 }
                 cli::AdminCommand::Touch { key, policy } => {
-                    let kt = match key.as_str() {
-                        "SIG" => KeyType::Signing,
-                        "DEC" => KeyType::Decryption,
-                        "AUT" => KeyType::Authentication,
-                        "ATT" => KeyType::Attestation,
-                        _ => {
-                            return Err(anyhow!("Unexpected Key Type {}", key).into());
-                        }
-                    };
+                    let kt = KeyType::from(key);
+
                     let pol = match policy.as_str() {
                         "Off" => TouchPolicy::Off,
                         "On" => TouchPolicy::On,