Skip to content

[ms-quic] update version to 2.2.0#29568

Closed
FrankXie05 wants to merge 25 commits intomicrosoft:masterfrom
FrankXie05:dev/Frank/26811
Closed

[ms-quic] update version to 2.2.0#29568
FrankXie05 wants to merge 25 commits intomicrosoft:masterfrom
FrankXie05:dev/Frank/26811

Conversation

@FrankXie05
Copy link
Copy Markdown
Contributor

@FrankXie05 FrankXie05 commented Feb 10, 2023

Fix #29459 #26811

Note: Update [ms-quic] version to 2.1.8
Remove the ci.baseline.txt records.
Fix the compilation on newer windows SDKs.
microsoft/msquic#3405

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@FrankXie05 FrankXie05 added info:internal category:port-update The issue is with a library, which is requesting update new revision labels Feb 10, 2023
github-actions[bot]
github-actions bot previously approved these changes Feb 10, 2023
github-actions[bot]
github-actions bot previously approved these changes Feb 10, 2023
Comment thread ports/ms-quic/portfile.cmake Outdated
Comment thread ports/ms-quic/portfile.cmake Outdated
Comment thread scripts/ci.baseline.txt Outdated
Comment thread ports/ms-quic/fix-install.patch
github-actions[bot]
github-actions bot previously approved these changes Feb 15, 2023
@FrankXie05 FrankXie05 added the depends:upstream-changes Waiting on a change to the upstream project label Feb 24, 2023
@medcelerate
Copy link
Copy Markdown

Is there any update on this?

@FrankXie05
Copy link
Copy Markdown
Contributor Author

@medcelerate
If it doesn't respond, I will complete this PR through a patch next week. :)
microsoft/msquic#3405 (comment)

@FrankXie05 FrankXie05 removed the depends:upstream-changes Waiting on a change to the upstream project label Mar 20, 2023
@FrankXie05 FrankXie05 changed the title [ms-quic] update version to 2.1.7 [ms-quic] update version to 2.1.8 Mar 20, 2023
@hexorer
Copy link
Copy Markdown
Contributor

hexorer commented May 17, 2023

@FrankXie05 would you rebase/sync with master branch?

@hexorer
Copy link
Copy Markdown
Contributor

hexorer commented May 24, 2023

/azp run

@azure-pipelines
Copy link
Copy Markdown

Commenter does not have sufficient privileges for PR 29568 in repo microsoft/vcpkg

@BillyONeal
Copy link
Copy Markdown
Member

/azp run

You can trigger this to happen yourself by just pushing into the branch again, such as a merge commit with master.

Comment thread ports/ms-quic/fix-platform.patch
@FrankXie05 FrankXie05 marked this pull request as ready for review May 25, 2023 02:18
@FrankXie05 FrankXie05 requested a review from Cheney-W May 25, 2023 02:18
FrankXie added 2 commits May 24, 2023 19:46
@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented May 25, 2023

Why does this port use a vendored copy of openssl 1.1.1k?

2.2.0 added support for openssl 3. And now there is 2.2.1 with additional improvements.

@FrankXie05
Copy link
Copy Markdown
Contributor Author

Why does this port use a vendored copy of openssl 1.1.1k?

https://github.com/quictls/openssl#:~:text=As%20stated%20in,for%20QUIC%20implementations.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented May 25, 2023

Why does this port use a vendored copy of openssl 1.1.1k?

https://github.com/quictls/openssl#:~:text=As%20stated%20in,for%20QUIC%20implementations.

Translating the link to a reasonable answer:
ms-quic doesn't use openssl, but provides a modified build of openssl (here: 1.1.1k).
(However, it might use openssl's pristine libcrypto.)

Correct me if I'm wrong.

Follow-up questions:

(When) Will this be upgraded to openssl 3?

Given the struggle to bring port openssl into a good shape, is the vendored build of openssl the reason for ms-quic being not tested in vcpkg CI for:

ms-quic:arm-uwp=fail
ms-quic:arm64-windows=fail
ms-quic:x64-uwp=fail
ms-quic:x64-windows-static-md=fail
ms-quic:x64-windows=fail
ms-quic:x86-windows=fail

(This doesn't leave many triplets.)

@FrankXie05
Copy link
Copy Markdown
Contributor Author

Upstream is updating to openssl 3.1 synchronously.
microsoft/msquic#3511

@nibanks
Copy link
Copy Markdown

nibanks commented May 25, 2023

MsQuic must use a custom fork of openssl for QUIC support. The normal openssl does not support the necessary QUIC APIs. But as it was mentioned above, at runtime we can leverage the pristine crypto layer (libcrypto) of openssl, just not the TLS (libssl) layer, so we statically link that layer into MsQuic.

And v2.2 of MsQuic does support both v1.1 and v3.1 of openssl, but you have to select which you want at compile time. Generally, we publish Linux packages that match the distro version's openssl (i.e., 20.04 uses v1.1 & 22.04 uses v3.1).

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented May 25, 2023

Upstream is updating to openssl 3.1 synchronously. microsoft/msquic#3511

The point is: the openssl copy in the ms-quic port isn't updating magically.

vcpkg_from_github(
OUT_SOURCE_PATH OPENSSL_SOURCE_PATH
REPO quictls/openssl
REF a6e9d76db343605dae9b59d71d2811b195ae7434
SHA512 23510a11203b96476c194a1987c7d4e758375adef0f6dfe319cd8ec4b8dd9b12ea64c4099cf3ba35722b992dad75afb1cfc5126489a5fa59f5ee4d46bdfbeaf6
HEAD_REF OpenSSL_1_1_1k+quic
)

quictls/openssl@a6e9d76 is from Apr 12, 2021. Unchanged by this PR. No security fixes in 13 months.

@nibanks
Copy link
Copy Markdown

nibanks commented May 25, 2023

microsoft/msquic submodules the quictls fork of openssl and we immediately update the dependencies as there are security issues. IMHO, there shouldn't be yet another place here in vcpkg that would need to be updated. vcpkg should point to the HEAD of the msquic release/* branches that are consistently kept up to date (and never have breaking changes). Just build msquic with its submodule references.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented May 25, 2023

@nibanks vcpkg doesn't do a git checkout of upstream's HEAD but fetches tarballs for known commits and verifies SHA512 checksums. Changing any source asset requires an explicit port update. This pattern also applies to submodules.

So this port is constructed in a way that it needs to follow a primary and a secondary upstream. The secondary relationship is under-maintained.

@nibanks
Copy link
Copy Markdown

nibanks commented May 25, 2023

What can be done here then? I cannot necessarily commit any resources of my team to help keep vcpkg updated, unless we have very clear instructions (in the msquic repo) for what needs to be done when we service a fix.

But generally, this seems like a super fragile system. With likely thousands of dependencies here, each has to be manually updated whenever a servicing change is patched for a release? Who does all that?

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented May 25, 2023

Some checks could be be automated in the portfiles.

But generally, this seems like a super fragile system. With likely thousands of dependencies here, each has to be manually updated whenever a servicing change is patched for a release?

There are several issues in vcpkg which cause fragility, such as not running tests, or updating ports without looking at
the fine print.
But I'm afraid it would be much more fragile if everything would change independently, and every upstream would throw its own forks and copies on the pile of libs.

Who does all that?

https://github.com/microsoft/vcpkg/graphs/contributors?from=2021-05-25&to=2023-05-25&type=c
Sigh.

@Cheney-W Cheney-W added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label May 26, 2023
@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented May 26, 2023

I can't believe that this PR gets https://github.com/microsoft/vcpkg/labels/info%3Areviewed after being loudly reminded of the importance of updating the undermaintained openssl download.
vcpkg considered harmful 😠

@FrankXie05 FrankXie05 removed the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label May 26, 2023
@FrankXie05 FrankXie05 marked this pull request as draft May 26, 2023 07:44
Comment on lines +25 to +28
if(VCPKG_TARGET_IS_WINDOWS)
LIST(APPEND QUIC_TLS "schannel")
else()
LIST(APPEND QUIC_TLS "openssl")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using schannel limits msquic to Windows 11 only and as per docs This configuration does not support 0-RTT due to Schannel's current lack of support.. I think it makes more sense to still default to openssl even for Windows.

@BillyONeal
Copy link
Copy Markdown
Member

Closing this change in favor of #39332 . Thanks for your contribution!

@BillyONeal BillyONeal closed this Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-update The issue is with a library, which is requesting update new revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ms-quic]:x64-windows build failure

8 participants