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.

Sign up for GitHub

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

aria2: build with GNUTLS instead of OpenSSL #241620

Merged
merged 1 commit into from
Jul 28, 2023
Merged

aria2: build with GNUTLS instead of OpenSSL #241620

merged 1 commit into from
Jul 28, 2023

Conversation

msfjarvis
Copy link
Contributor

Description of changes

Updates the aria2 derivation to use GNUTLS for SSL in place of OpenSSL, as a workaround for issues aria2 has with TLS v1.3 while running on Linux.

The Darwin build works fine with or without this change.

Searching for "protocol error" in the aria2 issue tracker comes up with multiple issues citing trouble with the default aria2 builds that use OpenSSL. One of these issues claims that building aria2 with GNUTLS yields a build of aria2 that works with TLS v1.3 correctly.

aria2 from nixpkgs

$ nix run nixpkgs#aria2 -- https://catboe.moe
07/05 12:26:53 [NOTICE] Downloading 1 item(s)

07/05 12:26:54 [ERROR] CUID#7 - Download aborted. URI=https://catbox.moe
Exception: [AbstractCommand.cc:351] errorCode=1 URI=https://catbox.moe
  -> [SocketCore.cc:1018] errorCode=1 SSL/TLS handshake failure: protocol error

07/05 12:26:54 [NOTICE] Download GID#f86420f990651763 not complete: 

Download Results:
gid   |stat|avg speed  |path/URI
======+====+===========+=======================================================
f86420|ERR |       0B/s|https://catbox.moe

Status Legend:
(ERR):error occurred.

aria2 will resume download if the transfer is restarted.
If there are any errors, then see the log file. See '-l' option in help/man page for details.

aria2 from this patch

$ nix run github:msfjarvis/nixpkgs/hs/workaround-aria2c-tls-issues#aria2 -- https://catbox.moe

07/05 12:29:14 [NOTICE] Downloading 1 item(s)
[#7f09d7 0B/0B CN:1 DL:0B]                                                                                                                                                         
07/05 12:29:16 [NOTICE] Download complete: /home/msfjarvis/index.html

Download Results:
gid   |stat|avg speed  |path/URI
======+====+===========+=======================================================
7f09d7|OK  |   4.4KiB/s|/home/msfjarvis/index.html

Status Legend:
(OK):download completed.
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from Br1ght0ne July 5, 2023 08:04
@ajs124
Copy link
Member

ajs124 commented Jul 6, 2023

It would be nice if you could include some of the nice reasoning and research you did for this change into the commit message, because git is (much more) persistent than github.

If I'd find this commit in the history without looking at your PR description, I'd just be confused why someone would use gnutls over openssl.

@msfjarvis
aria2: build with GNUTLS instead of OpenSSL
29bd4a3 
aria2's OpenSSL integration breaks down when interacting with TLS v1.3
enabled websites which manifests in errors like these:

```
07/05 12:26:53 [NOTICE] Downloading 1 item(s)

07/05 12:26:54 [ERROR] CUID#7 - Download aborted. URI=https://catbox.moe
Exception: [AbstractCommand.cc:351] errorCode=1 URI=https://catbox.moe
  -> [SocketCore.cc:1018] errorCode=1 SSL/TLS handshake failure: protocol error
```

There are multiple instances[1] of users reporting this to the aria2 issue
tracker, and one of those issues[2] documents using GnuTLS in place of OpenSSL
as a workaround for the TLS v1.3 woes. I've verified that it indeed fixes
the problem, and hence making this change in Nixpkgs.

1: https://github.com/aria2/aria2/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+%22protocol+error%22
2: aria2/aria2#1494
@msfjarvis msfjarvis force-pushed the hs/workaround-aria2c-tls-issues branch from bb7ec32 to 29bd4a3 Compare July 6, 2023 12:08
@msfjarvis
Copy link
Contributor Author

It would be nice if you could include some of the nice reasoning and research you did for this change into the commit message, because git is (much more) persistent than github.

If I'd find this commit in the history without looking at your PR description, I'd just be confused why someone would use gnutls over openssl.

Thanks for pointing it out, I've added a condensed summary of the PR description along with links in the commit description and rebased it on the master branch.

@ajs124
Copy link
Member

ajs124 commented Jul 6, 2023

Result of nixpkgs-review pr 241620 run on x86_64-linux 1

25 packages built:
  • adl
  • ani-cli
  • anime-downloader
  • anime-downloader.dist
  • aria
  • aria.bin
  • aria.dev
  • aria.doc
  • aria.man
  • dra-cla
  • kiwix
  • media-downloader
  • persepolis
  • persepolis.dist
  • python310Packages.aria2p
  • python310Packages.aria2p.dist
  • python311Packages.aria2p
  • python311Packages.aria2p.dist
  • tartube
  • tartube-yt-dlp
  • tartube-yt-dlp.dist
  • tartube.dist
  • uget
  • uget-integrator
  • xivlauncher

@mweinelt mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jul 7, 2023
@ajs124 ajs124 merged commit 074e1da into NixOS:master Jul 28, 2023
@msfjarvis msfjarvis deleted the hs/workaround-aria2c-tls-issues branch July 28, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajs124 ajs124 ajs124 approved these changes

@Br1ght0ne Br1ght0ne Awaiting requested review from Br1ght0ne

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@msfjarvis @ajs124 @mweinelt