Repository: caddyserver/caddy (language: go)

Issue:
"ocsp_stapling off" seems to have no effect
"ocsp_stapling off" seems to have no effect. Please see the following output:

```
# ./caddy version
v2.4.6 h1:HGkGICFGvyrodcqOOclHKfvJC0qTU7vny/7FhYp9hNw=
```

```
# cat Caddyfile 
{
	auto_https off
	ocsp_stapling off
}

:444

tls /etc/dehydrated/certs/.../fullchain.pem /etc/dehydrated/certs/.../privkey.pem
respond "Hello World"
```

```
# ./caddy run
2022/02/19 17:40:07.464	INFO	using adjacent Caddyfile
2022/02/19 17:40:07.469	INFO	admin	admin endpoint started	{"address": "tcp/localhost:2019", "enforce_origin": false, "origins": ["localhost:2019", "[::1]:2019", "127.0.0.1:2019"]}
2022/02/19 17:40:07.470	INFO	tls.cache.maintenance	started background certificate maintenance	{"cache": "0xc00053be30"}
2022/02/19 17:40:37.474	WARN	tls	stapling OCSP	{"error": "no OCSP stapling for [...]: making OCSP request: Post \"http://r3.o.lencr.org\": dial tcp 23.32.238.80:80: i/o timeout"}
2022/02/19 17:40:37.475	INFO	autosaved config (load with --resume flag)	{"file": "/root/.config/caddy/autosave.json"}
2022/02/19 17:40:37.475	INFO	serving initial configuration
2022/02/19 17:40:37.475	INFO	tls	cleaning storage unit	{"description": "FileStorage:/root/.local/share/caddy"}
2022/02/19 17:40:37.475	INFO	tls	finished cleaning storage units
```


Discussion / hints:
Hmm, that's strange.

All four places in certmagic which write the log message `stapling OCSP` all call the `stapleOCSP()` function, which has a check first thing to check if it's disabled. 

All the places that create certmagic configs in Caddy do seem to wire up the option correctly.

And the Caddyfile adapter does seem to properly set the option in JSON.

So I'm not sure where the problem is 🤔 
Using `caddy adapt` I can confirm that disable_ocsp_stapling is set to true.
However loading the config from JSON doesn't change the result.
The binary was downloaded from GitHub.
Is there anything else I could try?
Ah okay, I see the issue. The global option only applies to managed certificates (which Caddy maintains and renews), but not unmanaged certificates (i.e. from files with the `tls` directive).

Right now there's no wiring to configure this in the Caddyfile, but you can set the `disable_ocsp_stapling` boolean on the `tls` app in the JSON config for now https://caddyserver.com/docs/json/apps/tls/disable_ocsp_stapling/

I'll add the option in the Caddyfile adapter now, for the next release.
The option is present twice in the source code. "caddy adapt" sets the first one to true but not the second one.

If setting the second in the JSON file it works as expected. However I see no possibility to configure the same behaviour using the Caddyfile.

```
$ ag disable_ocsp_stapling
modules/caddytls/automation.go
132:	DisableOCSPStapling bool `json:"disable_ocsp_stapling,omitempty"`

modules/caddytls/tls.go
67:	DisableOCSPStapling bool `json:"disable_ocsp_stapling,omitempty"`
```

Removing the option from tls.go and setting it to t.Automation.[...]DisableOCSPStapling instead of the following could probably solve the issue:
```
               OCSP: certmagic.OCSPConfig{
                        DisableStapling: t.DisableOCSPStapling,
                },
```
I just figured it out as well. Thanks for the response and fixing the issue in the next release

Fix patch:
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index 48506d8137f..daaec95df91 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -301,6 +301,11 @@ func (st ServerType) buildTLSApp(
 		tlsApp.Automation.RenewCheckInterval = renewCheckInterval
 	}
 
+	// set whether OCSP stapling should be disabled for manually-managed certificates
+	if ocspConfig, ok := options["ocsp_stapling"].(certmagic.OCSPConfig); ok {
+		tlsApp.DisableOCSPStapling = ocspConfig.DisableStapling
+	}
+
 	// if any hostnames appear on the same server block as a key with
 	// no host, they will not be used with route matchers because the
 	// hostless key matches all hosts, therefore, it wouldn't be


Test patch:
diff --git a/caddytest/integration/caddyfile_adapt/global_options.txt b/caddytest/integration/caddyfile_adapt/global_options.txt
index dda644b28c3..69e2d9d57bc 100644
--- a/caddytest/integration/caddyfile_adapt/global_options.txt
+++ b/caddytest/integration/caddyfile_adapt/global_options.txt
@@ -10,6 +10,7 @@
 	}
 	acme_ca https://example.com
 	acme_ca_root /path/to/ca.crt
+	ocsp_stapling off
 
 	email test@example.com
 	admin off
@@ -61,7 +62,8 @@
 								"module": "internal"
 							}
 						],
-						"key_type": "ed25519"
+						"key_type": "ed25519",
+						"disable_ocsp_stapling": true
 					}
 				],
 				"on_demand": {
@@ -71,7 +73,8 @@
 					},
 					"ask": "https://example.com"
 				}
-			}
+			},
+			"disable_ocsp_stapling": true
 		}
 	}
 }