Re: svn commit: r1910820 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr60182.txt modules/ssl/ssl_util_stapling.c

[Joe Orton]

On Thu, Jul 06, 2023 at 10:58:07PM +0200, Christophe JAILLET wrote:
> Le 06/07/2023 à 18:11, jor...@apache.org a écrit :
> > Author: jorton
> > Date: Thu Jul  6 16:11:56 2023
> > New Revision: 1910820
> > 
> > URL: http://svn.apache.org/viewvc?rev=1910820&view=rev
> > Log:
> > Merge r1875355 from trunk:
> > 
> > * modules/ssl/ssl_util_stapling.c (stapling_check_response) Don't stop
> >Certificate Revoked messages.
> > 
> >Certificate Revoked Responder messages don't belong to 'error' class.
> >When the server receives one, it MUST be passed on to the client.
> >And stored for the normal period of basic responses.
> > 
> >Also don't log an error each time it is retrieved from cache,
> >only once when it is retrieved from the OCSP responder.
> > 
> > PR: 60182
> > Obtained from: 
> > https://github.com/apache/httpd/commit/7db9795f45fd4688ceb13ee36090e4e2becbc709.diff
> > Submitted by: 
> > Reviewed by: gbechis, icing, ylavic
> 
> Hi,
> 
> I've not seen it in STATUS.
> 
> I'm a bit distant this days, so I may have missed something, but usually
> STATUS is updated just before or after a commit, and I don't see it either.

Hi Christophe - this was voted for in STATUS under "Fix for BZ 66626" 
though I missed that the bug number mentioned is different to the commit 
message. I've fixed the changes entry just now since 66626 is the right 
one. https://github.com/apache/httpd/blob/2.4.x/STATUS#L196

Thanks for checking & thanks for fixing my typo too.

Regards, Joe

Re: svn commit: r1910820 - in /httpd/httpd/branches/2.4.x: ./ changes-entries/pr60182.txt modules/ssl/ssl_util_stapling.c

[Christophe JAILLET]

Le 06/07/2023 à 18:11, jor...@apache.org a écrit :

Author: jorton
Date: Thu Jul  6 16:11:56 2023
New Revision: 1910820

URL: http://svn.apache.org/viewvc?rev=1910820&view=rev
Log:
Merge r1875355 from trunk:

* modules/ssl/ssl_util_stapling.c (stapling_check_response) Don't stop
   Certificate Revoked messages.

   Certificate Revoked Responder messages don't belong to 'error' class.
   When the server receives one, it MUST be passed on to the client.
   And stored for the normal period of basic responses.

   Also don't log an error each time it is retrieved from cache,
   only once when it is retrieved from the OCSP responder.

PR: 60182
Obtained from: 
https://github.com/apache/httpd/commit/7db9795f45fd4688ceb13ee36090e4e2becbc709.diff
Submitted by: 
Reviewed by: gbechis, icing, ylavic


Hi,

I've not seen it in STATUS.

I'm a bit distant this days, so I may have missed something, but usually 
STATUS is updated just before or after a commit, and I don't see it either.




Added:
 httpd/httpd/branches/2.4.x/changes-entries/pr60182.txt
Modified:
 httpd/httpd/branches/2.4.x/   (props changed)
 httpd/httpd/branches/2.4.x/modules/ssl/ssl_util_stapling.c

Propchange: httpd/httpd/branches/2.4.x/
--
   Merged /httpd/httpd/trunk:r1875355

Added: httpd/httpd/branches/2.4.x/changes-entries/pr60182.txt
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/changes-entries/pr60182.txt?rev=1910820&view=auto
==
--- httpd/httpd/branches/2.4.x/changes-entries/pr60182.txt (added)
+++ httpd/httpd/branches/2.4.x/changes-entries/pr60182.txt Thu Jul  6 16:11:56 
2023
@@ -0,0 +1,2 @@
+  *) mod_ssl: Fix handling of of Certificate Revoked messags
+ in OCSP stapling. PR 60182 []


Unless I missed something, this CHANGES entry is not in trunk. Should it 
be added?




Modified: httpd/httpd/branches/2.4.x/modules/ssl/ssl_util_stapling.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/ssl/ssl_util_stapling.c?rev=1910820&r1=1910819&r2=1910820&view=diff
==
--- httpd/httpd/branches/2.4.x/modules/ssl/ssl_util_stapling.c (original)
+++ httpd/httpd/branches/2.4.x/modules/ssl/ssl_util_stapling.c Thu Jul  6 
16:11:56 2023
@@ -445,7 +445,7 @@ static int stapling_check_response(serve
  rv = SSL_TLSEXT_ERR_NOACK;
  }
  
-if (status != V_OCSP_CERTSTATUS_GOOD) {

+if (status != V_OCSP_CERTSTATUS_GOOD && pok) {
  char snum[MAX_STRING_LEN] = { '\0' };
  BIO *bio = BIO_new(BIO_s_mem());
  
@@ -466,12 +466,6 @@ static int stapling_check_response(serve

   (reason != OCSP_REVOKED_STATUS_NOSTATUS) ?
   OCSP_crl_reason_str(reason) : "n/a",
   snum[0] ? snum : "[n/a]");
-
-if (mctx->stapling_return_errors == FALSE) {
-if (pok)
-*pok = FALSE;
-rv = SSL_TLSEXT_ERR_NOACK;
-}
  }
  }
  


PR 60182 was about r1875355 and r1875356.
This backport in only for r1875355.

Is it done on purpose or is the 2nd commit missing?

CJ