Re: svn commit: r1926720 - /httpd/httpd/trunk/modules/ssl/ssl_private.h

On 6/25/25 4:42 PM, Joe Orton wrote:
> On Wed, Jun 25, 2025 at 04:15:48PM +0200, Ruediger Pluem wrote:
>>
>>
>> On 6/25/25 3:56 PM, Joe Orton wrote:
>>> On Wed, Jun 25, 2025 at 03:41:57PM +0200, Ruediger Pluem wrote:
>>>>> --- httpd/httpd/trunk/modules/ssl/ssl_private.h (original)
>>>>> +++ httpd/httpd/trunk/modules/ssl/ssl_private.h Wed Jun 25 12:53:18 2025
>>>>> @@ -296,8 +296,12 @@ void free_bio_methods(void);
>>>>>  #define X509_get_notAfter   X509_getm_notAfter
>>>>>  #endif
>>>>>  
>>>>> -#if OPENSSL_VERSION_NUMBER >= 0x10101000L && !defined(LIBRESSL_VERSION_NUMBER)
>>>>> -#define HAVE_OPENSSL_KEYLOG
>>>>> +/* For OpenSSL 3.5.0+, don't handle $SSLKEYLOGFILE since libssl does -
>>>>> + * unless OpenSSL was built with no-sslkeylog, which drops the env var
>>>>> + * handling, but leaves the API intact. */
>>>>> +#if OPENSSL_VERSION_NUMBER >= 0x10101000L && !defined(LIBRESSL_VERSION_NUMBER) \
>>>>> +    && (OPENSSL_VERSION_NUMBER <= 0x30500000L || !defined(OPENSSL_NO_SSLKEYLOG))
>>>>
>>>> When is OPENSSL_NO_SSLKEYLOG defined? When it was build with no-sslkeylog and thus drops the env var handling?
>>>> And in this case we want to handle it as before?
>>>
>>> Yep - exactly right.
>>
>> But shouldn't it be defined(OPENSSL_NO_SSLKEYLOG) then and not 
>> !defined(OPENSSL_NO_SSLKEYLOG) as we want to define 
>> HAVE_OPENSSL_KEYLOG on OpenSSL 3.5.0+ when OPENSSL_NO_SSLKEYLOG is 
>> defined.
> 
> Ah! Yes, I inverted the test - thank you very much for the review :)
> 
> Does this read correctly like this?
> 
> Index: modules/ssl/ssl_private.h
> ===================================================================
> --- modules/ssl/ssl_private.h	(revision 1926720)
> +++ modules/ssl/ssl_private.h	(working copy)
> @@ -296,11 +296,15 @@
>  #define X509_get_notAfter   X509_getm_notAfter
>  #endif
>  
> -/* For OpenSSL 3.5.0+, don't handle $SSLKEYLOGFILE since libssl does -
> - * unless OpenSSL was built with no-sslkeylog, which drops the env var
> - * handling, but leaves the API intact. */
> +/* The SSL_CTX_set_keylog_callback() API is present in 1.1.1+.
> + * 
> + * OpenSSL 3.5+ also provides optional native handling of
> + * $SSLKEYLOGFILE inside libssl, which duplicates the mod_ssl support.
> + * The mod_ssl support is hence disabled for 3.5+, unless that OpenSSL
> + * feature is itself disabled (and OPENSSL_NO_SSLKEYLOG is defined).
> + */
>  #if OPENSSL_VERSION_NUMBER >= 0x10101000L && !defined(LIBRESSL_VERSION_NUMBER) \
> -    && (OPENSSL_VERSION_NUMBER <= 0x30500000L || !defined(OPENSSL_NO_SSLKEYLOG))
> +    && (OPENSSL_VERSION_NUMBER <= 0x30500000L || defined(OPENSSL_NO_SSLKEYLOG))
>  #define HAVE_OPENSSL_KEYLOG 
>  #endif
>  

This looks good. Thanks :).

Regards

Rüdiger