You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/02/08 03:35:53 UTC

[GitHub] [trafficserver] maskit opened a new pull request #7497: Fix QUIC unit test failures

maskit opened a new pull request #7497:
URL: https://github.com/apache/trafficserver/pull/7497


   Recent BoringSSL requires ALPN extension if the SSL context is used for QUIC.
   https://boringssl.googlesource.com/boringssl/+/74161f485b5d54fe963cbd3d081b718ec84d2e00%5E%21/


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] bneradt commented on a change in pull request #7497: Fix QUIC unit test failures

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7497:
URL: https://github.com/apache/trafficserver/pull/7497#discussion_r572156251



##########
File path: iocore/net/quic/test/test_QUICPacketHeaderProtector.cc
##########
@@ -65,6 +65,20 @@ TEST_CASE("QUICPacketHeaderProtector")
   BIO *key_bio(BIO_new_mem_buf(server_key, sizeof(server_key)));
   EVP_PKEY *pkey = PEM_read_bio_PrivateKey(key_bio, nullptr, nullptr, nullptr);
   SSL_CTX_use_PrivateKey(server_ssl_ctx, pkey);
+  SSL_CTX_set_alpn_select_cb(
+    server_ssl_ctx,
+    [](SSL *ssl, const unsigned char **out, unsigned char *outlen, const unsigned char *in, unsigned inlen, void *) {
+      auto ret = SSL_select_next_proto(const_cast<unsigned char **>(out), outlen,
+                                       reinterpret_cast<const unsigned char *>("\6h3-foo"), 6, in, inlen);

Review comment:
       Same comment as above.

##########
File path: iocore/net/quic/test/test_QUICHandshakeProtocol.cc
##########
@@ -102,16 +102,32 @@ TEST_CASE("QUICHandshakeProtocol")
   BIO *key_bio(BIO_new_mem_buf(server_key, sizeof(server_key)));
   EVP_PKEY *pkey = PEM_read_bio_PrivateKey(key_bio, nullptr, nullptr, nullptr);
   SSL_CTX_use_PrivateKey(server_ssl_ctx, pkey);
+  SSL_CTX_set_alpn_select_cb(
+    server_ssl_ctx,
+    [](SSL *ssl, const unsigned char **out, unsigned char *outlen, const unsigned char *in, unsigned inlen, void *) {
+      auto ret = SSL_select_next_proto(const_cast<unsigned char **>(out), outlen,
+                                       reinterpret_cast<const unsigned char *>("\6h3-foo"), 6, in, inlen);

Review comment:
       The openssl docs aren't clear on this, but I believe the server_len parameter here should be 7 instead of 6. The prefix of '\6' in the server string is correct, but I think that the server_len has to count all characters in the server string, inclusive of any prefixes.
   
   I think I'm reading the code right:
   https://github.com/openssl/openssl/blob/3f71add9e57fb48cb5efdc765860daf754db40e9/ssl/ssl_lib.c#L2896
   
   It will need to loop over all characters in server and client.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] bneradt commented on pull request #7497: Fix QUIC unit test failures

Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7497:
URL: https://github.com/apache/trafficserver/pull/7497#issuecomment-776083553


   [approve ci autest]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] maskit merged pull request #7497: Fix QUIC unit test failures

Posted by GitBox <gi...@apache.org>.
maskit merged pull request #7497:
URL: https://github.com/apache/trafficserver/pull/7497


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] maskit commented on pull request #7497: Fix QUIC unit test failures

Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7497:
URL: https://github.com/apache/trafficserver/pull/7497#issuecomment-775584646


   @bneradt Good catch. You're right. I've updated the lengths.
   The protocol name was originally "h3-29" and I changed it to "h3-foo", and forgot to update the length.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] zwoop commented on pull request #7497: Fix QUIC unit test failures

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7497:
URL: https://github.com/apache/trafficserver/pull/7497#issuecomment-784384075


   Cherry-picked to v9.1.x branch.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org