Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngtcp2: fix QUIC transport parameter version #7771

Closed

Conversation

@katojunya
Copy link
Contributor

@katojunya katojunya commented Sep 24, 2021

Bug details

In the ngtcp2 + OpenSSL combination, the QUIC transport parameter is set to duplicate version of QUICv1 and draft, as shown below. Some QUIC stack, for example facebook.com's mvfst, returns a parameter error for this Initial.

IETF QUIC
  Version: draft-29 (0xff00001d)
    .
    .
  TLSv1.3 Record Layer: Handshake Protocol: Client Hello
        Frame Type: CRYPTO (0x0000000000000006)
        Crypto Data
        Handshake Protocol: Client Hello
            Handshake Type: Client Hello (1)
            Extension: application_layer_protocol_negotiation (len=8)
      **====> Extension: quic_transport_parameters (drafts version) (len=55)**
      **====> Extension: quic_transport_parameters (len=55)**
	       .
	       .

As a result, curl returns Resource temporarily unavailable.

# curl --http3 -v https://www.facebook.com/
* Trying 2a03:2880:f10f:83:face:b00c:0:25de:443...
* Connect socket 5 over QUIC to 2a03:2880:f10f:83:face:b00c:0:25de:443
* connect to 2a03:2880:f10f:83:face:b00c:0:25de port 443 failed: Resource temporarily unavailable
* Trying 31.13.82.36:443...
* Connect socket 6 over QUIC to 31.13.82.36:443
* Connect to 31.13.82.36 port 443 failed: Resource temporarily unavailable
* Failed to connect to www.facebook.com port 443 after 40 ms: Resource temporarily unavailable
* Closing connection 0
curl: (7) Failed to connect to www.facebook.com port 443 after 40 ms: Resource temporarily unavailable

Cause and patch

The cause is a missing call to the OpenSSL API. This patch fixes the transport parameter version to use DRAFT version. I think it is probably due to OpenSSL bug.

Default QUIC version

This patch keeps that curl with ngtcp2 uses DRAFT version (h3-29).

I think QUICv1+h3 should be the default version in the near future. If default version is changed to QUICv1, SSL_set_quic_use_legacy_codepoint(qs->ssl, 0); and some modifies enables it.

fix inappropriate version setting for QUIC transport parameters.
this patch keeps curl with ngtcp2 uses QUIC draft version (h3-29).
@bagder bagder added the HTTP/3 label Sep 24, 2021
@bagder bagder requested a review from tatsuhiro-t Sep 24, 2021
Copy link
Member

@bagder bagder left a comment

Ugha, that's a long name that makes it cross the too wide barrier:

./vquic/ngtcp2.c:443:82: warning: Longer than 79 columns (LONGLINE)
                                    NGTCP2_TLSEXT_QUIC_TRANSPORT_PARAMETERS_DRAFT,

@katojunya
Copy link
Contributor Author

@katojunya katojunya commented Sep 25, 2021

Thank you for reviewing my changes.

I modified the code to two-character indentation style, as with other functions with long arguments.

@bagder
Copy link
Member

@bagder bagder commented Sep 25, 2021

Thanks!

@bagder bagder closed this in 4a10a99 Sep 25, 2021
@katojunya katojunya deleted the fix-transport-parameter-version-draft branch Sep 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants