Re: Adding support for OpenSSL checksum implementations

2026-01-30 Thread Timofei Zhakov
On Thu, Jan 29, 2026 at 4:33 PM Daniel Sahlberg 
wrote:

> Den tors 29 jan. 2026 kl 11:24 skrev Joe Orton :
>
>> On Wed, Dec 31, 2025 at 11:44:26AM +0100, Daniel Sahlberg wrote:
>> > Hi,
>> >
>> > Subversion is using the APR/APR-util checksum implementations (SHA1 and
>> > MD5). One of our committers in Subversion made some tests switching out
>> > these for the ones in OpenSSL instead. OpenSSL is (opt-out) using an ASM
>> > optimized implementation on many platforms.
>> >
>> > Copy-pasting from the commit message[1] to include some raw numbers:
>>
>> Hi Daniel,
>>
>> The apr_crypto API already wraps the OpenSSL EVP API, so adding another
>> wrapper using the deprecated digest APIs wouldn't really make sense IMO.
>> It's also (again IMO) important to note that the OpenSSL digest
>> implementations should be treated as having restricted availability;
>> MD5_Init() etc will fail under FIPS mode (as do the EVP equivalents).
>
>
> Hi,
>
> I'm not sure I understand the above... If it relates to the actual
> implementation in Subversion, it wasn't my intention to say "hey, let's
> copypaste this to APR". I believe it should be updated to a current API
> (although I've seen Ivan's e-mails about performance - these need to be
> considered).
>
> My question was rather: Would it make sense for APR to use OpenSSL's
> digest algorithms, if available (and if built with OpenSSL support)?
>
>
Hello,

It might be not as straightforward to be done on APR's side because it
publicly exposes its context which most certainly doesn't match OpenSSL's
context. (considering apr_sha/apr_md APIs). But it's not as big of a deal
because a new version can be introduced that works around it.

I just realized that there is apr_crypto_digest* API in the APR crypto
module that is probably being discussed right now. I believe it's worth it
to support it in Subversion. But we need some additional testing to find
out if it adds any significant performance issues.

-- 
Timofei Zhakov


Re: Adding support for OpenSSL checksum implementations

2026-01-29 Thread Daniel Sahlberg
Den tors 29 jan. 2026 kl 11:24 skrev Joe Orton :

> On Wed, Dec 31, 2025 at 11:44:26AM +0100, Daniel Sahlberg wrote:
> > Hi,
> >
> > Subversion is using the APR/APR-util checksum implementations (SHA1 and
> > MD5). One of our committers in Subversion made some tests switching out
> > these for the ones in OpenSSL instead. OpenSSL is (opt-out) using an ASM
> > optimized implementation on many platforms.
> >
> > Copy-pasting from the commit message[1] to include some raw numbers:
>
> Hi Daniel,
>
> The apr_crypto API already wraps the OpenSSL EVP API, so adding another
> wrapper using the deprecated digest APIs wouldn't really make sense IMO.
> It's also (again IMO) important to note that the OpenSSL digest
> implementations should be treated as having restricted availability;
> MD5_Init() etc will fail under FIPS mode (as do the EVP equivalents).


Hi,

I'm not sure I understand the above... If it relates to the actual
implementation in Subversion, it wasn't my intention to say "hey, let's
copypaste this to APR". I believe it should be updated to a current API
(although I've seen Ivan's e-mails about performance - these need to be
considered).

My question was rather: Would it make sense for APR to use OpenSSL's digest
algorithms, if available (and if built with OpenSSL support)?

Cheers,
Daniel



>
> The SVN code referenced seems ignore the _Init() return values (copying
> dev@svn for this).
>

Thanks for noticing. We should review this.


> Regards, Joe
>
> >
> > Cheers,
> > Daniel
> >
> > [1] https://lists.apache.org/thread/o38r06vg08pyzzy35bfgg8ooovphsxss
>
>


Re: Adding support for OpenSSL checksum implementations

2026-01-29 Thread Ivan Zhakov
On Thu, 29 Jan 2026 at 14:17, Branko Čibej  wrote:

> On 29. 1. 26 11:23, Joe Orton wrote:
>
> On Wed, Dec 31, 2025 at 11:44:26AM +0100, Daniel Sahlberg wrote:
>
> Hi,
>
> Subversion is using the APR/APR-util checksum implementations (SHA1 and
> MD5). One of our committers in Subversion made some tests switching out
> these for the ones in OpenSSL instead. OpenSSL is (opt-out) using an ASM
> optimized implementation on many platforms.
>
> Copy-pasting from the commit message[1] to include some raw numbers:
>
> Hi Daniel,
>
> The apr_crypto API already wraps the OpenSSL EVP API, so adding another
> wrapper using the deprecated digest APIs wouldn't really make sense IMO.
> It's also (again IMO) important to note that the OpenSSL digest
> implementations should be treated as having restricted availability;
> MD5_Init() etc will fail under FIPS mode (as do the EVP equivalents).
> The SVN code referenced seems ignore the _Init() return values (copying
> dev@svn for this).
>
>
>
> The EVP API is in this case a wrapper around the deprecated functions. I
> see no reason not to use it instead, there won't be any detectable
> difference in performance.
>
> There are reports that EVP API is significantly slower in some cases. E.g:
https://github.com/openssl/openssl/issues/19612

And some tricks are necessary to avoid significant performance hit for
small buffers.

-- 
Ivan Zhakov


Re: Adding support for OpenSSL checksum implementations

2026-01-29 Thread Ivan Zhakov
On Thu, 29 Jan 2026 at 11:24, Joe Orton  wrote:

> On Wed, Dec 31, 2025 at 11:44:26AM +0100, Daniel Sahlberg wrote:
> > Hi,
> >
> > Subversion is using the APR/APR-util checksum implementations (SHA1 and
> > MD5). One of our committers in Subversion made some tests switching out
> > these for the ones in OpenSSL instead. OpenSSL is (opt-out) using an ASM
> > optimized implementation on many platforms.
> >
> > Copy-pasting from the commit message[1] to include some raw numbers:
>
> Hi Daniel,
>
> The apr_crypto API already wraps the OpenSSL EVP API, so adding another
> wrapper using the deprecated digest APIs wouldn't really make sense IMO.
> It's also (again IMO) important to note that the OpenSSL digest
> implementations should be treated as having restricted availability;
> MD5_Init() etc will fail under FIPS mode (as do the EVP equivalents).
>

May be I misunderstand something, but I don't see checks for FIPS mode in
**MD5_Init()**:
https://github.com/openssl/openssl/blob/66ead9927dc7aba0dcfb9068f9288ce1e4feda53/crypto/md5/md5_dgst.c#L29
[[[
int MD5_Init(MD5_CTX *c)
{
memset(c, 0, sizeof(*c));
c->A = INIT_DATA_A;
c->B = INIT_DATA_B;
c->C = INIT_DATA_C;
c->D = INIT_DATA_D;
return 1;
}
]]]

-- 
Ivan Zhakov


Re: Adding support for OpenSSL checksum implementations

2026-01-29 Thread Branko Čibej

On 29. 1. 26 11:23, Joe Orton wrote:

On Wed, Dec 31, 2025 at 11:44:26AM +0100, Daniel Sahlberg wrote:

Hi,

Subversion is using the APR/APR-util checksum implementations (SHA1 and
MD5). One of our committers in Subversion made some tests switching out
these for the ones in OpenSSL instead. OpenSSL is (opt-out) using an ASM
optimized implementation on many platforms.

Copy-pasting from the commit message[1] to include some raw numbers:

Hi Daniel,

The apr_crypto API already wraps the OpenSSL EVP API, so adding another
wrapper using the deprecated digest APIs wouldn't really make sense IMO.
It's also (again IMO) important to note that the OpenSSL digest
implementations should be treated as having restricted availability;
MD5_Init() etc will fail under FIPS mode (as do the EVP equivalents).
The SVN code referenced seems ignore the _Init() return values (copying
dev@svn for this).



The EVP API is in this case a wrapper around the deprecated functions. I 
see no reason not to use it instead, there won't be any detectable 
difference in performance.


-- Brane


Re: Adding support for OpenSSL checksum implementations

2026-01-29 Thread Joe Orton
On Wed, Dec 31, 2025 at 11:44:26AM +0100, Daniel Sahlberg wrote:
> Hi,
> 
> Subversion is using the APR/APR-util checksum implementations (SHA1 and
> MD5). One of our committers in Subversion made some tests switching out
> these for the ones in OpenSSL instead. OpenSSL is (opt-out) using an ASM
> optimized implementation on many platforms.
> 
> Copy-pasting from the commit message[1] to include some raw numbers:

Hi Daniel,

The apr_crypto API already wraps the OpenSSL EVP API, so adding another 
wrapper using the deprecated digest APIs wouldn't really make sense IMO. 
It's also (again IMO) important to note that the OpenSSL digest 
implementations should be treated as having restricted availability; 
MD5_Init() etc will fail under FIPS mode (as do the EVP equivalents). 
The SVN code referenced seems ignore the _Init() return values (copying 
dev@svn for this).

Regards, Joe

> 
> Cheers,
> Daniel
> 
> [1] https://lists.apache.org/thread/o38r06vg08pyzzy35bfgg8ooovphsxss