Conversation
17 tasks
4fa2675 to
d4fed3d
Compare
1862889 to
6268389
Compare
6268389 to
da9dd99
Compare
c4c3f7b to
9301b0d
Compare
arogge
approved these changes
Aug 20, 2024
Member
arogge
left a comment
There was a problem hiding this comment.
I was a bit disappointed that the -w option didn't work together with -s so I added that feature. I also fixed a memory-leak when the key wasn't unwrappable.
I checked that the key semantics do not change when compared with master and also made sure I can read a tape written by Bareos with dd after utilizing bscrypto or stenc (the latter is a bit weird to setup, as it needs the key as hex-string).
All in all I'm pretty happy with the changes, even though that tool could use a lot of re-factoring.
Comment on lines
+343
to
412
| option(scsi-crypto "Enable scsi-crypto" ON) | ||
| option(ndmp "Enable NDMP support" ON) |
Member
There was a problem hiding this comment.
maybe we want to mark these two (i.e. scsi-crypto and ndmp) as advanced, too?
|
|
||
| static void usage() | ||
|
|
||
| static void TerminateBscrypto(int exitcode) |
Member
There was a problem hiding this comment.
Suggested change
| static void TerminateBscrypto(int exitcode) | |
| [[noreturn]] static void TerminateBscrypto(int exitcode) |
Comment on lines
+1
to
+6
| # bareos testing with mhvtl | ||
| # bareos env_keep += "MHVTL_HOME_PATH" | ||
| build ALL = (root) NOPASSWD:/usr/bin/systemctl start mhvtl.target | ||
| build ALL = (root) NOPASSWD:/usr/bin/systemctl stop mhvtl.target | ||
| build ALL = (root) NOPASSWD:/usr/bin/rm -rf /var/lib/mhvtl/* | ||
| build ALL = (root) NOPASSWD:/usr/bin/make_vtl_media --config-dir /etc/mhvtl |
Member
There was a problem hiding this comment.
do we still need this? I think the setcap part if sufficient, don't you think so?
365ac87 to
b529b39
Compare
`bstrncpy` makes sure the destination string is always null terminated, which results in the last character of the string being replaced with `0` which in turn makes the passphrase string missing its last character.
Remove the hardcoded 1 to explicit STDOUT_FILENO
to redirect job messages to stdout
passphrase is not a cstring (no null terminator) so it cannot be printed with %s; bstrncpy can copy length + 1 bytes but only length bytes were allocated; etc.
- cmake scsi-crypto option can be ON by default, it is only triggered if tapes device are present. - systemtests: cleanup cryptoc cache file. Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
- Fix ScsicryptoSd call in bareos-config - developerFAQ: quote tapedevice list Without the quote example in copy and paste call the second and beyond tape drives are called by sh (due to ; separator) Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
Those tests will be activated with cmake autochanger and tapes drives flag is present in cojunction with -Dscsi-crypto=ON. We use quadstore-vtl to test and design the tests. - add vtl functions to global scripts - systemtests: remove obsolete plugin option values
this enables -w to work together with -s.
b529b39 to
8714eea
Compare
in ctest there is no way to permanently hold a resource lock while a fixture is set up. Thus there could be cases where the different autochanger tests, even though there is a lock in place, would run intermixed which doesn't work. This now adds simple dependencies to make sure the next test using an autochanger waits for the cleanup of another one.
the test's cleanup script now removes key material and encryption settings from the tape drives when it is done.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bscrypto
This PR moves CLI parsing of bscrypto to CLI11, along with some code updates and refactoring.
We also add a specific systemtest
and review the related documentation
It will certainly need a backport to 23.
this is based on the old #1350
Please check
If you have any questions or problems, please give a comment in the PR.
Helpful documentation and best practices
Checklist for the reviewer of the PR (will be processed by the Bareos team)
Make sure you check/merge the PR using
devtools/pr-toolto have some simple automated checks run and a proper changelog record added.General
Required backport PRs have been createdSource code quality
Tests