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

tpm2-pkcs11: init at 1.0.1 #72374

Merged
merged 1 commit into from Feb 1, 2020
Merged

tpm2-pkcs11: init at 1.0.1 #72374

merged 1 commit into from Feb 1, 2020

Conversation

lschuermann
Copy link
Member

This PR adds the TPM2 PKCS11 module as a Nix package.

It has a default target, which is the PKCS11 shared library (with the rpath correctly set to include all other required shared libraries). In addition to that, the bin target (selected by default for instance with nix-shell -p) makes the tpm2_ptool Python application for management available. Together with #72029 and this setup guide, usage is pretty straightforward.

Motivation for this change

I'd like to use my TPM2 chip as a generic Smartcard. This, for instance, enables storing SSH keys on a hardware device, which can improve security drastically.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Lassulus

@Lassulus
Copy link
Member

why not a stable version like 4.0.1?

@lschuermann
Copy link
Member Author

lschuermann commented Oct 31, 2019

I'm afraid there's no released version yet, according to the releases page at least.

However, I found this to be quite stable and working pretty well. I deem it useful for the Nix community and would therefore like to include it in nixpkgs. If there are any significant new features / critical bug fixes or an initial release, I'd of course file PRs for an update.


Alternatively, we can wait for a release which should come in the next weeks. I'd suggest to keep the PR open though to keep it only todo list ;).

@Lassulus
Copy link
Member

according to tags 4.0.1 was realesed 3 days ago

@lschuermann
Copy link
Member Author

I'm quite certain you're looking at tpm2-tools, not tpm2-pkcs11. Anyways, thanks for the hint, I'll create a PR for that update.

@Lassulus
Copy link
Member

ah indeed, then change the version to unstable-2019-09-04 and keep the rev and version separate

Copy link
Member

@0x4A6F 0x4A6F left a comment

Choose a reason for hiding this comment

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

Thanks, looks good.
Please update PR topic version string to unstable-2019-09-04.

@lschuermann
Copy link
Member Author

@0x4A6F I can do that, but recently noticed a few issues of the tpm2-pkcs11 software. I'd like to confirm that these are only affecting my system prior to merging first.

@lschuermann lschuermann changed the title tpm2-pkcs11: init at 0b7ceffb tpm2-pkcs11: init at 1.0.1 Jan 21, 2020
@lschuermann
Copy link
Member Author

lschuermann commented Jan 21, 2020

Was lange währt, wird auch nicht besser.

or will it?

After some delays, tpm2-pkcs11 is now finally at a 1.x.x major version, and appears to work quite well. Also, the database format (storing the private keys in encrypted form) should hopefully now be stable. I still advise to do backups.

Apart from a few issues with my packaging, I've tested this pretty thoroughly and it appears to work just fine. I even pushed my last commit using the TPM. 🎉

ping @Lassulus

in ''
patchelf \
--set-rpath ${rpath} \
${lib.optionalString abrmdSupport "--add-needed ${lib.makeLibraryPath [tpm2-abrmd]}/libtss2-tcti-tabrmd.so"} \
Copy link
Member Author

Choose a reason for hiding this comment

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

Somehow, just setting the RPATH (or rather RUNPATH) to include the required TCTI shared libraries for TPM communication does not appear to work when the modified file is itself loaded as a shared library. I am quite sure this once used to work for me.

However, adding the TCTIs as a NEEDED library appears to work. I have still included those in the shared object's RUNPATH. Is this the correct way to do this?

@lschuermann lschuermann mentioned this pull request Jan 21, 2020
10 tasks
pkgs/misc/tpm2-pkcs11/default.nix Outdated Show resolved Hide resolved
];

outputs = [ "out" "tools" ];
outputBin = "tools";
Copy link
Member

Choose a reason for hiding this comment

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

The output for binaries should be bin. And since with #72029 (comment) the library will be installed, you should also add a dev output to be able to install the library without the other bits.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe even add a lib output too, to explicitly have libraries in there

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, only the library is installed in the out output, no other bits. The only output containing any executables is in tools. I can change this to bin for sure. Is a dev output required then?

The library now has two outputs, "out" containing only the library, and "bin" containing only the tools. It works out of the box in nix-shell, as a build input and using the getLib.

Copy link
Member

Choose a reason for hiding this comment

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

out currently includes

result
├── lib
│  ├── pkgconfig
│  │  └── tpm2-pkcs11.pc
│  ├── libtpm2_pkcs11.la
│  ├── libtpm2_pkcs11.so -> libtpm2_pkcs11.so.0.0.0
│  ├── libtpm2_pkcs11.so.0 -> libtpm2_pkcs11.so.0.0.0
│  └── libtpm2_pkcs11.so.0.0.0
└── nix-support
   └── propagated-build-inputs

The pkgconfig and nix-support bits are development parts. I suggest doing this to split those:

     (python37.withPackages (ps: [ ps.pyyaml ps.cryptography ps.pyasn1-modules ]))
   ];
 
-  outputs = [ "out" "bin" ];
-  outputBin = "bin";
+  outputs = [ "out" "bin" "dev" ];
 
   dontStrip = true;
   dontPatchELF = true;

(and no need to assign outputBin because the default is bin already)

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I've integrated the requested changes. Now there are three outputs:

  • nix/store/452rf48l51c6rcybc217pclcyav9jdlx-tpm2-pkcs11-1.0.1
    └── lib
        ├── libtpm2_pkcs11.la
        ├── libtpm2_pkcs11.so -> libtpm2_pkcs11.so.0.0.0
        ├── libtpm2_pkcs11.so.0 -> libtpm2_pkcs11.so.0.0.0
        └── libtpm2_pkcs11.so.0.0.0
    
  • /nix/store/57ic3dsidp6lm4g1wn5h1d76xl0dlwqn-tpm2-pkcs11-1.0.1-dev
    ├── lib
    │   └── pkgconfig
    │       └── tpm2-pkcs11.pc
    └── nix-support
        └── propagated-build-inputs
    
  • /nix/store/16xlkaw49ayl10zi6wsbkja44qf3mbbf-tpm2-pkcs11-1.0.1-bin
    ├── bin
    │   └── tpm2_ptool
    └── share
        └── tpm2_pkcs11
            ├── tpm2_pkcs11
            │   ├── tpm2_ptool.py
            │   └── [...]
            ├── tpm2_ptool.py
            └── [...]
    

Good to know that this is the right(TM) way to do it.

pkgs/misc/tpm2-pkcs11/default.nix Outdated Show resolved Hide resolved
@lschuermann
Copy link
Member Author

@GrahamcOfBorg build tpm2-pkcs11

@lschuermann
Copy link
Member Author

@GrahamcOfBorg build tpm2-pkcs11

@infinisil infinisil merged commit 5c9198d into NixOS:master Feb 1, 2020
anna328p pushed a commit to anna328p/nixpkgs that referenced this pull request Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants