From f9ebbbcce9bec5ccd1277c53eb7c062790f501e9 Mon Sep 17 00:00:00 2001
From: "Dustin C. Hatch" <dustin@hatch.name>
Date: Thu, 1 Feb 2024 09:05:04 -0600
Subject: [PATCH] ca/user: Add certificate extensions

According to the *sshd(8)* manual page:

> Certificates may encode access restrictions similar to these key
> options.  If both certificate restrictions and key options are
> present, the most restrictive union of the two is applied.

This would seem to apply that if a certificate has no restrictions, all
features are allowed unless restricted in the `authorized_keys` file.
Unfortunately, this is not actually the case.  A certificate with no
extensions apparently trumps all other configuration.  As such,
certificates need to explicitly list the features users will need.

The list of extensions to add to user certificates is configurable via
the `ca.user.extensions` array.  The default set should provide a good
user experience without being overly permissive.
---
 src/ca.rs          | 38 ++++++++++++++++++++++----------------
 src/config.rs      | 15 +++++++++++++++
 src/server/user.rs | 10 +++++++++-
 3 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/src/ca.rs b/src/ca.rs
index 055bcf7..145f0da 100644
--- a/src/ca.rs
+++ b/src/ca.rs
@@ -144,7 +144,20 @@ pub fn sign_host_cert(
     privkey: &PrivateKey,
     alias: &[&str],
 ) -> Result<Certificate, CertError> {
-    sign_cert(hostname, pubkey, duration, privkey, alias, CertType::Host)
+    let now = SystemTime::now();
+    let not_before = now.duration_since(UNIX_EPOCH)?.as_secs();
+    let not_after = not_before + duration.as_secs();
+
+    let mut builder = Builder::new_with_random_nonce(
+        &mut OsRng, pubkey, not_before, not_after,
+    )?;
+    builder.cert_type(CertType::Host)?;
+    builder.valid_principal(hostname)?;
+    for a in alias {
+        builder.valid_principal(*a)?;
+    }
+
+    Ok(builder.sign(privkey)?)
 }
 
 /// Create a signed SSH certificate for a user public key
@@ -158,17 +171,7 @@ pub fn sign_user_cert(
     duration: Duration,
     privkey: &PrivateKey,
     alias: &[&str],
-) -> Result<Certificate, CertError> {
-    sign_cert(username, pubkey, duration, privkey, alias, CertType::User)
-}
-
-fn sign_cert(
-    principal: &str,
-    pubkey: &PublicKey,
-    duration: Duration,
-    privkey: &PrivateKey,
-    alias: &[&str],
-    cert_type: CertType,
+    extensions: &[impl AsRef<str>],
 ) -> Result<Certificate, CertError> {
     let now = SystemTime::now();
     let not_before = now.duration_since(UNIX_EPOCH)?.as_secs();
@@ -177,11 +180,14 @@ fn sign_cert(
     let mut builder = Builder::new_with_random_nonce(
         &mut OsRng, pubkey, not_before, not_after,
     )?;
-    builder.cert_type(cert_type)?;
-    builder.valid_principal(principal)?;
+    builder.cert_type(CertType::User)?;
+    builder.valid_principal(username)?;
     for a in alias {
         builder.valid_principal(*a)?;
     }
+    for e in extensions {
+        builder.extension(e.as_ref(), "")?;
+    }
 
     Ok(builder.sign(privkey)?)
 }
@@ -201,9 +207,9 @@ mod test {
         let host_pub_key = host_key.public_key();
         let duration = Duration::from_secs(86400 * 30);
         let hostname = "cloud0.example.org";
-        let cert = sign_cert(
+        let cert = sign_host_cert(
             hostname,
-            &host_pub_key,
+            host_pub_key,
             duration,
             &ca_key,
             &["nextcloud.example.org"],
diff --git a/src/config.rs b/src/config.rs
index e3ed190..c69b868 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -75,6 +75,10 @@ pub struct UserCaConfig {
     /// Duration of issued user certificates
     #[serde(default = "default_user_cert_duration")]
     pub cert_duration: u64,
+
+    /// Certificate extensions
+    #[serde(default = "default_user_cert_extensions")]
+    pub extensions: Vec<String>,
 }
 
 impl Default for UserCaConfig {
@@ -83,6 +87,7 @@ impl Default for UserCaConfig {
             private_key_file: default_user_ca_key(),
             private_key_passphrase_file: None,
             cert_duration: default_user_cert_duration(),
+            extensions: default_user_cert_extensions(),
         }
     }
 }
@@ -171,6 +176,16 @@ fn default_user_cert_duration() -> u64 {
     3600
 }
 
+fn default_user_cert_extensions() -> Vec<String> {
+    vec![
+        "permit-X11-forwarding".into(),
+        "permit-agent-forwarding".into(),
+        "permit-port-forwarding".into(),
+        "permit-pty".into(),
+        "permit-user-rc".into(),
+    ]
+}
+
 /// Load configuration from a TOML file
 ///
 /// If `path` is provided, the configuration will be loaded from the
diff --git a/src/server/user.rs b/src/server/user.rs
index 68a093e..92df14f 100644
--- a/src/server/user.rs
+++ b/src/server/user.rs
@@ -168,6 +168,7 @@ pub(super) async fn sign_user_cert(
 
     let config = &ctx.config;
     let duration = Duration::from_secs(config.ca.user.cert_duration);
+    let extensions = &config.ca.user.extensions;
     let privkey = ca::load_private_key(
         &config.ca.user.private_key_file,
         config.ca.user.private_key_passphrase_file.as_ref(),
@@ -192,7 +193,14 @@ pub(super) async fn sign_user_cert(
         pubkey.algorithm().as_str(),
         username
     );
-    let cert = ca::sign_user_cert(username, &pubkey, duration, &privkey, &alias)?;
+    let cert = ca::sign_user_cert(
+        username,
+        &pubkey,
+        duration,
+        &privkey,
+        &alias,
+        &extensions[..],
+    )?;
     info!(
         "Signed {} key for {}",
         pubkey.algorithm().as_str(),