tailscale, keys: randomize disco key, don't store

src/config.rs

@@ -2,6 +2,9 @@
 
 use std::path::Path;
 
+use serde::Serializer;
+use ts_keys::PersistState;
+
 use crate::keys::NodeState;
 
 const CONTROL_URL_VAR: &str = "TS_CONTROL_URL";
@@ -10,11 +13,7 @@ const AUTHKEY_VAR: &str = "TS_AUTH_KEY";
 
 /// Config for connecting to Tailscale.
 pub struct Config {
-    /// The path of the file used to store cryptographic keys.
-    ///
-    /// This file represents this node's identity. The key file format is specific to
-    /// tailscale-rs; key files are not interchangeable with those produced by other
-    /// implementations of Tailscale, e.g. `tailscaled` or `tsnet`.
+    /// The cryptographic keys representing this node's identity.
     pub key_state: NodeState,
 
     // TODO(npry): let clients also define an app name once the sdk-level name moves
@@ -44,7 +43,7 @@ impl Config {
     /// the key file.
     pub async fn default_with_key_file(p: impl AsRef<Path>) -> Result<Self, crate::Error> {
         Ok(Config {
-            key_state: load_key_file(p, Default::default()).await?,
+            key_state: load_key_file(p, Default::default()).await?.into(),
             ..Default::default()
         })
     }
@@ -89,17 +88,70 @@ pub fn auth_key_from_env() -> Option<String> {
 pub async fn load_key_file(
     p: impl AsRef<Path>,
     bad_format: BadFormatBehavior,
-) -> Result<NodeState, crate::Error> {
+) -> Result<PersistState, crate::Error> {
     let p = p.as_ref();
 
     tracing::trace!(key_file = %p.display(), "loading key file");
 
-    let key_file = load_or_init::<KeyFile>(&p, Default::default, bad_format).await?;
-    Ok(key_file.key_state)
+    let key_file = load_or_init::<KeyFile>(
+        &p,
+        Default::default,
+        |x| match x {
+            #[allow(deprecated)]
+            KeyFile::Old(old) => Some(KeyFile::New(KeyFileNew {
+                key_state: PersistState::from(&old.key_state),
+            })),
+            _ => None,
+        },
+        bad_format,
+    )
+    .await?;
+    Ok(key_file.key_state())
+}
+
+#[derive(serde::Deserialize)]
+#[serde(untagged)]
+enum KeyFile {
+    #[deprecated]
+    Old(KeyFileOld),
+    New(KeyFileNew),
+}
+
+impl KeyFile {
+    #[allow(deprecated)]
+    pub fn key_state(&self) -> PersistState {
+        match self {
+            Self::Old(old) => (&old.key_state).into(),
+            Self::New(new) => new.key_state.clone(),
+        }
+    }
+}
+
+impl Default for KeyFile {
+    fn default() -> Self {
+        KeyFile::New(KeyFileNew::default())
+    }
 }
 
-#[derive(serde::Serialize, serde::Deserialize, Default)]
-struct KeyFile {
+impl serde::Serialize for KeyFile {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        KeyFileNew {
+            key_state: self.key_state(),
+        }
+        .serialize(serializer)
+    }
+}
+
+#[derive(serde::Deserialize, serde::Serialize, Default)]
+struct KeyFileNew {
+    key_state: PersistState,
+}
+
+#[derive(serde::Deserialize)]
+struct KeyFileOld {
     key_state: NodeState,
 }
 
@@ -145,6 +197,7 @@ pub enum BadFormatBehavior {
 async fn load_or_init<KeyState>(
     path: impl AsRef<Path>,
     default: impl FnOnce() -> KeyState,
+    migrate: impl FnOnce(&KeyState) -> Option<KeyState>,
     bad_format_behavior: BadFormatBehavior,
 ) -> Result<KeyState, crate::Error>
 where
@@ -162,6 +215,18 @@ where
     match tokio::fs::read(path).await {
         Ok(contents) => match serde_json::from_slice::<KeyState>(&contents) {
             Ok(state) => {
+                if let Some(migrated) = migrate(&state) {
+                    match try_write(path, &migrated).await {
+                        Ok(_) => {
+                            tracing::info!("migrated key file to new disco-less format");
+                            return Ok(migrated);
+                        }
+                        Err(e) => {
+                            tracing::error!(error = %e, "unable to migrate key file");
+                        }
+                    }
+                }
+
                 return Ok(state);
             }
             Err(e) => match bad_format_behavior {
@@ -186,9 +251,17 @@ where
     }
 
     let value = default();
+    try_write(path, &value).await?;
+    Ok(value)
+}
+
+async fn try_write(
+    path: impl AsRef<Path>,
+    value: &impl serde::Serialize,
+) -> Result<(), crate::Error> {
     tokio::fs::write(
         path,
-        serde_json::to_vec(&value).map_err(|e| {
+        serde_json::to_vec(value).map_err(|e| {
             tracing::error!(error = %e, "serializing key state");
             crate::Error::InternalFailure
         })?,
@@ -199,5 +272,5 @@ where
         crate::Error::InternalFailure
     })?;
 
-    Ok(value)
+    Ok(())
 }

src/lib.rs

@@ -317,7 +317,7 @@ pub mod keys {
     pub use ts_keys::{
         DiscoKeyPair, DiscoPrivateKey, DiscoPublicKey, MachineKeyPair, MachinePrivateKey,
         MachinePublicKey, NetworkLockKeyPair, NetworkLockPrivateKey, NetworkLockPublicKey,
-        NodeKeyPair, NodePrivateKey, NodePublicKey, NodeState,
+        NodeKeyPair, NodePrivateKey, NodePublicKey, NodeState, PersistState,
     };
 }
 

ts_elixir/native/ts_elixir/src/lib.rs

@@ -129,6 +129,7 @@ fn load_key_file(env: rustler::Env, path: &str) -> impl Encoder {
     let result = TOKIO_RUNTIME
         .block_on(tailscale::config::load_key_file(path, Default::default()))
         .map(|keys| {
+            let keys: tailscale::keys::NodeState = keys.into();
             let result: Keystate = keys.into();
             result
         })

ts_ffi/src/keys.rs

@@ -138,6 +138,7 @@ pub unsafe extern "C" fn ts_load_key_file(
 
     match TOKIO_RUNTIME.block_on(tailscale::config::load_key_file(s, mode)) {
         Ok(state) => {
+            let state: tailscale::keys::NodeState = state.into();
             *key_state = state.into();
             tracing::info!(?key_state, "loaded key state");
 

ts_keys/src/keystate.rs

@@ -1,17 +1,65 @@
 use core::fmt::{Debug, Display, Formatter};
 
-use crate::{DiscoKeyPair, MachineKeyPair, NetworkLockKeyPair, NodeKeyPair};
+use crate::{
+    DiscoKeyPair, MachineKeyPair, MachinePrivateKey, NetworkLockKeyPair, NetworkLockPrivateKey,
+    NodeKeyPair, NodePrivateKey,
+};
 
-/// The complete key state for a Tailscale node.
+/// The portion of the key state that should be retained between runs of the same device.
+///
+/// Disco keys are ephemeral and should be generated anew each time a device runs, so are
+/// excluded from this state.
+#[derive(Clone, Debug)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct PersistState {
+    /// The [`MachinePrivateKey`] for the hardware this Tailnet peer runs on.
+    pub machine_key: MachinePrivateKey,
+
+    /// The [`NetworkLockPrivateKey`] for this Tailnet peer, for use with Tailnet Lock.
+    pub network_lock_key: NetworkLockPrivateKey,
+
+    /// The [`NodePrivateKey`] for this Tailnet peer.
+    pub node_key: NodePrivateKey,
+}
+
+impl From<&NodeState> for PersistState {
+    fn from(value: &NodeState) -> Self {
+        Self {
+            node_key: value.node_keys.private,
+            machine_key: value.machine_keys.private,
+            network_lock_key: value.network_lock_keys.private,
+        }
+    }
+}
+
+impl From<NodeState> for PersistState {
+    fn from(value: NodeState) -> Self {
+        Self::from(&value)
+    }
+}
+
+impl Default for PersistState {
+    fn default() -> Self {
+        Self {
+            machine_key: MachinePrivateKey::random(),
+            network_lock_key: NetworkLockPrivateKey::random(),
+            node_key: NodePrivateKey::random(),
+        }
+    }
+}
+
+/// The complete runtime key state for a Tailscale node.
 #[derive(Clone, Default)]
-#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
+#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
 pub struct NodeState {
     /// The [`DiscoKeyPair`] this Tailnet peer uses for the Disco protocol.
+    ///
+    /// These should be randomly generated for each run of a Tailscale device.
     pub disco_keys: DiscoKeyPair,
 
     /// The [`MachineKeyPair`] for the hardware this Tailnet peer runs on.
     pub machine_keys: MachineKeyPair,
-    // TODO (dylan): is this meant to be peer-specific?
+
     /// The [`NetworkLockKeyPair`] for this Tailnet peer, for use with Tailnet Lock.
     pub network_lock_keys: NetworkLockKeyPair,
 
@@ -42,3 +90,20 @@ impl NodeState {
         Default::default()
     }
 }
+
+impl From<&PersistState> for NodeState {
+    fn from(value: &PersistState) -> Self {
+        Self {
+            disco_keys: Default::default(),
+            node_keys: value.node_key.into(),
+            machine_keys: value.machine_key.into(),
+            network_lock_keys: value.network_lock_key.into(),
+        }
+    }
+}
+
+impl From<PersistState> for NodeState {
+    fn from(value: PersistState) -> Self {
+        Self::from(&value)
+    }
+}

ts_keys/src/lib.rs

@@ -7,7 +7,7 @@ mod keystate;
 mod macros;
 
 #[doc(inline)]
-pub use keystate::NodeState;
+pub use keystate::{NodeState, PersistState};
 use macros::{
     _create_x25519_base_key_type, create_x25519_keypair_types, create_x25519_private_key_type,
     create_x25519_public_key_type,