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

Port libkeyutils tests #34

Merged
merged 47 commits into from
Jan 10, 2020
Merged

Port libkeyutils tests #34

merged 47 commits into from
Jan 10, 2020

Conversation

mathstuf
Copy link
Owner

@mathstuf mathstuf commented Jul 8, 2019

Continuted work towards #32.

TODO:

  • fix formatting in each commit
  • add KDF API/tests

Cc: @josephlr

@mathstuf
Copy link
Owner Author

mathstuf commented Jul 8, 2019

Seems that the CI containers do not have a new enough libkeyutils for keyctl_dh_compute (another reason to move off the C library and return a Rust-y version of ENOSYS instead). See #24.

@mathstuf
Copy link
Owner Author

mathstuf commented Jul 8, 2019

Marking it as a weak symbol seems to be a perma-unstable solution: rust-lang/rust#29603. For now, I'll comment out the dh_compute tests and once we finish #24, they can be added back again with the appropriate ENOSYS detection logic.

@mathstuf mathstuf force-pushed the port-libkeyutils-tests branch 2 times, most recently from 99b6361 to 318a35a Compare July 30, 2019 11:45
@mathstuf
Copy link
Owner Author

Seems that the EPERM error occurs more often on the CI machines than locally. Looking into the kernel as to what codepath makes that happen.

// Avoid a panic in the code below be ensuring that we actually have a keyring. Parsing
// a key's payload as a keyring payload.
let desc = self.description()?;
if desc.type_ != keytypes::Keyring::name() {
Copy link
Owner Author

Choose a reason for hiding this comment

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

See #35 for a better way of doing this.

@mathstuf
Copy link
Owner Author

There are some tests which end up with EDQUOT (quota reached) when run in parallel. Tracking down what tests are overflowing should probably be guarded with #[serial]. Or we just commit to running "rerun tests" when it happens. It's not common, but not rare either.

@mathstuf mathstuf force-pushed the port-libkeyutils-tests branch 2 times, most recently from a4384cb to 6fb0df5 Compare August 14, 2019 13:48
The documentation states that a keyring may be given or a special
keyring used. The special IDs are looked up in the requesting process'
namespace. An ID of `0` means that no link is requested.
The parser for the key payload is blindly trusting that the key ID is
actually a keyring and not transformed or replaced in some way. Guard
against this case be ensuring that we actually have a keyring ID.
The destination keyring does not need to be the same as the one that is
searched. Fix the API and remove the now-broken tests.
The API did not match what is actually possible with the kernel API.
Instead, just offer two simple methods which do what is needed.

Also remove the old test which doesn't match what the API looks like
anymore.
This is more ergonomic and allows for just passing a plain string
around.
Some tests handle it manually to test behavior once the keyring is
deallocated, so leave the old functionality there.
@codecov
Copy link

codecov bot commented Jan 10, 2020

Codecov Report

Merging #34 into master will increase coverage by 22.61%.
The diff coverage is 98.91%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #34       +/-   ##
===========================================
+ Coverage    67.1%   89.71%   +22.61%     
===========================================
  Files          19       33       +14     
  Lines         766     2032     +1266     
===========================================
+ Hits          514     1823     +1309     
+ Misses        252      209       -43
Impacted Files Coverage Δ
src/constants.rs 100% <ø> (ø) ⬆️
src/keytypes/encrypted.rs 0% <0%> (ø) ⬆️
src/tests/update.rs 100% <100%> (ø)
src/tests/revoke.rs 100% <100%> (ø)
src/tests/utils/mod.rs 100% <100%> (ø)
src/tests/invalidate.rs 100% <100%> (ø)
src/tests/describe.rs 100% <100%> (ø)
src/tests/link.rs 100% <100%> (ø)
src/tests/search.rs 100% <100%> (ø)
src/tests/add.rs 98.93% <100%> (+0.78%) ⬆️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5d26a4...7728eda. Read the comment docs.

@mathstuf
Copy link
Owner Author

Well, this is far better.

@mathstuf mathstuf merged commit 04e4f30 into master Jan 10, 2020
@mathstuf mathstuf deleted the port-libkeyutils-tests branch January 10, 2020 01:47
@mathstuf mathstuf mentioned this pull request May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant