[Patch v1] Added TLS 1.3 support for PHP

[codarrenvelvindron]

• Compiled php with openssl 1.1.1 official
• Added tls 1.3 support for php
• Added/Updated test files
• Ran make tests - OK

Work done during IETF 103 hackathon

~ codarren at cyberstorm.mu ~

[bukka]

I added some comments which are mainly about missing compatibility with older OpenSSL versions. That's really required as we need to support multiple OpenSSL versions (currently 1.0.1+).

We will also need to address the setting of the cipher suites which works differently for 1.3 - see https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_ciphersuites.html . I think it doesn't need to be addressed in this patch but we shouldn't enable TLS 1.3 by default for now.

[bukka]

Another reason why we shouldn't enable it by default at this point is that I'm not sure how well it will work with our non-blocking handling that has got some issues and it's a bit mess. So considering that TLSv1.3 sends more non-application data records after the handshake is finished, it might show some bugs.

[codarrenvelvindron]

@bukka : backwards compatibility with tls <=1.2 works correctly with the fixes.

[bukka]

@codarrenvelvindron Just tested with OpenSSL 1.1.1 and the session_meta_capture_tlsv13.phpt is failing for me:

001+ Notice: Undefined index: session_meta in /home/jakub/prog/php/master/ext/openssl/tests/ServerClientTestCase.inc(96) : eval()'d code on line 14
001- string(7) "TLSv1.3"
002+ NULL

Is it actually working for you?

[codarrenvelvindron]

It's working with versions less than 1.1.1 but for >= 1.1.1 the actual test is failing for me as well.

According to the error messages, it seems unable to negotiate a proper cipher for tls 1.3.

Seems we'll have to implement SSL_CTX_set_ciphersuites() in this patch.

@bukka : What do you think?

[bukka]

you need to replace it with

    $serverCtx = stream_context_create(['ssl' => [
        'local_cert' => __DIR__ . '/bug54992.pem',
        'crypto_method' => STREAM_CRYPTO_METHOD_TLSv1_3_SERVER,
    ]]);

[bukka]

@codarrenvelvindron See my comment how to fix it: https://github.com/php/php-src/pull/3650/files#r238112239 . Once you fix it, please squash it and I will test it a bit more and then possibly merge it.

I don't think we need SSL_CTX_set_ciphersuites in this PR as the default is actually fine so it's more nice to have but it can be done later.

[codarrenvelvindron]

@bukka
See v2 :
#3700