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

Jump to bottom

Proposed ALPN failure clarification: append ", continuing with post-TLS negotiation" #8643

Closed
ncoghlan opened this issue Mar 28, 2022 · 2 comments
Closed

Proposed ALPN failure clarification: append ", continuing with post-TLS negotiation" #8643

ncoghlan opened this issue Mar 28, 2022 · 2 comments
Labels
TLS

Comments

@ncoghlan
Copy link

Following up on the previous reports of the technically accurate but confusing to non-experts ALPN notification, a clarification that would have helped me would be:

ALPN: server did not agree on a protocol, continuing with post-TLS negotiation

I would have submitted a PR, but this message appears in multiple places, so I wasn't sure if the preferred approach would be to just change them all or to factor them out as a shared constant (assuming that the proposed change is even acceptable in the first place).

(I would have commented on #2749 instead of filing a new ticket but it's locked)
@bagder bagder added the TLS label Mar 28, 2022
@bagder
Copy link
Member

bagder commented Mar 28, 2022

I'm personally not convinced this suggested new wording will change much. I figure the fact it continues is already visible.

The string could indeed get moved into vtls.c somehow to get shared and used by all TLS backends since right now they all use their own duplicated version of it.

bagder added a commit that referenced this issue Mar 30, 2022
@bagder
vtls: provide a unified APLN-disagree string for all backends
5fc4957 
Also rephrase to make it sound less dangerous:

 "ALPN: server did not agree on a protocol. Uses default."

Reported-by: Nick Coghlan
Fixes #8643
@bagder bagder mentioned this issue Mar 30, 2022
Closed
@bagder
Copy link
Member

bagder commented Mar 30, 2022

My change makes it instead say:

ALPN: server did not agree on a protocol. Uses default.

@bagder bagder closed this as completed in 3bc5b32 Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
TLS
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

vtls: provide a unified APLN-disagree string for all backends curl/curl
2 participants
@bagder @ncoghlan