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

veracrypt: 1.22 -> 1.23 / truecrypt: remove and alias to veracrypt #49074

Merged
merged 2 commits into from Oct 29, 2018
Merged

veracrypt: 1.22 -> 1.23 / truecrypt: remove and alias to veracrypt #49074

merged 2 commits into from Oct 29, 2018

Conversation

c0bw3b
Copy link
Contributor

@c0bw3b c0bw3b commented Oct 24, 2018 •
edited

Motivation for this change

Update veracrypt to latest version 1.23
Release notes : nothing critical security-wise

cc @dsferruzza for testing and review

+
TrueCrypt has been retired (upstream) for a while now and the source archive we pointed to is gone. Moreover the VeraCrypt fork is available, maintained and fixes issues previous audits found in TrueCrypt.
Cryptsetup is also able to mount TC volumes.
This PR proposes to remove the (broken) package and keep an alias to point to the backward-compatible fork VeraCrypt.

  • the change is documented in 19.03 release notes
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@c0bw3b
truecrypt: remove and alias to veracrypt
b47fccf 
TrueCrypt has been retired for a while now and the source archive we
pointed to is gone. Moreover the VeraCrypt fork is available, maintained
and fixes issues previous audits found in TrueCrypt.
@c0bw3b
veracrypt: 1.22 -> 1.23
ce67a63 
Removed the wxGUI switch because it was never working.
To build VeraCrypt text-only you still need wxGTK _and_ you need to
rebuild wxGTK without GUI too...
Moreover, the normal build provides both GUI and text-only interfaces
so it is already usable in pure CLI contexts.
@c0bw3b
Copy link
Contributor Author

c0bw3b commented Oct 24, 2018

I forgot to explain the removal of the wxGUI switch in veracrypt : in fact it was never working (build failing if set to false).

To build VeraCrypt text-only you still need wxGTK and you need to rebuild wxGTK without GUI too...
See https://sourceforge.net/p/veracrypt/tickets/30/
So this is totally unpractical and it won't reduce the closure size...

Moreover, the normal build provides both GUI and text-only interfaces so it is perfectly usable in pure CLI contexts already. The NOGUI build doesn't make much sense in the end.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: veracrypt

Partial log (click to expand)

                            ^~~~~~~~
Linking veracrypt
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/i3vcsyi7vk1ypnly6ldiz84k1gzvri6l-veracrypt-1.23
shrinking /nix/store/i3vcsyi7vk1ypnly6ldiz84k1gzvri6l-veracrypt-1.23/bin/veracrypt
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/i3vcsyi7vk1ypnly6ldiz84k1gzvri6l-veracrypt-1.23/bin
patching script interpreter paths in /nix/store/i3vcsyi7vk1ypnly6ldiz84k1gzvri6l-veracrypt-1.23
checking for references to /build in /nix/store/i3vcsyi7vk1ypnly6ldiz84k1gzvri6l-veracrypt-1.23...

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: veracrypt

Partial log (click to expand)

Linking veracrypt
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/sbanv44bmij03xrbiql6f7lg18511b6x-veracrypt-1.23
shrinking /nix/store/sbanv44bmij03xrbiql6f7lg18511b6x-veracrypt-1.23/bin/veracrypt
strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/sbanv44bmij03xrbiql6f7lg18511b6x-veracrypt-1.23/bin
patching script interpreter paths in /nix/store/sbanv44bmij03xrbiql6f7lg18511b6x-veracrypt-1.23
checking for references to /build in /nix/store/sbanv44bmij03xrbiql6f7lg18511b6x-veracrypt-1.23...
/nix/store/sbanv44bmij03xrbiql6f7lg18511b6x-veracrypt-1.23

@dsferruzza
Copy link
Member

Hi @c0bw3b !
Thanks for this PR!

This makes sense for me to remove TrueCrypt: VeraCrypt is more mature/secure/tested and can mount TC volumes. Not to mention TrueCrypt has been abandoned years ago...

I have 2 questions about ce67a63:

  1. You remove code that allowed to build VC without GUI support (to use it via CLI); why?
  2. Why did you changed the license? It seems that the VeraCrypt License is still the one (https://www.veracrypt.fr/code/VeraCrypt/tree/License.txt).

Also, for my culture, what is the install command you used in the installPhase script?

Have a nice day!

@c0bw3b
Copy link
Contributor Author

c0bw3b commented Oct 25, 2018 •
edited

Hello @dsferruzza

All good questions so here we go:

  1. I explained the reason to remove the wxGUI build switch in my comment above : basically setting NOGUI=1 was not sufficient to build VC with text-only interface and the normal build exposes both GUI and text UI (veracrypt --text --help for example or short form veracrypt -t -h) so it is perfectly usable via CLI even when built with wxGTK

  2. Licensing: official website explains that VC is dual-licensed under Apache License 2.0 and TrueCrypt License version 3.0. I added both in our package definition to translate this dual-licensing. Moreover, since the TC license is not really considered a free/FLOSS license, adding the ASL2 gives us some 'warranty' that we can indeed redistribute VeraCrypt as a free software

  3. install is just a coreutils command that is useful for packaging because it allows to set rights, modes, owners and create directories as well as copying files. Basically does the same thing as mkdir+cp+chown but I favor one-liners 😸

Nice day to you too and thanks for reviewing.

/cc @ryantm too as maintainer of truecrypt

ryantm
ryantm approved these changes Oct 25, 2018
View reviewed changes
Copy link
Member

@ryantm ryantm left a comment

Choose a reason for hiding this comment

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

Looks good to me. You might consider allowing the wxGUI to be passed in, but if it is, failing with a nicer error message.

@c0bw3b
Copy link
Contributor Author

c0bw3b commented Oct 26, 2018

Technically feasible but I can't see a use case where this would be useful (a boolean switch that would just make the build fail).
Keeping in mind that it was already failing up to now, so anyone trying to do it out of tree would see the build failure.

Thank you for the review and the approval nonetheless! @ryantm

@xeji xeji merged commit 21a7ca7 into NixOS:master Oct 29, 2018
@c0bw3b c0bw3b deleted the pkg/veracrypt branch October 31, 2018 08:29
@c0bw3b c0bw3b added this to To do in Picking up garbage via automation May 10, 2019
@c0bw3b c0bw3b moved this from To do to Done in Picking up garbage May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryantm ryantm ryantm approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: clean-up 8.has: documentation 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
Picking up garbage
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@c0bw3b @GrahamcOfBorg @dsferruzza @ryantm @xeji