From bfaf2a8201b83d7369772cb6f2439abe66d9342a Mon Sep 17 00:00:00 2001
From: Kyle McCullough <kylemcc@gmail.com>
Date: Mon, 5 Dec 2022 23:12:26 -0800
Subject: [PATCH] acme_server: Configurable default lifetime for issued
 certificates (#5232)

* acme_server: add certificate lifetime configuration option

Signed-off-by: Kyle McCullough <kylemcc@gmail.com>

* pki: allow intermediate cert lifetime to be configured

Signed-off-by: Kyle McCullough <kylemcc@gmail.com>

Signed-off-by: Kyle McCullough <kylemcc@gmail.com>
---
 caddyconfig/httpcaddyfile/pkiapp.go           |  18 ++-
 .../caddyfile_adapt/acme_server_lifetime.txt  | 108 ++++++++++++++++++
 .../global_options_skip_install_trust.txt     |   2 +-
 caddytest/integration/pki_test.go             | 101 ++++++++++++++++
 modules/caddypki/acmeserver/acmeserver.go     |  14 ++-
 modules/caddypki/acmeserver/caddyfile.go      |  19 +++
 modules/caddypki/ca.go                        |  10 +-
 modules/caddypki/certificates.go              |   4 +-
 8 files changed, 268 insertions(+), 8 deletions(-)
 create mode 100644 caddytest/integration/caddyfile_adapt/acme_server_lifetime.txt
 create mode 100644 caddytest/integration/pki_test.go

diff --git a/caddyconfig/httpcaddyfile/pkiapp.go b/caddyconfig/httpcaddyfile/pkiapp.go
index a67ac992b..3414636ee 100644
--- a/caddyconfig/httpcaddyfile/pkiapp.go
+++ b/caddyconfig/httpcaddyfile/pkiapp.go
@@ -15,6 +15,7 @@
 package httpcaddyfile
 
 import (
+	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig"
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddypki"
@@ -28,9 +29,10 @@ func init() {
 //
 //	pki {
 //	    ca [<id>] {
-//	        name            <name>
-//	        root_cn         <name>
-//	        intermediate_cn <name>
+//	        name                  <name>
+//	        root_cn               <name>
+//	        intermediate_cn       <name>
+//	        intermediate_lifetime <duration>
 //	        root {
 //	            cert   <path>
 //	            key    <path>
@@ -83,6 +85,16 @@ func parsePKIApp(d *caddyfile.Dispenser, existingVal any) (any, error) {
 						}
 						pkiCa.IntermediateCommonName = d.Val()
 
+					case "intermediate_lifetime":
+						if !d.NextArg() {
+							return nil, d.ArgErr()
+						}
+						dur, err := caddy.ParseDuration(d.Val())
+						if err != nil {
+							return nil, err
+						}
+						pkiCa.IntermediateLifetime = caddy.Duration(dur)
+
 					case "root":
 						if pkiCa.Root == nil {
 							pkiCa.Root = new(caddypki.KeyPair)
diff --git a/caddytest/integration/caddyfile_adapt/acme_server_lifetime.txt b/caddytest/integration/caddyfile_adapt/acme_server_lifetime.txt
new file mode 100644
index 000000000..6099440a4
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/acme_server_lifetime.txt
@@ -0,0 +1,108 @@
+{
+	pki {
+		ca internal {
+			name "Internal"
+			root_cn "Internal Root Cert"
+			intermediate_cn "Internal Intermediate Cert"
+		}
+		ca internal-long-lived {
+			name "Long-lived"
+			root_cn "Internal Root Cert 2"
+			intermediate_cn "Internal Intermediate Cert 2"
+		}
+	}
+}
+
+acme-internal.example.com {
+	acme_server {
+		ca internal
+	}
+}
+
+acme-long-lived.example.com {
+	acme_server {
+		ca internal-long-lived
+		lifetime 7d
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":443"
+					],
+					"routes": [
+						{
+							"match": [
+								{
+									"host": [
+										"acme-long-lived.example.com"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"ca": "internal-long-lived",
+													"handler": "acme_server",
+													"lifetime": 604800000000000
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						},
+						{
+							"match": [
+								{
+									"host": [
+										"acme-internal.example.com"
+									]
+								}
+							],
+							"handle": [
+								{
+									"handler": "subroute",
+									"routes": [
+										{
+											"handle": [
+												{
+													"ca": "internal",
+													"handler": "acme_server"
+												}
+											]
+										}
+									]
+								}
+							],
+							"terminal": true
+						}
+					]
+				}
+			}
+		},
+		"pki": {
+			"certificate_authorities": {
+				"internal": {
+					"name": "Internal",
+					"root_common_name": "Internal Root Cert",
+					"intermediate_common_name": "Internal Intermediate Cert"
+				},
+				"internal-long-lived": {
+					"name": "Long-lived",
+					"root_common_name": "Internal Root Cert 2",
+					"intermediate_common_name": "Internal Intermediate Cert 2"
+				}
+			}
+		}
+	}
+}
diff --git a/caddytest/integration/caddyfile_adapt/global_options_skip_install_trust.txt b/caddytest/integration/caddyfile_adapt/global_options_skip_install_trust.txt
index 8116a4b39..3a175a0d7 100644
--- a/caddytest/integration/caddyfile_adapt/global_options_skip_install_trust.txt
+++ b/caddytest/integration/caddyfile_adapt/global_options_skip_install_trust.txt
@@ -165,4 +165,4 @@ acme-bar.example.com {
 			}
 		}
 	}
-}
\ No newline at end of file
+}
diff --git a/caddytest/integration/pki_test.go b/caddytest/integration/pki_test.go
new file mode 100644
index 000000000..5e9928c0c
--- /dev/null
+++ b/caddytest/integration/pki_test.go
@@ -0,0 +1,101 @@
+package integration
+
+import (
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/caddytest"
+)
+
+func TestLeafCertLifetimeLessThanIntermediate(t *testing.T) {
+	caddytest.AssertLoadError(t, `
+    {
+      "apps": {
+        "http": {
+          "servers": {
+            "srv0": {
+              "listen": [
+                ":443"
+              ],
+              "routes": [
+                {
+                  "handle": [
+                    {
+                      "handler": "subroute",
+                      "routes": [
+                        {
+                          "handle": [
+                            {
+                              "ca": "internal",
+                              "handler": "acme_server",
+                              "lifetime": 604800000000000
+                            }
+                          ]
+                        }
+                      ]
+                    }
+                  ]
+                }
+              ]
+            }
+          }
+        },
+        "pki": {
+          "certificate_authorities": {
+            "internal": {
+              "install_trust": false,
+              "intermediate_lifetime": 604800000000000,
+              "name": "Internal CA"
+            }
+          }
+        }
+      }
+    }
+	`, "json", "certificate lifetime (168h0m0s) should be less than intermediate certificate lifetime (168h0m0s)")
+}
+
+func TestIntermediateLifetimeLessThanRoot(t *testing.T) {
+	caddytest.AssertLoadError(t, `
+    {
+      "apps": {
+        "http": {
+          "servers": {
+            "srv0": {
+              "listen": [
+                ":443"
+              ],
+              "routes": [
+                {
+                  "handle": [
+                    {
+                      "handler": "subroute",
+                      "routes": [
+                        {
+                          "handle": [
+                            {
+                              "ca": "internal",
+                              "handler": "acme_server",
+                              "lifetime": 2592000000000000
+                            }
+                          ]
+                        }
+                      ]
+                    }
+                  ]
+                }
+              ]
+            }
+          }
+        },
+        "pki": {
+          "certificate_authorities": {
+            "internal": {
+              "install_trust": false,
+              "intermediate_lifetime": 311040000000000000,
+              "name": "Internal CA"
+            }
+          }
+        }
+      }
+    }
+	`, "json", "intermediate certificate lifetime must be less than root certificate lifetime (86400h0m0s)")
+}
diff --git a/modules/caddypki/acmeserver/acmeserver.go b/modules/caddypki/acmeserver/acmeserver.go
index 921d0b879..6ecdfdc66 100644
--- a/modules/caddypki/acmeserver/acmeserver.go
+++ b/modules/caddypki/acmeserver/acmeserver.go
@@ -48,6 +48,9 @@ type Handler struct {
 	// the default ID is "local".
 	CA string `json:"ca,omitempty"`
 
+	// The lifetime for issued certificates
+	Lifetime caddy.Duration `json:"lifetime,omitempty"`
+
 	// The hostname or IP address by which ACME clients
 	// will access the server. This is used to populate
 	// the ACME directory endpoint. If not set, the Host
@@ -95,6 +98,9 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 	if ash.PathPrefix == "" {
 		ash.PathPrefix = defaultPathPrefix
 	}
+	if ash.Lifetime == 0 {
+		ash.Lifetime = caddy.Duration(12 * time.Hour)
+	}
 
 	// get a reference to the configured CA
 	appModule, err := ctx.App("pki")
@@ -107,6 +113,12 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 		return err
 	}
 
+	// make sure leaf cert lifetime is less than the intermediate cert lifetime. this check only
+	// applies for caddy-managed intermediate certificates
+	if ca.Intermediate == nil && ash.Lifetime >= ca.IntermediateLifetime {
+		return fmt.Errorf("certificate lifetime (%s) should be less than intermediate certificate lifetime (%s)", time.Duration(ash.Lifetime), time.Duration(ca.IntermediateLifetime))
+	}
+
 	database, err := ash.openDatabase()
 	if err != nil {
 		return err
@@ -122,7 +134,7 @@ func (ash *Handler) Provision(ctx caddy.Context) error {
 					Claims: &provisioner.Claims{
 						MinTLSDur:     &provisioner.Duration{Duration: 5 * time.Minute},
 						MaxTLSDur:     &provisioner.Duration{Duration: 24 * time.Hour * 365},
-						DefaultTLSDur: &provisioner.Duration{Duration: 12 * time.Hour},
+						DefaultTLSDur: &provisioner.Duration{Duration: time.Duration(ash.Lifetime)},
 					},
 				},
 			},
diff --git a/modules/caddypki/acmeserver/caddyfile.go b/modules/caddypki/acmeserver/caddyfile.go
index fe12712ba..ae2d8ef11 100644
--- a/modules/caddypki/acmeserver/caddyfile.go
+++ b/modules/caddypki/acmeserver/caddyfile.go
@@ -15,6 +15,9 @@
 package acmeserver
 
 import (
+	"time"
+
+	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddypki"
 )
@@ -27,6 +30,7 @@ func init() {
 //
 //	acme_server [<matcher>] {
 //	    ca <id>
+//	    lifetime <duration>
 //	}
 func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
 	if !h.Next() {
@@ -55,6 +59,21 @@ func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error
 					ca = new(caddypki.CA)
 				}
 				ca.ID = acmeServer.CA
+			case "lifetime":
+				if !h.NextArg() {
+					return nil, h.ArgErr()
+				}
+
+				dur, err := caddy.ParseDuration(h.Val())
+				if err != nil {
+					return nil, err
+				}
+
+				if d := time.Duration(ca.IntermediateLifetime); d > 0 && dur > d {
+					return nil, h.Errf("certificate lifetime (%s) exceeds intermediate certificate lifetime (%s)", dur, d)
+				}
+
+				acmeServer.Lifetime = caddy.Duration(dur)
 			}
 		}
 	}
diff --git a/modules/caddypki/ca.go b/modules/caddypki/ca.go
index 914eddf8e..1ba08900a 100644
--- a/modules/caddypki/ca.go
+++ b/modules/caddypki/ca.go
@@ -48,6 +48,9 @@ type CA struct {
 	// intermediate certificates.
 	IntermediateCommonName string `json:"intermediate_common_name,omitempty"`
 
+	// The lifetime for the intermediate certificates
+	IntermediateLifetime caddy.Duration `json:"intermediate_lifetime,omitempty"`
+
 	// Whether Caddy will attempt to install the CA's root
 	// into the system trust store, as well as into Java
 	// and Mozilla Firefox trust stores. Default: true.
@@ -118,6 +121,11 @@ func (ca *CA) Provision(ctx caddy.Context, id string, log *zap.Logger) error {
 	if ca.IntermediateCommonName == "" {
 		ca.IntermediateCommonName = defaultIntermediateCommonName
 	}
+	if ca.IntermediateLifetime == 0 {
+		ca.IntermediateLifetime = caddy.Duration(defaultIntermediateLifetime)
+	} else if time.Duration(ca.IntermediateLifetime) >= defaultRootLifetime {
+		return fmt.Errorf("intermediate certificate lifetime must be less than root certificate lifetime (%s)", defaultRootLifetime)
+	}
 
 	// load the certs and key that will be used for signing
 	var rootCert, interCert *x509.Certificate
@@ -341,7 +349,7 @@ func (ca CA) loadOrGenIntermediate(rootCert *x509.Certificate, rootKey crypto.Si
 func (ca CA) genIntermediate(rootCert *x509.Certificate, rootKey crypto.Signer) (interCert *x509.Certificate, interKey crypto.Signer, err error) {
 	repl := ca.newReplacer()
 
-	interCert, interKey, err = generateIntermediate(repl.ReplaceAll(ca.IntermediateCommonName, ""), rootCert, rootKey)
+	interCert, interKey, err = generateIntermediate(repl.ReplaceAll(ca.IntermediateCommonName, ""), rootCert, rootKey, time.Duration(ca.IntermediateLifetime))
 	if err != nil {
 		return nil, nil, fmt.Errorf("generating CA intermediate: %v", err)
 	}
diff --git a/modules/caddypki/certificates.go b/modules/caddypki/certificates.go
index c3b88a10a..e30042938 100644
--- a/modules/caddypki/certificates.go
+++ b/modules/caddypki/certificates.go
@@ -35,8 +35,8 @@ func generateRoot(commonName string) (*x509.Certificate, crypto.Signer, error) {
 	return root, signer, nil
 }
 
-func generateIntermediate(commonName string, rootCrt *x509.Certificate, rootKey crypto.Signer) (*x509.Certificate, crypto.Signer, error) {
-	template, signer, err := newCert(commonName, x509util.DefaultIntermediateTemplate, defaultIntermediateLifetime)
+func generateIntermediate(commonName string, rootCrt *x509.Certificate, rootKey crypto.Signer, lifetime time.Duration) (*x509.Certificate, crypto.Signer, error) {
+	template, signer, err := newCert(commonName, x509util.DefaultIntermediateTemplate, lifetime)
 	if err != nil {
 		return nil, nil, err
 	}