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

[John Baldwin]

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.

-- 
John Baldwin