Re: [PATCH] Allow OCSP repsonses containing multiple single responses

2017-10-18 Thread Robert Thralls
Felt bad about not including a documentation patch, so here it is.

Changes:
  - Clarifies that HAProxy does not fetch OCSP responses. Apache just has a
couple of set-and-forget directives that handle everything for you, so this
was definitely non-obvious to me.
  - Removes mention of needing to be a single response (my patch from
ealier today).
  - Removes mention of "good" status, per Emmanuel's patch last week.

I'm not sure if Emmanuel's patch will actually be accepted or not, but I
think the resulting language is clearer regardless, since it avoids
multiple contexts for "valid". Anyone needing the fine details can look at
the code and the (probably rare) people seeing the error already have the
information they need either way. Similarly, I saw there was no mention of
requiring the nextupdate field, but that is indeed a requirement and the
error itself was apparently sufficient documentation.


Rob Thralls




On Wed, Oct 18, 2017 at 3:46 PM, Robert Thralls <rthra...@econsys.com>
wrote:

> Obligatory "I am not a C programmer" and "my first upstream patch"
> messages.
>
> We had an issue where one of our server certificate issuers was sending us
> responses with 20 different single responses included. The serial numbers
> in the Certificate IDs were exactly sequential, so I'm guessing they're
> pre-generating the responses in chunks. HAProxy didn't like it:
>
> "OCSP response ignored because contains multiple single responses (20).
> Content will be ignored."
>
> I did see the comment in src/ssl_sock.c. "Note: OCSP response containing
> more than one OCSP Single response is not considered valid." But I'm not
> sure how true that really is nowadays. From my searches this morning, it
> seems the standards themselves have been found to be lacking, which has
> resulted in browser support chaos (surprising no one). I'm coming from
> Apache httpd, which happily serves the full responses.
>
> If accepted or adapted, the documentation should be updated as well.
>
>
> Rob Thralls
>
--- haproxy-1.7.9/doc/configuration.txt	Fri Aug 18 02:33:20 2017
+++ haproxy-1.7.9/doc/configuration.txt.new	Wed Oct 18 19:17:00 2017
@@ -10213,16 +10213,14 @@
   For each PEM file, haproxy checks for the presence of file at the same path
   suffixed by ".ocsp". If such file is found, support for the TLS Certificate
   Status Request extension (also known as "OCSP stapling") is automatically
-  enabled. The content of this file is optional. If not empty, it must contain
-  a valid OCSP Response in DER format. In order to be valid an OCSP Response
-  must comply with the following rules: it has to indicate a good status,
-  it has to be a single response for the certificate of the PEM file, and it
-  has to be valid at the moment of addition. If these rules are not respected
-  the OCSP Response is ignored and a warning is emitted. In order to  identify
-  which certificate an OCSP Response applies to, the issuer's certificate is
-  necessary. If the issuer's certificate is not found in the PEM file, it will
-  be loaded from a file at the same path as the PEM file suffixed by ".issuer"
-  if it exists otherwise it will fail with an error.
+  enabled, though haproxy does not currently fetch OCSP responses. The contents
+  of this file are optional. If not empty, it must contain an OCSP response in
+  DER format that is valid at the moment of addition. Otherwise, the contents
+  are ignored and a warning emitted. In order to identify which certificate an
+  OCSP response applies to, the issuer's certificate is necessary. If the
+  issuer's certificate is not found in the PEM file, it will be loaded from a
+  file at the same path as the PEM file suffixed by ".issuer" if it exists,
+  otherwise it will fail with an error.
 
   For each PEM file, haproxy also checks for the presence of file at the same
   path suffixed by ".sctl". If such file is found, support for Certificate


[PATCH] Allow OCSP repsonses containing multiple single responses

2017-10-18 Thread Robert Thralls
Obligatory "I am not a C programmer" and "my first upstream patch" messages.

We had an issue where one of our server certificate issuers was sending us
responses with 20 different single responses included. The serial numbers
in the Certificate IDs were exactly sequential, so I'm guessing they're
pre-generating the responses in chunks. HAProxy didn't like it:

"OCSP response ignored because contains multiple single responses (20).
Content will be ignored."

I did see the comment in src/ssl_sock.c. "Note: OCSP response containing
more than one OCSP Single response is not considered valid." But I'm not
sure how true that really is nowadays. From my searches this morning, it
seems the standards themselves have been found to be lacking, which has
resulted in browser support chaos (surprising no one). I'm coming from
Apache httpd, which happily serves the full responses.

If accepted or adapted, the documentation should be updated as well.


Rob Thralls
--- haproxy-1.7.9/src/ssl_sock.c	Fri Aug 18 02:33:20 2017
+++ haproxy-1.7.9/src/ssl_sock.c.new	Wed Oct 18 13:04:22 2017
@@ -336,7 +333,8 @@
 	OCSP_SINGLERESP *sr;
 	OCSP_CERTID *id;
 	unsigned char *p = (unsigned char *)ocsp_response->str;
-	int rc , count_sr;
+	int rc;
+	int idx_sr = -1;
 	ASN1_GENERALIZEDTIME *revtime, *thisupd, *nextupd = NULL;
 	int reason;
 	int ret = 1;
@@ -359,13 +357,13 @@
 		goto out;
 	}
 
-	count_sr = OCSP_resp_count(bs);
-	if (count_sr > 1) {
-		memprintf(err, "OCSP response ignored because contains multiple single responses (%d)", count_sr);
+	idx_sr = OCSP_resp_find(bs, cid, -1);
+	if (idx_sr == -1) {
+		memprintf(err, "Failed to get index of OCSP single response");
 		goto out;
 	}
 
-	sr = OCSP_resp_get0(bs, 0);
+	sr = OCSP_resp_get0(bs, idx_sr);
 	if (!sr) {
 		memprintf(err, "Failed to get OCSP single response");
 		goto out;