[pbs-devel] [PATCH backup v3 1/2] http_client: store tickets in the user's config directory

[pbs-devel] [PATCH backup v3 1/2] http_client: store tickets in the user's config directory

[Maximiliano Sandoval]

The environment variable XDG_RUNTIME_DIR is only set if the user is
logged into a seat. If, for example, the backup client was run with
`sudo` then the ticket would not be a stored.

By storing the ticket in the user's configuration directory, it can be
reused later if the user logs out.

Since the tickets are only valid for a limited time, it is not a problem
if this file is not automatically cleaned.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/http_client.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
index c95def07b..4f525f2ec 100644
--- a/pbs-client/src/http_client.rs
+++ b/pbs-client/src/http_client.rs
@@ -218,8 +218,8 @@ pub struct HttpClient {
 pub fn delete_ticket_info(prefix: &str, server: &str, username: &Userid) -> Result<(), Error> {
     let base = BaseDirectories::with_prefix(prefix)?;
 
-    // usually /run/user/<uid>/...
-    let path = base.place_runtime_file("tickets")?;
+    // usually ~/.config/<prefix>/tickets
+    let path = base.place_config_file("tickets")?;
 
     let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
 
@@ -307,8 +307,8 @@ fn store_ticket_info(
 ) -> Result<(), Error> {
     let base = BaseDirectories::with_prefix(prefix)?;
 
-    // usually /run/user/<uid>/...
-    let path = base.place_runtime_file("tickets")?;
+    // usually ~/.config/<prefix>/tickets
+    let path = base.place_config_file("tickets")?;
 
     let mode = nix::sys::stat::Mode::from_bits_truncate(0o0600);
 
@@ -347,8 +347,8 @@ fn store_ticket_info(
 fn load_ticket_info(prefix: &str, server: &str, userid: &Userid) -> Option<(String, String)> {
     let base = BaseDirectories::with_prefix(prefix).ok()?;
 
-    // usually /run/user/<uid>/...
-    let path = base.place_runtime_file("tickets").ok()?;
+    // usually ~/.config/<prefix>/tickets
+    let path = base.place_config_file("tickets").ok()?;
     let data = file_get_json(path, None).ok()?;
     let now = proxmox_time::epoch_i64();
     let ticket_lifetime = proxmox_auth_api::TICKET_LIFETIME - 60;
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH backup v3 2/2] http_client: add warning when we fail to place a config file

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/http_client.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
index 4f525f2ec..ba9892218 100644
--- a/pbs-client/src/http_client.rs
+++ b/pbs-client/src/http_client.rs
@@ -348,7 +348,12 @@ fn load_ticket_info(prefix: &str, server: &str, userid: &Userid) -> Option<(Stri
     let base = BaseDirectories::with_prefix(prefix).ok()?;
 
     // usually ~/.config/<prefix>/tickets
-    let path = base.place_config_file("tickets").ok()?;
+    let path = base
+        .place_config_file("tickets")
+        .inspect_err(|err| {
+            warn!("could not place tickets file in the user's config directory: {err}")
+        })
+        .ok()?;
     let data = file_get_json(path, None).ok()?;
     let now = proxmox_time::epoch_i64();
     let ticket_lifetime = proxmox_auth_api::TICKET_LIFETIME - 60;
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH backup v3 1/2] http_client: store tickets in the user's config directory

[Thomas Lamprecht]

Am 16.04.25 um 14:56 schrieb Maximiliano Sandoval:
> The environment variable XDG_RUNTIME_DIR is only set if the user is
> logged into a seat. If, for example, the backup client was run with
> `sudo` then the ticket would not be a stored.
> 
> By storing the ticket in the user's configuration directory, it can be
> reused later if the user logs out.

Hmm, but XDG_CONFIG_HOME does not have to point to ~/.config, so is
this really solving the problem?

Would it maybe be nicer to keep the default in XDG_RUNTIME_DIR and
fallback to some other mechanism, like kernel keyring or alternatively
maybe systemd creds?

The [keyrings manpage] description would make it seem like an ideal
candidate for such things:

"The Linux key-management facility is primarily a way for various
kernel components to retain or cache security data, authentication
keys, encryption keys, and other data in the kernel."

I.e., the ticket _is_ security data and an authentication key that
needs to be cached. One can even set an expiry time for such keys.


[keyrings manpage]: https://manpages.debian.org/bookworm/manpages/keyrings.7.en.html

> Since the tickets are only valid for a limited time, it is not a problem
> if this file is not automatically cleaned.

But that's also why it certainly isn't a config, so feels IMO also
wrong besides above point about this being rather a lateral move.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH backup v3 1/2] http_client: store tickets in the user's config directory

[Maximiliano Sandoval]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> Am 16.04.25 um 14:56 schrieb Maximiliano Sandoval:
>> The environment variable XDG_RUNTIME_DIR is only set if the user is
>> logged into a seat. If, for example, the backup client was run with
>> `sudo` then the ticket would not be a stored.
>> 
>> By storing the ticket in the user's configuration directory, it can be
>> reused later if the user logs out.
>
> Hmm, but XDG_CONFIG_HOME does not have to point to ~/.config, so is
> this really solving the problem?

The xdg crate will error out if XDG_RUNTIME_DIR is not set, however it
will fall back to ~/.config if the XDG_CONFIG_HOME env variable is not
set. That is the key difference.

> Would it maybe be nicer to keep the default in XDG_RUNTIME_DIR and
> fallback to some other mechanism, like kernel keyring or alternatively
> maybe systemd creds?

That could be done. In v1 I proposed simply using `/run/proxmox-backup`
(or /run/user/$uid/proxmox-backup), but a different mechanism could be
used.

Regarding kernel keyring or systemd creds, at least the later requires
root access as of debian 12. An issue common to these three mechanisms
is that they all make assumptions about permissions, the backup client
could be run as an arbitrary user which might not have permissions to
any of /run, the keyring, nor the system credentials.

> The [keyrings manpage] description would make it seem like an ideal
> candidate for such things:
>
> "The Linux key-management facility is primarily a way for various
> kernel components to retain or cache security data, authentication
> keys, encryption keys, and other data in the kernel."
>
> I.e., the ticket _is_ security data and an authentication key that
> needs to be cached. One can even set an expiry time for such keys.
>
>
> [keyrings manpage]: https://manpages.debian.org/bookworm/manpages/keyrings.7.en.html
>
>> Since the tickets are only valid for a limited time, it is not a problem
>> if this file is not automatically cleaned.
>
> But that's also why it certainly isn't a config, so feels IMO also
> wrong besides above point about this being rather a lateral move.

I think this was a massive mental lapsus on my end, this should have
been ~/.cache and definitively not ~/.config.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH backup v3 1/2] http_client: store tickets in the user's config directory

[Thomas Lamprecht]

Am 18.04.25 um 14:47 schrieb Maximiliano Sandoval:
> Regarding kernel keyring or systemd creds, at least the later requires
> root access as of debian 12. An issue common to these three mechanisms
> is that they all make assumptions about permissions, the backup client
> could be run as an arbitrary user which might not have permissions to
> any of /run, the keyring, nor the system credentials.

The kernel keyring does not read root, it works as every user and has
a per user (uid) keyring as one of it default keyrings, which makes it
quite neat and normally exactly the right choice for such stuff.

You can use the keyctl as CLI tool to experiment with the keyring without
having to write a program using the syscalls directly.

The following example works just fine a standard user, as it should work
for every UID on the system. It adds a user key named "test" in the "@u"
uid keyring:

  keyctl add user test "super secure ticket" @u

It returns the key serial number (ID), but you can also list the keys
from a keyring:

  keyctl list @u


Or search by name:

  keyctl search @u user test

And then use the serial number to read the content, e.g.:

  keyctl print 304368094

The only "downside" is that it won't survive a reboot, so if a user
frequently reboots but would like to stay logged in then this won't
work, but tbh. I'd just tell them: though luck, use an API token and
handle passing it yourself in that case that is IMO rather.

I'd not be surprised if Wolfgang either has rust code for accessing
the keyring nicely or maybe knows a sane and current crate for that.
In any case, this seems like a much nicer solution all around to me,
but I naturally could have overlooked some drawback.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH backup v3 1/2] http_client: store tickets in the user's config directory

[Maximiliano Sandoval]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> Am 18.04.25 um 14:47 schrieb Maximiliano Sandoval:
>> Regarding kernel keyring or systemd creds, at least the later requires
>> root access as of debian 12. An issue common to these three mechanisms
>> is that they all make assumptions about permissions, the backup client
>> could be run as an arbitrary user which might not have permissions to
>> any of /run, the keyring, nor the system credentials.
>
> The kernel keyring does not read root, it works as every user and has
> a per user (uid) keyring as one of it default keyrings, which makes it
> quite neat and normally exactly the right choice for such stuff.
>

Interesting. I will take a look.

>
> You can use the keyctl as CLI tool to experiment with the keyring without
> having to write a program using the syscalls directly.
>
> The following example works just fine a standard user, as it should work
> for every UID on the system. It adds a user key named "test" in the "@u"
> uid keyring:
>
>   keyctl add user test "super secure ticket" @u
>
> It returns the key serial number (ID), but you can also list the keys
> from a keyring:
>
>   keyctl list @u
>
>
> Or search by name:
>
>   keyctl search @u user test
>
> And then use the serial number to read the content, e.g.:
>
>   keyctl print 304368094
>
> The only "downside" is that it won't survive a reboot, so if a user

Not an issue in this particular case, as the current approach won't
survive the user logging out either.

> frequently reboots but would like to stay logged in then this won't
> work, but tbh. I'd just tell them: though luck, use an API token and
> handle passing it yourself in that case that is IMO rather.
>
> I'd not be surprised if Wolfgang either has rust code for accessing
> the keyring nicely or maybe knows a sane and current crate for that.
> In any case, this seems like a much nicer solution all around to me,
> but I naturally could have overlooked some drawback.

Regarding the system credentials, it is also possible to use them as a
regular user on newer systemd versions (than the one offered by Debian
12), however I have not read yet about what security guarantees they
offer.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel