git: aa906e2a4957 - main - OpenSSL: Support for kernel TLS offload (KTLS)

[Guido Falsi]

On 03/02/21 01:47, John Baldwin wrote:
> On 1/31/21 10:41 AM, Guido Falsi wrote:
>> On 28/01/21 19:25, John Baldwin wrote:
>>> The branch main has been updated by jhb:
>>>
>>> URL: 
>>> https://cgit.FreeBSD.org/src/commit/?id=aa906e2a4957db700d9e6cc60857e1afe1aecc85 
>>>
>>>
>>> commit aa906e2a4957db700d9e6cc60857e1afe1aecc85
>>> Author:     John Baldwin <jhb at FreeBSD.org>
>>> AuthorDate: 2021-01-16 00:17:31 +0000
>>> Commit:     John Baldwin <jhb at FreeBSD.org>
>>> CommitDate: 2021-01-28 18:24:13 +0000
>>>
>>>       OpenSSL: Support for kernel TLS offload (KTLS)
>>>       This merges upstream patches from OpenSSL's master branch to add
>>>       KTLS infrastructure for TLS 1.0-1.3 including both RX and TX
>>>       offload and SSL_sendfile support on both Linux and FreeBSD.
>>>       Note that TLS 1.3 only supports TX offload.
>>>       A new WITH/WITHOUT_OPENSSL_KTLS determines if OpenSSL is built 
>>> with
>>>       KTLS support.  It defaults to enabled on amd64 and disabled on all
>>>       other architectures.
>>>       Reviewed by:    jkim (earlier version)
>>>       Approved by:    secteam
>>>       Obtained from:  OpenSSL (patches from master)
>>>       MFC after:      1 week
>>>       Relnotes:       yes
>>>       Sponsored by:   Netflix
>>>       Differential Revision:  https://reviews.freebsd.org/D28273
>>> ---
>>
>> This commit causes a strange interaction/regression with subverison
>> client when using https protocol.
>>
>> I filed a bug report about this:
>>
>> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253135
>>
>> Workarounds:
>>
>> - Compiling system defining WITHOUT_OPENSSL_KTLS
>> - using the svn:// scheme
> 
> I'm still waiting for a build to finish so I can test it, but I believe
> this is a bug in serf.  This is the patch I'm going to test:
> 
> diff --git a/contrib/serf/buckets/ssl_buckets.c 
> b/contrib/serf/buckets/ssl_buckets.c
> index b01e5359db08..3c8b7e2a685f 100644
> --- a/contrib/serf/buckets/ssl_buckets.c
> +++ b/contrib/serf/buckets/ssl_buckets.c
> @@ -407,7 +407,7 @@ static int bio_bucket_destroy(BIO *bio)
> 
>   static long bio_bucket_ctrl(BIO *bio, int cmd, long num, void *ptr)
>   {
> -    long ret = 1;
> +    long ret = 0;
> 
>       switch (cmd) {
>       default:
> @@ -415,6 +415,7 @@ static long bio_bucket_ctrl(BIO *bio, int cmd, long 
> num, void *ptr)
>           break;
>       case BIO_CTRL_FLUSH:
>           /* At this point we can't force a flush. */
> +        ret = 1;
>           break;
>       case BIO_CTRL_PUSH:
>       case BIO_CTRL_POP:
> 
> serf defines its own custom OpenSSL BIO classes, and the BIO_ctrl(3)
> manpage documents that the control methods of custom BIOs are supposed
> to return 0 for unknown or unsupported requests:
> 
>         Source/sink BIOs return an 0 if they do not recognize the 
> BIO_ctrl()
>         operation.
> 
> However, the custom BIOs in serf broke this rule and returned 1 for
> unknown operations instead.  OpenSSL uses BIO_ctrl methods to determine
> if a given BIO for a read or write side of an SSL connection is using
> KTLS.  Because of the serf bug, this caused OpenSSL to believe that
> these BIOs were using KTLS when they in fact were not.  serf will also
> probably break with OpenSSL 3.0 even without KTLS due to the recently
> added control for determining if a BIO has hit EOF which also returns
> 1 to indicate it has hit EOF.
> 

Thanks for the feedback.

I have tested this patch. After reinstalling the system, from newly 
compiled sources.

svnlite from base works fine now.

svn from the package and also rebuilt from port fails. I guess ports 
provided serf library also has this bug, so we should fix it there too.

-- 
Guido Falsi <madpilot at FreeBSD.org>