[CIT-218] Extract the device id from the device certificate (#190)

* [CIT-218] Derive the device id from the device certificate

* [CIT-218] Remove un-necssary updates of the config

* [CIT-218] Fix error message

* [CIT-218] Fix typo in help meessage

* [CIT-218] Avoid to serialize device.id

Co-authored-by: Wenzek <diw@softwareag.com>

6 files changed, 140 insertions, 68 deletions

diff --git a/tedge_config/Cargo.toml b/tedge_config/Cargo.toml
index 13a1828d..a030bfd0 100644
--- a/tedge_config/Cargo.toml
+++ b/tedge_config/Cargo.toml
@@ -5,6 +5,7 @@ authors = ["Software AG <thin-edge-team@softwareag.com>"]
 edition = "2018"
 
 [dependencies]
+certificate = { path = "../common/certificate" }
 toml = "0.5"
 thiserror = "1.0"
 serde = { version = "1.0", features = ["derive"] }
diff --git a/tedge_config/src/config_setting.rs b/tedge_config/src/config_setting.rs
index fde2c46c..3dcbfdab 100644
--- a/tedge_config/src/config_setting.rs
+++ b/tedge_config/src/config_setting.rs
@@ -63,4 +63,7 @@ pub enum ConfigSettingError {
 
     #[error("Conversion into String failed")]
     ConversionIntoStringFailed,
+
+    #[error("Derivation for `{key}` failed: {cause}")]
+    DerivationFailed { key: &'static str, cause: String },
 }
diff --git a/tedge_config/src/settings.rs b/tedge_config/src/settings.rs
index 24c16e6f..f247cfe7 100644
--- a/tedge_config/src/settings.rs
+++ b/tedge_config/src/settings.rs
@@ -21,19 +21,6 @@ impl ConfigSetting for DeviceIdSetting {
     type Value = String;
 }
 
-/// XXX: This is just here to ease migration of `tedge cert` to the new config API
-/// until the `device.id` is infered from the certificate itself.
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-pub struct WritableDeviceIdSetting;
-
-impl ConfigSetting for WritableDeviceIdSetting {
-    const KEY: &'static str = "";
-
-    const DESCRIPTION: &'static str = "FOR INTERNAL USE ONLY";
-
-    type Value = String;
-}
-
 ///
 /// Path to the private key file. Example: /home/user/.tedge/tedge-private-key.pem
 ///
diff --git a/tedge_config/src/tedge_config.rs b/tedge_config/src/tedge_config.rs
index 6d8a7af2..29f611f5 100644
--- a/tedge_config/src/tedge_config.rs
+++ b/tedge_config/src/tedge_config.rs
@@ -1,4 +1,5 @@
 use crate::*;
+use certificate::{CertificateError, PemCertificate};
 use std::convert::{TryFrom, TryInto};
 
 /// Represents the complete configuration of a thin edge device.
@@ -14,57 +15,46 @@ pub struct TEdgeConfig {
 
 impl ConfigSettingAccessor<DeviceIdSetting> for TEdgeConfig {
     fn query(&self, _setting: DeviceIdSetting) -> ConfigSettingResult<String> {
-        self.data
-            .device
-            .id
-            .clone()
-            .ok_or(ConfigSettingError::ConfigNotSet {
-                key: DeviceIdSetting::KEY,
-            })
+        let cert_path = self.query(DeviceCertPathSetting)?;
+        let pem = PemCertificate::from_pem_file(cert_path)
+            .map_err(|err| cert_error_into_config_error(DeviceIdSetting::KEY, err))?;
+        let device_id = pem
+            .subject_common_name()
+            .map_err(|err| cert_error_into_config_error(DeviceIdSetting::KEY, err))?;
+        Ok(device_id)
     }
 
     fn update(&mut self, _setting: DeviceIdSetting, _value: String) -> ConfigSettingResult<()> {
-        Err(ConfigSettingError::ReadonlySetting {
-            message: concat!(
-                "Setting the device id is only allowed with `tedge cert create`.\n",
-                "To set 'device.id', use `tedge cert create --device-id <id>`."
-            ),
-        })
+        Err(device_id_read_only_error())
     }
 
     fn unset(&mut self, _setting: DeviceIdSetting) -> ConfigSettingResult<()> {
-        Err(ConfigSettingError::ReadonlySetting {
-            message: concat!(
-                "Setting the device id is only allowed with `tedge cert create`.\n",
-                "To set 'device.id', use `tedge cert create --device-id <id>`."
-            ),
-        })
+        Err(device_id_read_only_error())
     }
 }
 
-impl ConfigSettingAccessor<WritableDeviceIdSetting> for TEdgeConfig {
-    fn query(&self, _setting: WritableDeviceIdSetting) -> ConfigSettingResult<String> {
-        self.data
-            .device
-            .id
-            .clone()
-            .ok_or(ConfigSettingError::ConfigNotSet {
-                key: WritableDeviceIdSetting::KEY,
-            })
-    }
-
-    fn update(
-        &mut self,
-        _setting: WritableDeviceIdSetting,
-        value: String,
-    ) -> ConfigSettingResult<()> {
-        self.data.device.id = Some(value);
-        Ok(())
+fn device_id_read_only_error() -> ConfigSettingError {
+    ConfigSettingError::ReadonlySetting {
+        message: concat!(
+            "The device id is read from the device certificate and cannot be set directly.\n",
+            "To set 'device.id' to some <id>, you can use `tedge cert create --device-id <id>`.",
+        ),
     }
+}
 
-    fn unset(&mut self, _setting: WritableDeviceIdSetting) -> ConfigSettingResult<()> {
-        self.data.device.id = None;
-        Ok(())
+fn cert_error_into_config_error(key: &'static str, err: CertificateError) -> ConfigSettingError {
+    match &err {
+        CertificateError::IoError(io_err) => match io_err.kind() {
+            std::io::ErrorKind::NotFound => ConfigSettingError::ConfigNotSet { key },
+            _ => ConfigSettingError::DerivationFailed {
+                key,
+                cause: format!("{}", err),
+            },
+        },
+        _ => ConfigSettingError::DerivationFailed {
+            key,
+            cause: format!("{}", err),
+        },
     }
 }
 
diff --git a/tedge_config/src/tedge_config_dto.rs b/tedge_config/src/tedge_config_dto.rs
index 2927901c..df5533b8 100644
--- a/tedge_config/src/tedge_config_dto.rs
+++ b/tedge_config/src/tedge_config_dto.rs
@@ -22,8 +22,10 @@ pub(crate) struct TEdgeConfigDto {
 #[serde(deny_unknown_fields)]
 #[derive(Debug, Default, Deserialize, Serialize)]
 pub(crate) struct DeviceConfigDto {
-    /// The unique id of the device
-    pub(crate) id: Option<String>,
+    /// The unique id of the device (DEPRECATED)
+    /// This id is now derived from the device certificate
+    #[serde(rename(deserialize = "id"), skip_serializing)]
+    pub(crate) _id: Option<String>,
 
     /// Path where the device's private key is stored.
     /// Defaults to $HOME/.tedge/tedge-private.pem
diff --git a/tedge_config/tests/test_tedge_config.rs b/tedge_config/tests/test_tedge_config.rs
index 9f7f3e74..8bf404c4 100644
--- a/tedge_config/tests/test_tedge_config.rs
+++ b/tedge_config/tests/test_tedge_config.rs
@@ -8,7 +8,6 @@ use tempfile::TempDir;
 fn test_parse_config_with_all_values() -> Result<(), TEdgeConfigError> {
     let toml_conf = r#"
 [device]
-id = "ABCD1234"
 key_path = "/path/to/key"
 cert_path = "/path/to/cert"
 
@@ -29,7 +28,7 @@ connect = "false"
     let config =
         TEdgeConfigRepository::new_with_defaults(config_location, config_defaults).load()?;
 
-    assert_eq!(config.query(DeviceIdSetting)?, "ABCD1234");
+    assert!(config.query_optional(DeviceIdSetting)?.is_none());
     assert_eq!(
         config.query(DeviceKeyPathSetting)?,
         FilePath::from("/path/to/key")
@@ -64,7 +63,6 @@ connect = "false"
 fn test_store_config() -> Result<(), TEdgeConfigError> {
     let toml_conf = r#"
 [device]
-id = "ABCD1234"
 key_path = "/path/to/key"
 cert_path = "/path/to/cert"
 
@@ -91,7 +89,7 @@ root_cert_path = "/path/to/azure/root/cert"
 
     {
         let mut config = config_repo.load()?;
-        assert_eq!(config.query(DeviceIdSetting)?, "ABCD1234");
+        assert!(config.query_optional(DeviceIdSetting)?.is_none());
         assert_eq!(
             config.query(DeviceKeyPathSetting)?,
             FilePath::from("/path/to/key")
@@ -129,7 +127,7 @@ root_cert_path = "/path/to/azure/root/cert"
     {
         let config = config_repo.load()?;
 
-        assert_eq!(config.query(DeviceIdSetting)?, "ABCD1234");
+        assert!(config.query_optional(DeviceIdSetting)?.is_none());
         assert_eq!(
             config.query(DeviceKeyPathSetting)?,
             FilePath::from("/path/to/key")
@@ -159,7 +157,6 @@ root_cert_path = "/path/to/azure/root/cert"
 fn test_parse_config_missing_c8y_configuration() -> Result<(), TEdgeConfigError> {
     let toml_conf = r#"
 [device]
-id = "ABCD1234"
 "#;
 
     let (_tempdir, config_location) = create_temp_tedge_config(toml_conf)?;
@@ -173,7 +170,7 @@ id = "ABCD1234"
     let config =
         TEdgeConfigRepository::new_with_defaults(config_location, config_defaults).load()?;
 
-    assert_eq!(config.query(DeviceIdSetting)?, "ABCD1234");
+    assert!(config.query_optional(DeviceIdSetting)?.is_none());
     assert_eq!(
         config.query(DeviceCertPathSetting)?,
         FilePath::from("/etc/ssl/certs/tedge-certificate.pem")
@@ -195,7 +192,6 @@ id = "ABCD1234"
 fn test_parse_config_missing_azure_configuration() -> Result<(), TEdgeConfigError> {
     let toml_conf = r#"
 [device]
-id = "ABCD1234"
 "#;
 
     let (_tempdir, config_location) = create_temp_tedge_config(toml_conf)?;
@@ -209,7 +205,7 @@ id = "ABCD1234"
     let config =
         TEdgeConfigRepository::new_with_defaults(config_location, config_defaults).load()?;
 
-    assert_eq!(config.query(DeviceIdSetting)?, "ABCD1234");
+    assert!(config.query_optional(DeviceIdSetting)?.is_none());
     assert_eq!(
         config.query(DeviceCertPathSetting)?,
         FilePath::from("/etc/ssl/certs/tedge-certificate.pem")
@@ -354,7 +350,6 @@ fn test_parse_invalid_toml_file() -> Result<(), TEdgeConfigError> {
 fn test_crud_config_value() -> Result<(), TEdgeConfigError> {
     let toml_conf = r#"
 [device]
-id = "ABCD1234"
 key_path = "/path/to/key"
 cert_path = "/path/to/cert"
 
@@ -377,11 +372,10 @@ root_cert_path = "/path/to/azure/root/cert"
     let mut config =
         TEdgeConfigRepository::new_with_defaults(config_location, config_defaults).load()?;
 
-    let original_device_id = "ABCD1234".to_string();
     let original_device_key_path = FilePath::from("/path/to/key");
     let original_device_cert_path = FilePath::from("/path/to/cert");
 
-    assert_eq!(config.query(DeviceIdSetting)?, original_device_id);
+    assert!(config.query_optional(DeviceIdSetting)?.is_none());
     assert_eq!(
         config.query(DeviceKeyPathSetting)?,
         original_device_key_path
@@ -426,7 +420,6 @@ root_cert_path = "/path/to/azure/root/cert"
 fn test_crud_config_value_azure() -> Result<(), TEdgeConfigError> {
     let toml_conf = r#"
 [device]
-id = "ABCD1234"
 key_path = "/path/to/key"
 cert_path = "/path/to/cert"
 
@@ -475,6 +468,88 @@ root_cert_path = "/path/to/azure/root/cert"
     Ok(())
 }
 
+#[test]
+fn test_any_device_id_provided_by_the_configuration_is_ignored() -> Result<(), TEdgeConfigError> {
+    let toml_conf = r#"
+[device]
+id = "ABCD1234"              # ignored for backward compatibility
+cert_path = "/path/to/cert"
+"#;
+
+    let (_tempdir, config_location) = create_temp_tedge_config(toml_conf)?;
+    let config =
+        TEdgeConfigRepository::new_with_defaults(config_location, dummy_tedge_config_defaults())
+            .load()?;
+
+    assert!(config.query_optional(DeviceIdSetting)?.is_none());
+    Ok(())
+}
+
+#[test]
+fn test_device_id_is_none_when_there_is_no_certificate() -> Result<(), TEdgeConfigError> {
+    let toml_conf = r#"
+[device]
+cert_path = "/path/to/cert"
+"#;
+
+    let (_tempdir, config_location) = create_temp_tedge_config(toml_conf)?;
+    let config =
+        TEdgeConfigRepository::new_with_defaults(config_location, dummy_tedge_config_defaults())
+            .load()?;
+
+    assert!(config.query_optional(DeviceIdSetting)?.is_none());
+    Ok(())
+}
+
+#[test]
+fn test_device_id_is_err_when_cert_path_is_not_a_certificate() -> Result<(), TEdgeConfigError> {
+    let toml_conf = r#"
+[device]
+cert_path = "/path/to/cert"
+"#;
+
+    let (tempdir, config_location) = create_temp_tedge_config(toml_conf)?;
+    let mut config =
+        TEdgeConfigRepository::new_with_defaults(config_location, dummy_tedge_config_defaults())
+            .load()?;
+
+    let cert_path = tempdir.path().join("not-a-certificate.pem");
+    std::fs::File::create(cert_path.clone()).expect("fail to create a fake certificate");
+    config.update(DeviceCertPathSetting, cert_path.into())?;
+
+    match config.query(DeviceIdSetting) {
+        Err(ConfigSettingError::DerivationFailed { key, cause }) => {
+            assert_eq!(key, "device.id");
+            assert_eq!(cause, "PEM file format error");
+        }
+        Err(_) => assert!(false, "unexpected error"),
+        Ok(_) => assert!(false, "unexpected ok result"),
+    }
+    Ok(())
+}
+
+#[test]
+fn test_device_id_is_extraxted_from_device_certificate() -> Result<(), TEdgeConfigError> {
+    let toml_conf = r#"
+[device]
+cert_path = "/path/to/cert"
+"#;
+    let device_id = "device-serial-number";
+
+    let (tempdir, config_location) = create_temp_tedge_config(toml_conf)?;
+    let mut config =
+        TEdgeConfigRepository::new_with_defaults(config_location, dummy_tedge_config_defaults())
+            .load()?;
+
+    let cert_path = tempdir.path().join("certificate.pem");
+    create_certificate(cert_path.clone(), device_id).expect("fail to create a certificate");
+    config.update(DeviceCertPathSetting, cert_path.into())?;
+
+    assert_eq!(config.query(DeviceIdSetting)?, device_id);
+
+    Ok(())
+}
+
 fn create_temp_tedge_config(content: &str) -> std::io::Result<(TempDir, TEdgeConfigLocation)> {
     let dir = TempDir::new()?;
     let config_location = TEdgeConfigLocation::from_custom_root(dir.path());
@@ -491,3 +566,17 @@ fn dummy_tedge_config_defaults() -> TEdgeConfigDefaults {
         default_azure_root_cert_path: FilePath::from("/dev/null"),
     }
 }
+
+fn create_certificate(
+    path: std::path::PathBuf,
+    device_id: &str,
+) -> Result<(), certificate::CertificateError> {
+    let keypair = certificate::KeyCertPair::new_selfsigned_certificate(
+        &certificate::NewCertificateConfig::default(),
+        device_id,
+    )?;
+    let pem = keypair.certificate_pem_string()?;
+    let mut file = std::fs::File::create(path.clone())?;
+    file.write_all(pem.as_bytes())?;
+    Ok(())
+}