From fca41d5a245023376c7d7716a3f84abc2aaa4b8e Mon Sep 17 00:00:00 2001
From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Sun, 17 Aug 2014 16:38:32 +0100
Subject: [PATCH] Override an unchanged default hosts_request_ocsp when DANE is
 used

---
 doc/doc-txt/experimental-spec.txt | 23 +++++++++++------------
 src/src/tls-openssl.c             | 27 ++++++++++++++++++++++-----
 src/src/transports/smtp.c         |  2 +-
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/doc/doc-txt/experimental-spec.txt b/doc/doc-txt/experimental-spec.txt
index c060a6c5a..80e970cc1 100644
--- a/doc/doc-txt/experimental-spec.txt
+++ b/doc/doc-txt/experimental-spec.txt
@@ -1236,24 +1236,23 @@ The use of OCSP-stapling should be considered, allowing
 for fast revocation of certificates (which would otherwise
 be limited by the DNS TTL on the TLSA records).  However,
 this is likely to only be usable with DANE_TA.  NOTE: the
-default is to request OCSP for all hosts; the certificate
-chain in DANE_EE usage will be insufficient to validate
-the OCSP proof and verification will fail.  Either disable
-OCSP completely or use the (new) variable $tls_out_tlsa_usage
-like so:
-
-  hosts_request_ocsp = ${if or { {= {4}{$tls_out_tlsa_usage}} \
-				 {= {0}{$tls_out_tlsa_usage}} } \
+default of requesting OCSP for all hosts is modified iff
+DANE is in use, to:
+
+  hosts_request_ocsp = ${if or { {= {0}{$tls_out_tlsa_usage}} \
+				 {= {4}{$tls_out_tlsa_usage}} } \
                          {*}{}}
-The variable is a bitfield with numbered bits set for TLSA
-record usage codes. The zero above means DANE was not in use,
+
+The (new) variable $tls_out_tlsa_usage is a bitfield with
+numbered bits set for TLSA record usage codes.
+The zero above means DANE was not in use,
 the four means that only DANE_TA usage TLSA records were
 found. If the definition of hosts_require_ocsp or
 hosts_request_ocsp includes the string "tls_out_tlsa_usage",
 they are re-expanded in time to control the OCSP request.
 
-[ All a bit complicated.  Should we make that definition
-the default?  Should we override the user's definition? ]
+This modification of hosts_request_ocsp is only done if
+it has the default value of "*".
 
 
 For client-side DANE there are two new smtp transport options,
diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c
index de2e7a3bd..343122615 100644
--- a/src/src/tls-openssl.c
+++ b/src/src/tls-openssl.c
@@ -1771,11 +1771,28 @@ else if (dane_required)
 
 #ifndef DISABLE_OCSP
   {
-  require_ocsp = verify_check_this_host(&ob->hosts_require_ocsp,
-    NULL, host->name, host->address, NULL) == OK;
-  request_ocsp = require_ocsp ? TRUE
-    : verify_check_this_host(&ob->hosts_request_ocsp,
-	NULL, host->name, host->address, NULL) == OK;
+  if ((require_ocsp = verify_check_this_host(&ob->hosts_require_ocsp,
+    NULL, host->name, host->address, NULL) == OK))
+    request_ocsp = TRUE;
+  else
+    {
+# ifdef EXPERIMENTAL_DANE
+    if (  dane
+       && ob->hosts_request_ocsp[0] == '*'
+       && ob->hosts_request_ocsp[1] == '\0'
+       )
+      {
+      /* Unchanged from default.  Use a safer one under DANE */
+      request_ocsp = TRUE;
+      ob->hosts_request_ocsp = US"${if or { {= {0}{$tls_out_tlsa_usage}} "
+					"   {= {4}{$tls_out_tlsa_usage}} } "
+				   " {*}{}}";
+      }
+    else
+# endif
+      request_ocsp = verify_check_this_host(&ob->hosts_request_ocsp,
+	  NULL, host->name, host->address, NULL) == OK;
+    }
   }
 #endif
 
diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index 9abc69d51..1865adee8 100644
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -216,7 +216,7 @@ smtp_transport_options_block smtp_transport_option_defaults = {
   NULL,                /* hosts_try_prdr */
 #endif
 #ifndef DISABLE_OCSP
-  US"*",               /* hosts_request_ocsp */
+  US"*",               /* hosts_request_ocsp (except under DANE) */
   NULL,                /* hosts_require_ocsp */
 #endif
   NULL,                /* hosts_require_tls */
-- 
2.25.1