Hannes Duerr [Fri, 29 Mar 2024 15:07:36 +0000 (16:07 +0100)]
docs: move custom.js and custom.css into _static folder
The sphinx documentation [0] describes the _static folder as the
location for the custom.js and custom.css so we move the files there, as
we do not need those files outside the directory.
This also removes the error message when building:
WARNING: html_static_path entry '_static' does not exist
Gabriel Goller [Fri, 8 Mar 2024 13:36:22 +0000 (14:36 +0100)]
api: make prune-group a real workertask
`prune-group` is currently not a real workertask, ie it behaves like one
but doesn't start a thread nor a task to do its work.
Changed it to start a tokio-task, so that we can delete snapshots
asynchronously. The `dry-run` feature still behaves in the same way and
returns early.
This paves the way for the new logging infra (which uses `task_local` to
define a logger) and improves performance of bigger backup-groups.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
Markus Frank [Thu, 30 Nov 2023 10:37:24 +0000 (11:37 +0100)]
api: datastore create: allow re-using existing dirs if empty & not a mountpoint
When formatting and creating a filesystem on a disk it's important
that the target directory in `/mnt/datastore/<name>` either doesn't
exist yet, or is empty and not a mountpoint of an existing FS. As that
way we ensure that no data is lost, or gets hidden, on creating a new
datastore. Our current check was a bit stricter than required, it
always bailed if the target directory existed, even if it was a plain
& empty directory on the root file-system.
So adapt the check and also check whether an existing target directory
is empty and not already mounted, as then it can be used just fine.
Signed-off-by: Markus Frank <m.frank@proxmox.com> Tested-by: Christian Ebner <c.ebner@proxmox.com>
[ TL: reword subject and commit message to include more details ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Christian Ebner [Thu, 28 Mar 2024 12:36:25 +0000 (13:36 +0100)]
client: backup writer: only borrow http client
Instead of taking ownership of the http client when starting a new
BackupWriter instance, only borrow the client.
This allows to reuse the http client to later reuse it to start also a
BackupReader instance as required for backup runs with metadata based
file change detection mode, where both must use the same http client.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
instead of rejecting any non-leaf certificate not pre-validated by OpenSSL,
treat them as valid but keep track of the fact that the pre-validation result
is no logner trustable.
certificate chains completely trusted by openssl are still accepted like
before, and leaf certificates without a chain are also handled the same (since
the verify callback is only ever called with depth == 0 in that case).
Christoph Heiss [Fri, 12 Jan 2024 16:16:01 +0000 (17:16 +0100)]
realm sync: generic-ify `LdapSyncSettings` and `GeneralSyncSettings`
Since both only needs a handful of attributes anyway, pass them
explicitly instead of as an LDAP-specific config object, such that these
types can be reused for other realms like the new Active Directory one.
No functional changes.
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Christian Ebner [Fri, 8 Mar 2024 13:01:50 +0000 (14:01 +0100)]
api: sync job: log stats for removed vanished entities
Extend the current task log summary to include a log entry stating the
number of removed because vanished on the source side snapshots,
backup groups and namespaces.
The additional task log line states, e.g.:
> Summary: removed vanished: snapshots: 2, groups: 1, namespaces: 0
The log line is not shown if the sync jobs `remove_vanished` flag was
not set and therefore no removed vanished stats are present.
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Christian Ebner <c.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Christian Ebner [Fri, 8 Mar 2024 13:01:48 +0000 (14:01 +0100)]
datastore: group: return basic stats on backup group destroy
No functional change intended: In preparation for including the
removed vanished groups and snapshots statistics in a sync jobs task
log output.
Instead of returning a boolean value showing whether all of the
snapshots of the group have been removed, return an instance of
`BackupGroupDeleteStats`, containing the count of deleted and
protected snapshots, the latter not having been removed from the
group.
The `removed_all` method is introduced as replacement for the previous
boolean return value and can be used to check if all snapshots have
been removed. If there are no protected snapshots, the group is
considered to be deleted.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Gabriel Goller [Thu, 14 Mar 2024 09:32:24 +0000 (10:32 +0100)]
fix #5188: gui: add copy to clipboard on snapshots
When navigating to Datastores -> Content, it is now possible to
right-click on a snapshot/group and copy the name to the clipboard.
This makes the proxmox-backup-client much easier to use, especially when
restoring archives.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Gabriel Goller [Thu, 14 Mar 2024 09:32:23 +0000 (10:32 +0100)]
gui: remove document.execCommand calls
The `document.execCommand` call is deprecated since a few years [0] so I
went ahead and removed it. We only use it to copy stuff to the clipboard
and the recommended way now is to use `navigator.clipboard.writeText`
[1]. `writeText` is kind of new, but I think we'll be alright regarding
compatibility (Compat table is also available at [1]).
Making the handler functions async is okay because extjs executes the
handler and does not expect any result from it, nor does it need to do
some work afterwards.
Hannes Laimer [Mon, 4 Mar 2024 13:26:18 +0000 (14:26 +0100)]
datastore: remove datastore from internal cache based on maintenance mode
We keep a DataStore cache, so ChunkStore's and lock files are kept by
the proxy process and don't have to be reopened every time. However,
for specific maintenance modes, e.g. 'offline', our process should not
keep file in that datastore open. This clears the cache entry of a
datastore if it is in a specific maintanance mode and the last task
finished, which also drops any files still open by the process.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com> Reviewed-by: Gabriel Goller <g.goller@proxmox.com> Tested-by: Gabriel Goller <g.goller@proxmox.com>
add 'confirmation-password' parameter to user password change API/UI
Similar to a recent change in pve-access-control [0], add a new
'confirmation-password' parameter to the change-password endpoint and
require non-root users to confirm their passwords.
Doing so avoids that an attacker that has direct access to a computer
where a user is logged in to the PVE interface can change the password
of said user and thus either prolong their possibility to attack,
and/or create a denial of service situation, where the original user
cannot login into the PVE host using their old credentials.
Note that this might sound worse than it is, as for this attack to
work the attacker needs either:
- physical access to an unlocked computer that is currently logged in
to a PVE host
- having taken over such a computer already through some unrelated
vulnerability
As these required pre-conditions are pretty big implications, which
allow (temporary) access to all of the resources (including PVE ones)
that the user can control, we see this as slight improvement that
won't hurt, might protect one in some specific cases that is simply
too cheap not to do.
For now we avoid additional confirmation through a second factor, as
that is a much higher complexity without that much gain, and some
forms like (unauthenticated) button press on a WebAuthn token or the
TOTP code would be easy to circumvent in the physical access case and
in the local access case one might be able to MITM themselves too.
Thomas Lamprecht [Mon, 25 Mar 2024 13:08:12 +0000 (14:08 +0100)]
ui: re-use password edit window from widget-toolkit
no need to keep a copy of that component here, just re-use the common
one from widget-toolkit. That one provides also some more features
that will be used here with a next commit.
Originally-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: move switch to common widget up front ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Commit 2416aea8d456 accidentally removed this since they looked the
same as the ones we already have in proxmox-schema now. However, we
make use of the *capture groups* here.
Added a comment to the code to avoid this in the future.
Fixes 2416aea8d456 ("pbs-api-types: use const_format and new api-types from proxmox-schema") Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Stefan Lendl [Thu, 7 Mar 2024 14:23:43 +0000 (15:23 +0100)]
gitignore: ignore generated synopsis and config docs
Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
[ TL: condense this to something more general ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Wed, 6 Mar 2024 14:04:25 +0000 (15:04 +0100)]
ui: tape: transfer: increase timeout to 3 minutes
the default timeout of 30 seconds is too short to properly wait for a
slot transfer. Increase the timeout to a value of 3 minutes. In my
tests, it took about 60 seconds in a very basic changer to move a tape
between two slots, so triple that to account for bigger and more
complicated libraries.
Christian Ebner [Wed, 6 Mar 2024 14:11:53 +0000 (15:11 +0100)]
server: sync job: format downloaded amount in human readable units
Use the methods provided by HumanByte for the output for consistency
with the rest of the task log and better readability.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com> Reviewed-by: Max Carrara <m.carrara@proxmox.com> Tested-by: Max Carrara <m.carrara@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Adds a summary to the end of the task log showing the size and number
of chunks pulled as well as the average transfer rate.
Such an entry looks something like:
> Summary: sync job pulled 214.445 MiB in 166 chunks (average rate: 111.012 MiB/s)
Link: https://bugzilla.proxmox.com/show_bug.cgi?id=5285 Signed-off-by: Christian Ebner <c.ebner@proxmox.com> Reviewed-by: Max Carrara <m.carrara@proxmox.com> Tested-by: Max Carrara <m.carrara@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Christian Ebner [Wed, 6 Mar 2024 14:11:51 +0000 (15:11 +0100)]
server: sync job: return `PullStats` for pull related methods
Return basic statistics on pull related methods via `PullStats`
objects, in order to construct a global summary for sync jobs.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com> Reviewed-by: Max Carrara <m.carrara@proxmox.com> Tested-by: Max Carrara <m.carrara@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
values.username just does not exist, and we do not need to delete the
username part anyway, as that field is used to assemble the full
userid by concatenating the name@realm parts.
While at it move this over to let-assignments and do not call setting
expiry explicitly a hack, it's fine and warranted code, because if one
wants to use a datefield's empty value as 0 one needs to do so
explicitly, nothing hacky there..
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Sterz [Wed, 6 Mar 2024 11:45:56 +0000 (12:45 +0100)]
ui: user edit: don't send realm property
the api does not accept a realm property here, it is only needed to
construct a proper user id of the form `{username}@{realm}`. so
remove it before sending it to the api and getting an error in return.
Dominik Csapak [Wed, 6 Mar 2024 11:21:03 +0000 (12:21 +0100)]
ui: sync job: don't send delete value on creation
since that's not a valid api parameter there
we have to pass the `isCreate` value through to the inputpanel, we even
used it there already but it was never set.
Dominik Csapak [Wed, 6 Mar 2024 11:21:02 +0000 (12:21 +0100)]
ui: prune: fix sending invalid parameters
the prune input panel is used in various contexts (add/editing a
prunejob, adding a datastore, executing a prune). These different api
calls don't all take the same parameters, so we have to correctly set
the `isCreate` to not send a `delete` paramter for those request if
there was an empty field.
Also set 'max-depth:0' only when recursive was not set *and* we can
set 'recursive', because for creating a datastore that is not supported
by the api, and for the prune job editing we override the whole
onGetValues anyway so that's not an issue there.
Dominik Csapak [Wed, 6 Mar 2024 11:21:01 +0000 (12:21 +0100)]
ui: tape backup job: don't send delete value on creation
this is not a valid parameter for the create call. To do that in the
onGetValues method, we have to pass the 'isCreate' value through to the
input panels via cbind.
Dominik Csapak [Wed, 6 Mar 2024 11:21:00 +0000 (12:21 +0100)]
ui: metrics: don't send digest when creating a new influxdbupd host
we accidentally always tried to load an existing config, even when
creating a new entry. This returned the list of all configured ones plus
the digest (which gets set by the edit window). When the digest is set,
the edit window will send it along, but that does not exist for the
create api call, so it failed.
To fix it, guard the load behind the `serverid` property, which is only
set when we edit an existing entry.
Max Carrara [Fri, 1 Mar 2024 13:49:06 +0000 (14:49 +0100)]
fix #5217: api: send missing header when upgrading to HTTP/2
The "Connection: upgrade" header is strictly expected to be included
in the response sent by the server when an upgrade to a different
protocol is requested by the client.
A detailed explanation as well as additional context follows below.
Background
----------
Neither RFC 9110 (HTTP Semantics) [0] or RFC 7540 (HTTP/2) [1]
*explicitly state* that the "Connection: upgrade" header must be
included *in the server's response* when a client requests an upgrade
to a different protocol. For clients, however, it is specified [2]:
> A sender of Upgrade MUST also send an "Upgrade" connection option in
> the Connection header field (Section 7.6.1) to inform intermediaries
> not to forward this field.
Yet, the example for a response provided in RFC 9110 [3] does include
the header:
> HTTP/1.1 101 Switching Protocols
> Connection: upgrade
> Upgrade: websocket
>
> [... data stream switches to websocket with an appropriate response
> (as defined by new protocol) to the "GET /hello" request ...]
The example in RFC 7540 [4] also includes the header:
Additionally, RFC 9113 [5], which obsoletes RFC 7540 [1], mentions:
> The HTTP/1.1 Upgrade mechanism is deprecated and no longer specified
> in this document. It was never widely deployed, with plaintext
> HTTP/2 users choosing to use the prior-knowledge implementation
> instead.
I therefore initially concluded that whether the "Connection: upgrade"
header should / should not / must / must not be included in the
server's response was unspecified.
Further Revelations
-------------------
As per Thomas's suggestion [6], I opened a discussion over at Caddy's
GitHub issue tracker [7]. This discussion revealed that RFC 7230 [8],
which is obsoleted by RFC 9110 [1], does in fact specify that the
header must be included [9], thus proving my initial conclusion to be
incorrect:
> When a header field aside from Connection is used to supply control
> information for or about the current connection, the sender MUST
> list the corresponding field-name within the Connection header
> field. [...]
The discussion [7] also revealed that the WebSocket RFC 6455 [10]
specifies the usage of the "Connection" header in more detail [11]:
> 3. If the response lacks a |Connection| header field or the
> |Connection| header field doesn't contain a token that is an ASCII
> case-insensitive match for the value "Upgrade", the client MUST
> _Fail the WebSocket Connection_.
Furthermore [12]:
> 5. If the server chooses to accept the incoming connection, it
> MUST reply with a valid HTTP response indicating the following.
>
> [...]
>
> 3. A |Connection| header field with value "Upgrade".
Although we're using the upgrade mechanism for HTTP/2, the WebSocket
RFC [10] specifies its usage more clearly and most importantly, in an
explicit manner.
Final Conclusion
----------------
The "Connection: upgrade" header must therefore definitely be included
as per RFC 7230 section 6.1 [8], even if the newer RFC 9110 [1] does
not specify this explicitly anymore.
Finally, this fixes bug #5217 [13] and allows PBS to be deployed
behind Caddy. Also tested with nginx, which still works as expected.
Dominik Csapak [Fri, 1 Mar 2024 09:03:41 +0000 (10:03 +0100)]
ui: system config: fix deleting DNS entries
While PVE and PMG use a rather brittle "replace whole config" style on
their DNS entry CRUD API, the PBS one was made with a per-entry level
granularity, so that single entries can modified, or deleted, without
touching the others.
But the UI from the widget-toolkit was made for the older PVE/PMG
behavior and did not sent along the delete-array of to-be-deleted
keys.
Since widget-toolkit commit 8d161ac ("dns: update comment to avoid
coupling to downstream dependency") the DNS edit window supports
opting into that by setting the new `deleteEmpty` config parameter.
Stefan Sterz [Fri, 1 Mar 2024 09:40:19 +0000 (10:40 +0100)]
ui: trim whitespaces when adding a subscription key
users that add the correct subscription key just get unnecessarily
confused with a "value does not match the regex pattern" error if
they accidentally have a stray whitespace at the end or beginning
otherwise.
Switch to using our `proxmoxtextfield` component that provides a
`trimValue` config option since widget-toolkit commit 5d7d30d ("text
field: add trimValue config") that was made just for this case.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
[ TL: reference widget toolkit commit ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Gabriel Goller [Wed, 14 Feb 2024 09:52:01 +0000 (10:52 +0100)]
fix #4975: client: ignore E2BIG error flag
Some filesystems (f.e. zfs) support xattrs bigger than 64kB, sadly we
can't get them because the kernel vfs limits us. The syscalls listxattr
and getxattr will return a E2BIG error in this case.
Added a flag --ignore-e2big-xattr to the client, this will ignore the
metadata (but still backup the file) if this error occurs.
Signed-off-by: Gabriel Goller <g.goller@proxmox.com> Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
```
warning: in a `match` scrutinee, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
--> src/bin/proxmox-backup-proxy.rs:874:58
|
874 | let stats = match tokio::task::spawn_blocking(|| {
| __________________________________________________________^
875 | | let hoststats = collect_host_stats_sync();
876 | | let (hostdisk, datastores) = collect_disk_stats_sync();
877 | | Arc::new((hoststats, hostdisk, datastores))
878 | | })
| |_________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions
= note: `#[warn(clippy::blocks_in_conditions)]` on by default
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
```
warning: `to_string` applied to a type that implements `Display` in `writeln!` args
--> src/server/report.rs:141:72
|
141 | let _ = writeln!(out, "error during read-dir - {}", err.to_string());
| ^^^^^^^^^^^^ help: remove this
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
access first element with first() rather than get(0)
Fixes the clippy lint
```
warning: accessing first element with `self.transports.get(0)`
--> pbs-tape/src/lib.rs:283:9
|
283 | / self.transports
284 | | .get(0)
| |___________________^ help: try: `self.transports.first()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#get_first
= note: `#[warn(clippy::get_first)]` on by default
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
use or_default instead of or_insert_with(Default::default)
We need to annotate some cases to allow the compile to infer the types.
Fixes the clippy lint:
```
warning: use of `or_insert_with` to construct default value
--> src/api2/tape/restore.rs:750:18
|
750 | .or_insert_with(Vec::new);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_or_default
= note: `#[warn(clippy::unwrap_or_default)]` on by default
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
```
warning: using `SeekFrom::Current` to start from current position
--> src/tape/media_catalog.rs:798:23
|
798 | let pos = file.seek(SeekFrom::Current(0))?; // get current pos
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `file.stream_position()`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#seek_from_current
= note: `#[warn(clippy::seek_from_current)]` on by default
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
```
warning: the borrowed expression implements the required traits
--> src/server/report.rs:193:47
|
193 | get_directory_content(&path)
| ^^^^^ help: change this to: `path`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
= note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
```
warning: redundant explicit link target
--> src/tools/mod.rs:47:42
|
47 | /// Returns a new instance of [`Client`](proxmox_http::client::Client) configured for PBS usage.
| -------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ explicit target is redundant
| |
| because label contains path that resolves to same destination
|
= note: when a link's destination is not specified,
the label is used to resolve intra-doc links
= note: `#[warn(rustdoc::redundant_explicit_links)]` on by default
help: remove explicit link target
|
47 | /// Returns a new instance of [`Client`] configured for PBS usage.
| ~~~~~~~~~~
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Dominik Csapak [Mon, 12 Feb 2024 12:27:34 +0000 (13:27 +0100)]
fix #5229: tape: remove max sequence number limit
The idea was to limit the number of tapes in a media set, but this was
not enforced when adding a medium to a media set, only on read/parsing
the inventory. With that, it is possible to create media sets greater
than the limit which in turn blocks access to most functions via
api/cli/gui due to the check.
Instead of enforcing an arbitrary limit, simply warn on creation when
the media-set is very large (20).
To restore the whole media set, the time taken would still be at least 38
hours for LTO-4 and 250 hours for LTO-9.
We already have a section in the docs where we tell about the
disadvantages of large media sets.
Gabriel Goller [Tue, 6 Feb 2024 10:09:07 +0000 (11:09 +0100)]
fix #5190: api: OIDC: accept generic URIs for the ACR value
Allow more complex strings for the acr-value when using openid. The
openid documentation only specifies the acr-value *should* be an URI
[0]. Implemented a regex that loosely disallows some of the reserved
URI characters specified in the RFC [1].
Currently values like:
- "urn:mace:incommon:iap:silver"
- "urn:comsolve.nl:idp:contract:rba:location"
do NOT work, although they are correct URI's and common acr tokens.
For Proxmox VE we had to actually make this more strict to align with
each other, as there we accepted any string.
Since we additonally also support delcaring a "type" property for
`oneOf` schemas (to use with serde's *internally* tagged enum
representation, this contains an additional `typeProperty` and
`typeSchema` value.
It dumps as follows:
{
"type": "object",
"description": ...,
"typeProperty": "name-of-type-property",
"typeSchema": {
"type": "string",
"enum": [ ... ], // technically not enforced by the code
},
"oneOf": [
{
"title": "<value from the above 'enum' array>",
<schema>,
},
{
"title": "<value from the above 'enum' array>",
<schema>,
},
... <one for each 'enum' above>
// ^ exact match is not technically enforced by code
}
}
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
tape: factor getting encryption fingerprint tuple out
makes it a bit more readable as there's less "noise" in the read_label
function and as the separate new fn allows us to nicely use ? to early
return as it has an option in the return signature avoiding 5 lines of
code while not really getting more terse.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Wed, 31 Jan 2024 13:42:33 +0000 (14:42 +0100)]
tape: fix regression in restoring key from medium
Since commit 1343dcaf we automatically try to load the key into the
drive after reading the media-set label, this cannot work for the case
where we actually restore the key from the tape itself.
To address this special case while preserving the automatic key
loading, everything except the setup of the key has been separated
from the 'read_label' method into a new function named
'read_label_without_loading_key'. Consequently, the 'restore-key' API
endpoint can be switched to utilize this new method, thereby avoiding
the issue.
Fixes: 1343dcaf ("tape: move 'set_encryption' calls to the TapeDriver") Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: reword and shorten commit message ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Nightly rustc now warns about unused private fields in the case of a
non-pub newtype struct, so use an underscore-prefixed dummy field name
to get rid of the warning.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Dominik Csapak [Mon, 22 Jan 2024 11:50:34 +0000 (12:50 +0100)]
tape: assert encryption mode when using the PoolWriter
by introducing an 'assert_encryption_mode' that checks the desired
state, and bails out if it's different, called directly where we
previously set the encryption mode (which is now done automatically)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: add drive_ prefix and fleece in comment ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Mon, 22 Jan 2024 11:50:33 +0000 (12:50 +0100)]
tape: move 'set_encryption' calls to the TapeDriver (and implementation)
namely everytime we know what the key for the tape has to be:
* after we write the MediaSetLabel
* after reading the MediaSetLabel
When handling data on tape, we always have to have the MediaSetLabel, so
we should always trigger one of these. Because of that, we should not be
able to forget to set the encryption mode.
Dominik Csapak [Mon, 22 Jan 2024 11:50:32 +0000 (12:50 +0100)]
tape: fix wrongly unloading encryption key
For security, we want to automatically unload the encryption key from
the drive when we're done, so there was a Drop handler for SgTape that
handles that. Sadly, our tool we use to set it in the first place, also
invoked the Drop handler, thus unloading the keys again immediately
To fix that, move the Drop handler one logical level higher to the
LtoTapeHandle, which is not used by the 'sg-tape-cmd'.
Dominik Csapak [Mon, 22 Jan 2024 11:50:30 +0000 (12:50 +0100)]
tape: use SgTape in sg-tape-cmd
instead of LtoTapeHandle. This way, we can simply always call the binary
from LtoTapeHandle, and don't have to concern ourselves with the sg_tape
calling.