Re: [pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured
I think this is https://bugzilla.proxmox.com/show_bug.cgi?id=3739 ;)
@wolfgang could you take a look at this?
On September 14, 2022 3:42 pm, Dominik Csapak wrote:
> With change to our new tfa mechanism, we now lock the tfa config
> when verifying the second factor and when creating the challenge for it
> that makes sense, since at least one tfa type can change the config
> (recovery keys must be deleted from there).
>
> The downside is that we cannot authenticate users anymore without quorum
> (since locking requires write access to pmxcfs), even for users without
> tfa configured (and also for clusters without any tfa configured at all).
>
> With this patch, we loosen that restriction a bit, by checking if the
> user has tfa configured outside the lock. We still query it again
> inside the lock to get the current config inside the lock again.
> (slightly out of the diff context)
>
> There is a minimal (IMHO unimportant) race here:
> if a user is logging in and for this user tfa is configured
> simultaneously, it may happen that during the first check tfa is still
> not present.
>
> This is not a real problem though, since a logged in user will not be
> logged out when a tfa is configured, so it's the same as when the user
> would have logged in before the tfa is being configured.
>
> There were quite a bit confused users that ran into that, and preventing
> them from logging in at all because of a not quorate server (when
> there is not a good technical reason for it) seems bad
>
> It's still not possible to login when tfa is enabled though, but at
> least for simple setups it should be a bit better
>
> Signed-off-by: Dominik Csapak
> ---
> src/PVE/AccessControl.pm | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm
> index c32dcc3..52ad84b 100644
> --- a/src/PVE/AccessControl.pm
> +++ b/src/PVE/AccessControl.pm
> @@ -790,6 +790,12 @@ sub authenticate_2nd_old : prototype($$$) {
> sub authenticate_2nd_new : prototype() {
> my ($username, $realm, $otp, $tfa_challenge) = @_;
>
> +my ($tfa_cfg) = user_get_tfa($username, $realm, 1);
> +
> +if (!defined($tfa_cfg)) {
> + return undef;
> +}
> +
> my $result = lock_tfa_config(sub {
> my ($tfa_cfg, $realm_tfa) = user_get_tfa($username, $realm, 1);
>
> --
> 2.30.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC kernel] revert problematic TSC multiplier commit
Am 14.09.22 um 11:38 schrieb Eneko Lacunza: > Hi, > > El 14/9/22 a las 10:40, Eneko Lacunza escribió: >> >> El 14/9/22 a las 9:50, Fiona Ebner escribió: >>> Am 05.09.22 um 10:25 schrieb Eneko Lacunza: I just confirmed that in addition to issue reported in https://bugzilla.proxmox.com/show_bug.cgi?id=4073 (live migrated VM hung using 100% CPU), we also reproduce issue reported in https://forum.proxmox.com/threads/zeitspr%C3%BCnge-in-vms-seit-pve-7-2.112756/ At least what I understand google translating it :) - From 5.15.39-4-pve (Ryzen 2600X) to 5.15.39-4-pve (Ryzen 1700): Problems * Migration of a VM hung 4 other VMs in destination host (no 100% CPU use, consoles show time related issues, watchdog, etc.). We can test the patch if you can provide a kernel .deb package. >>> Hi, >>> a kernel package for 5.19 is available in all repositories now [0]. The >>> fix for the TSC multiplier issue is included there. >>> >>> [0]: https://forum.proxmox.com/threads/115090/ >> >> Thanks, I'll try to test today and will report back. > > I just made a kick test and 5.19.7-1 kernel continues to have issues for > us (tried live migration of 5 VMs from 5.13 to 5.19, 4 VMs froze using > 100% CPU). IIUC, the migrated VMs themselves hung? Is the other issue with causing time jumps and CPU stalls in *other* VMs now gone? I'm not sure if a fix for the FPU-related issue is present in this kernel or still outstanding, likely @Thomas knows? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] ui: eslint: fix undefined check
'typeof' cannot return 'undefined' only the string '"undefined"', newer
eslint versions detect that as error
to fix it, directly check it for undefined instead of using typeof
Signed-off-by: Dominik Csapak
---
www/manager6/storage/PBSEdit.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/www/manager6/storage/PBSEdit.js b/www/manager6/storage/PBSEdit.js
index d880308c..5b6b6bb8 100644
--- a/www/manager6/storage/PBSEdit.js
+++ b/www/manager6/storage/PBSEdit.js
@@ -365,7 +365,7 @@ Ext.define('PVE.panel.PBSEncryptionKeyTab', {
} catch (e) {
return "Failed to parse key - " + e;
}
- if (typeof key.data === undefined) {
+ if (key.data === undefined) {
return "Does not seems like a valid Proxmox
Backup key!";
}
}
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured
Am 14/09/2022 um 15:42 schrieb Dominik Csapak: > The downside is that we cannot authenticate users anymore without quorum > (since locking requires write access to pmxcfs), even for users without > tfa configured (and also for clusters without any tfa configured at all) question is more if we should disallow login on unquorate clusters for all but root@pam, as for all others you cannot be sure if they still got the permissions and for the pve realm the credentials are still correct, or if the non-existing TFA entry is still up-to-date (the quorate partition could have TFA configured for that user since cluster split). root@pam is a hard-coded super admin and verified via PAM, which normally should be pmxcfs, and thus quorum, independent, at least if nobody was crazy enough to link /etc/shadow to a file in pmxcfs or wrote a PAM that works on pmxcfs info in other ways. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC kernel] revert problematic TSC multiplier commit
Am 14/09/2022 um 11:38 schrieb Eneko Lacunza via pve-devel: > I just made a kick test and 5.19.7-1 kernel continues to have issues for us > (tried live migration of 5 VMs from 5.13 to 5.19, 4 VMs froze using 100% CPU). Well, that is somewhat expected.. How 5.19 to 5.19 is working out would be more interesting, if you got a test host/vm combo to try. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v7 1/1] api: update: check 'admin' tags privileges
On 9/14/22 16:15, Aaron Lauterer wrote: Something that crossed my mind: Have you thought about not allowing tags if they match an admin tag, except for the '+'? Depending on what they will be used for in the future, there could be some potential to trick an admin by creating a similar regular tag. Any code relying on admin tags should not have an issue with that, but even though the color in the GUI should be different, one could try to trick an admin to do something they should not, depending on the tags. Visual spoofing with similar looking UTF8 characters should not be much of an issue, due to the regex used. i get what you mean, but it's difficult to implement. in the current version, we only ever have the tags currently defined, not the global defined ones. alternatively we could let an admin define a set of admin tags in the cluster, which could then be off-limits for setting/removing for non-admins that would potentially also solve the problem of having a seperate regex for them in the first place as for confusion: admin tags always are prefixed with a '+' symbol currently so, imho '+backup' and 'backup' are different enough? On 6/21/22 11:19, Dominik Csapak wrote: normal tags require 'VM.Config.Options' on the VM, admin tags require 'Sys.Modify' on '/' a user can set/delete/reorder tags, as long as no admin tags get added/removed Signed-off-by: Dominik Csapak [...] ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v3 container] fix #4192: revamp check for systemd version
Instead of iterating through several folders, it might just be easier to
check the objdump output of /sbin/init and getting the version from there.
Resolving the /sbin/init symlink happens inside the chroot, but the
objdump from the host system is used, as to not run any untrusted
executables.
Signed-off-by: Leo Nunner
---
I think putting the subroutine to resolve the /sbin/init symlink into
Setup.pm makes the most sense, since this isn't realy a task for the
plugin.
src/PVE/LXC/Setup.pm | 18 +-
src/PVE/LXC/Setup/Alpine.pm| 2 +-
src/PVE/LXC/Setup/Base.pm | 34 ++
src/PVE/LXC/Setup/Devuan.pm| 2 +-
src/PVE/LXC/Setup/Plugin.pm| 2 +-
src/PVE/LXC/Setup/Unmanaged.pm | 2 +-
6 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
index b72a18e..fe6f0db 100644
--- a/src/PVE/LXC/Setup.pm
+++ b/src/PVE/LXC/Setup.pm
@@ -285,7 +285,7 @@ sub post_create_hook {
sub unified_cgroupv2_support {
my ($self) = @_;
-return $self->protected_call(sub {
$self->{plugin}->unified_cgroupv2_support() });
+return
$self->{plugin}->unified_cgroupv2_support($self->get_ct_init_path());
}
# os-release(5):
@@ -335,4 +335,20 @@ sub get_ct_os_release {
return &$parse_os_release($data);
}
+# Checks whether /sbin/init is a symlink, and if it is,
+# resolves it to the actual binary
+sub get_ct_init_path {
+my ($self) = @_;
+
+my $init = $self->protected_call(sub {
+ my $init_path = "/sbin/init";
+ if($self->{plugin}->ct_is_symlink($init_path)) {
+ $init_path = $self->{plugin}->ct_readlink($init_path);
+ }
+ return $init_path;
+});
+
+return $init;
+}
+
1;
diff --git a/src/PVE/LXC/Setup/Alpine.pm b/src/PVE/LXC/Setup/Alpine.pm
index b56d895..87d72be 100644
--- a/src/PVE/LXC/Setup/Alpine.pm
+++ b/src/PVE/LXC/Setup/Alpine.pm
@@ -102,7 +102,7 @@ sub setup_network {
# non systemd based containers work with pure cgroupv2
sub unified_cgroupv2_support {
-my ($self) = @_;
+my ($self, $init) = @_;
return 1;
}
diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
index cc12914..09155cf 100644
--- a/src/PVE/LXC/Setup/Base.pm
+++ b/src/PVE/LXC/Setup/Base.pm
@@ -514,40 +514,42 @@ sub clear_machine_id {
}
}
-# tries to guess the systemd (major) version based on the existence of
-# (/usr)?/lib/systemd/libsystemd-shared.so. It was introduced in v231.
+# tries to guess the systemd (major) version based on the
+# libsystemd-shared.so linked with /sbin/init
sub get_systemd_version {
-my ($self) = @_;
+my ($self, $init) = @_;
-my $sd_lib_dir = $self->ct_is_directory("/lib/systemd") ?
- "/lib/systemd" : "/usr/lib/systemd";
-my $libsd = PVE::Tools::dir_glob_regex($sd_lib_dir,
"libsystemd-shared-.+\.so");
-if (defined($libsd) && $libsd =~ /libsystemd-shared-(\d+)(?:\..*)?\.so/) {
- return $1;
-}
+my $version = undef;
+PVE::Tools::run_command(
+ ['objdump', '-p', $self->{rootdir}.$init],
+ outfunc => sub {
+ my $line = shift;
+ if ($line =~ /libsystemd-shared-(\d+)(?:\.[a-zA-Z0-9]*)?\.so:$/) {
+ $version = $1;
+ }},
+ errmsg => "objdump on $init failed",
+);
-return undef;
+return $version;
}
sub unified_cgroupv2_support {
-my ($self) = @_;
+my ($self, $init) = @_;
# https://www.freedesktop.org/software/systemd/man/systemd.html
# systemd is installed as symlink to /sbin/init
-my $systemd = $self->ct_readlink('/sbin/init');
-
# assume non-systemd init will run with unified cgroupv2
-if (!defined($systemd) || $systemd !~ m@/systemd$@) {
+if (!defined($init) || $init !~ m@/systemd$@) {
return 1;
}
# systemd version 232 (e.g. debian stretch) supports the unified hierarchy
-my $sdver = $self->get_systemd_version();
+my $sdver = $self->get_systemd_version($init);
if (!defined($sdver) || $sdver < 232) {
return 0;
}
-return 1
+return 1;
}
sub ssh_host_key_types_to_generate {
diff --git a/src/PVE/LXC/Setup/Devuan.pm b/src/PVE/LXC/Setup/Devuan.pm
index 3e15bb2..059f145 100644
--- a/src/PVE/LXC/Setup/Devuan.pm
+++ b/src/PVE/LXC/Setup/Devuan.pm
@@ -42,7 +42,7 @@ sub new {
# non systemd based containers work with pure cgroupv2
sub unified_cgroupv2_support {
-my ($self) = @_;
+my ($self, $init) = @_;
return 1;
}
diff --git a/src/PVE/LXC/Setup/Plugin.pm b/src/PVE/LXC/Setup/Plugin.pm
index 8458ad8..7024856 100644
--- a/src/PVE/LXC/Setup/Plugin.pm
+++ b/src/PVE/LXC/Setup/Plugin.pm
@@ -48,7 +48,7 @@ sub set_user_password {
}
sub unified_cgroupv2_support {
-my ($self) = @_;
+my ($self, $init) = @_;
croak "implement me in sub-class\n";
}
diff --git a/src/PVE/LXC/Setup/Unmanaged.pm b/src/PVE/LXC/Setup/Unmanaged.pm
index 3b9febf..280af04 100644
--- a/src/PVE/LXC/Setup/Unman
Re: [pve-devel] [PATCH manager v7 10/14] ui: tree/ResourceTree: show Tags in tree
On 9/14/22 16:15, Aaron Lauterer wrote:
Why the change from vm.text to vm_text in {lxc,qemu}/Config.js?
AFAICT we have exactly the same string in the now not used "vm.text".
If these changes are needed and should be part of this commmit, a short explanation would be good as
it does not seem to have anything to do with the resource tree.
you're right in that the explanation is missing:
to show the tags in the tree, we have to modify the text in those records,
which are passed
through to the components and end up in 'vm.text', so the tags would end up
there
but since we want the tags to be shown differently there (to be able to
add/edit them)
we cannot use that anymore
On 6/21/22 11:20, Dominik Csapak wrote:
[snip]
node: {
@@ -114,6 +116,8 @@ Ext.define('PVE.tree.ResourceTree', {
}
}
+ info.text += PVE.Utils.renderTags(info.tags, PVE.Utils.tagOverrides);
+
info.text = status + info.text;
},
here we modify the 'text' property
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v7 14/14] ui: form/Tag(Edit): add drag & drop when editing tags
On 9/14/22 16:15, Aaron Lauterer wrote:
On 6/21/22 11:20, Dominik Csapak wrote:
Signed-off-by: Dominik Csapak
---
www/manager6/form/Tag.js | 22 +++--
www/manager6/form/TagEdit.js | 96 +++-
2 files changed, 114 insertions(+), 4 deletions(-)
diff --git a/www/manager6/form/Tag.js b/www/manager6/form/Tag.js
index 91190051..dcbd9597 100644
--- a/www/manager6/form/Tag.js
+++ b/www/manager6/form/Tag.js
@@ -31,6 +31,9 @@ Ext.define('Proxmox.Tag', {
if (event.target.tagName !== 'I') {
return;
}
+ if (event.target.classList.contains('handle')) {
+ return;
+ }
switch (me.mode) {
case 'editable':
if (me.addTag) {
@@ -156,12 +159,14 @@ Ext.define('Proxmox.Tag', {
let text = me.tag;
let cursor = 'pointer';
let padding = '0px';
+ let dragHandleStyle = 'none';
switch (mode) {
case 'normal':
iconStyle += 'display: none;';
padding = undefined;
break;
case 'editable':
+ dragHandleStyle = '';
Is there a reason for the '' here compared to the 'none' above and below?
yes, to show it, we set
style.display = dragHandleStyle
which if '', shows it ('none' hides it)
break;
case 'edit':
me.tagEl().contentEditable = true;
@@ -174,12 +179,14 @@ Ext.define('Proxmox.Tag', {
if (me.addTag) {
me.setText(text);
me.setStyle('cursor', cursor);
+ dragHandleStyle = 'none';
}
[...]
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured
On 9/15/22 12:43, Thomas Lamprecht wrote: Am 14/09/2022 um 15:42 schrieb Dominik Csapak: The downside is that we cannot authenticate users anymore without quorum (since locking requires write access to pmxcfs), even for users without tfa configured (and also for clusters without any tfa configured at all) question is more if we should disallow login on unquorate clusters for all but root@pam, as for all others you cannot be sure if they still got the permissions and for the pve realm the credentials are still correct, or if the non-existing TFA entry is still up-to-date (the quorate partition could have TFA configured for that user since cluster split). fair point, but then we'd also have to deny api requests in general for non root@pam users when the cluster is not quorate, otherwise they can continue making requests with existing tickets (aside creating a new one) root@pam is a hard-coded super admin and verified via PAM, which normally should be pmxcfs, and thus quorum, independent, at least if nobody was crazy enough to link /etc/shadow to a file in pmxcfs or wrote a PAM that works on pmxcfs info in other ways. AFAICS a user can't do anything on a non quorate cluster that wouldn't be possible over ssh for example ? since most operations already need the cluster to be quorate (aside from things such as network configuration, which might be the exact reason why the cluster is not quorate, so the admin/user wants to fix it) so imho @pam could be allowed in general, but for the other realms it's very much a design decision. just to note: before accidentally breaking such logins by locking the tfa config, we always let users login, regardless of cluster quorum state ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured
Am 15/09/2022 um 14:40 schrieb Dominik Csapak: > On 9/15/22 12:43, Thomas Lamprecht wrote: >> Am 14/09/2022 um 15:42 schrieb Dominik Csapak: >>> The downside is that we cannot authenticate users anymore without quorum >>> (since locking requires write access to pmxcfs), even for users without >>> tfa configured (and also for clusters without any tfa configured at all) >> >> question is more if we should disallow login on unquorate clusters for all >> but root@pam, as for all others you cannot be sure if they still got the >> permissions and for the pve realm the credentials are still correct, or >> if the non-existing TFA entry is still up-to-date (the quorate partition >> could have TFA configured for that user since cluster split). > > fair point, but then we'd also have to deny api requests in general > for non root@pam users when the cluster is not quorate, > otherwise they can continue making requests with existing tickets > (aside creating a new one) That is something rather different and bound to the ticket life time, they won't be able to renew the ticket or create new ones, so while the session continues to be available for maximal 2h for GET calls it has already the same limitations as new logins. IOW. if you want to see that as the same thing you need to adapt tickets to have a cluster state first that can actively be revoked on a per-ticket/user basis, as otherwise it'll always be "valid ticket = valid login". >> >> root@pam is a hard-coded super admin and verified via PAM, which normally >> should be pmxcfs, and thus quorum, independent, at least if nobody was crazy >> enough to link /etc/shadow to a file in pmxcfs or wrote a PAM that works >> on pmxcfs info in other ways. > > AFAICS a user can't do anything on a non quorate cluster that wouldn't be > possible > over ssh for example ? since most operations already need the cluster to be > quorate > (aside from things such as network configuration, which might be the exact > reason > why the cluster is not quorate, so the admin/user wants to fix it) > > so imho @pam could be allowed in general, but for the other realms it's > very much a design decision. Standard pam accounts do not get their Proxmox VE access level if connected via SSH (or getting a shell in any other way), as the CLIHandler isn't user account aware, so not sure how that would be an argument for allowing all of pam (which may not even be allowed to SSH in or login (e.g., shell to /bin/false or the like, you just cannot (simply!) derive/know that) > > just to note: before accidentally breaking such logins by locking the tfa > config, we always let users login, regardless of cluster quorum state > yes I know, the question if that's right still stands, more secure is to not allow it, and iff, we could provide a datacenter.cfg option to allow such things with the (natural) warning that changes to that will only affected quorate notes. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] Proxmox Offline Mirror released!
Hi all, We are proud to announce the first release of our new Proxmox Offline Mirror tool. With the Proxmox Offline Mirror tool, you can manage a local apt mirror for all package updates for Proxmox and Debian projects. From this local apt mirror you can create an external medium, for example a USB flash drive or a local network share, to update systems which cannot access the package repositories directly (or proxied) via the internet. Such systems might be restricted by policies to access the public internet or are completely air-gapped. Finally, you can also manage subscriptions for such restricted hosts. For more details see the documentation and the forum announcement: https://pom.proxmox.com/ https://forum.proxmox.com/threads/proxmox-offline-mirror-released.115219/ -- best regards, Thomas Lamprecht ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-apt/proxmox-offline-mirror 0/7] misc improvements
this series adds some features and fixes some issues that pop up when attempting to mirror Ubuntu repositories, and should also improve resilience with other third-party repositories. tested with Ubuntu Jammy (main, security and updates repositories). ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-offline-mirror 2/4] mirror: skip failed, non Packages references
these contain extra data that is not that important for the main
repository use case - providing deb packages.
if they are not retrievable (e.g., Ubuntu *only* provides some of they
via by-hash, which proxmox-offline-mirror doesn't yet support) a warning
should be enough, instead of failing the whole snapshot creation.
Signed-off-by: Fabian Grünbichler
---
src/mirror.rs | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/mirror.rs b/src/mirror.rs
index 14b0c6a..f910e6a 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -534,6 +534,7 @@ pub fn create_snapshot(
let mut packages_size = 0_usize;
let mut packages_indices = HashMap::new();
+let mut failed_references = Vec::new();
for (component, references) in per_component {
println!("\nFetching indices for component '{component}'");
let mut component_deb_size = 0;
@@ -555,7 +556,18 @@ pub fn create_snapshot(
}
// this will ensure the uncompressed file will be written
locally
-let res = fetch_index_file(&config, prefix, reference,
uncompressed_ref)?;
+let res = match fetch_index_file(&config, prefix, reference,
uncompressed_ref) {
+Ok(res) => res,
+Err(err) if !reference.file_type.is_package_index() => {
+eprintln!(
+"Failed to fetch '{:?}' type reference '{}',
skipping - {err}",
+reference.file_type, reference.path
+);
+failed_references.push(reference);
+continue;
+}
+Err(err) => bail!(err),
+};
fetch_progress.update(&res);
if package_index_data.is_none() &&
reference.file_type.is_package_index() {
@@ -577,6 +589,12 @@ pub fn create_snapshot(
total_progress += fetch_progress;
}
println!("Total deb size: {packages_size}");
+if !failed_references.is_empty() {
+eprintln!("Failed to download non-package-index references:");
+for reference in failed_references {
+eprintln!("\t{}", reference.path);
+}
+}
println!("\nFetching packages..");
for (basename, references) in packages_indices {
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-apt 2/3] release: add 'architecture' helper
which returns if a file reference is architecture specific, and for
which architecture it is relevant.
Signed-off-by: Fabian Grünbichler
---
src/deb822/release_file.rs | 16
1 file changed, 16 insertions(+)
diff --git a/src/deb822/release_file.rs b/src/deb822/release_file.rs
index 2b7245b..5888728 100644
--- a/src/deb822/release_file.rs
+++ b/src/deb822/release_file.rs
@@ -228,6 +228,22 @@ impl FileReferenceType {
}
}
+pub fn architecture(&self) -> Option<&Architecture> {
+match self {
+FileReferenceType::Commands(arch, _)
+| FileReferenceType::Contents(arch, _)
+| FileReferenceType::ContentsUdeb(arch, _)
+| FileReferenceType::Packages(arch, _) => Some(arch),
+FileReferenceType::PseudoRelease(arch) => arch.as_ref(),
+FileReferenceType::Unknown
+| FileReferenceType::PDiff
+| FileReferenceType::Sources(_)
+| FileReferenceType::Dep11(_)
+| FileReferenceType::Translation(_)
+| FileReferenceType::Ignored => None,
+}
+}
+
pub fn is_package_index(&self) -> bool {
matches!(self, FileReferenceType::Packages(_, _))
}
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-apt 3/3] release: fix typo in 'Acquire-By-Hash'
to allow detection of repositories that support downloading indices via
their hash instead of their filename.
Signed-off-by: Fabian Grünbichler
---
src/deb822/release_file.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/deb822/release_file.rs b/src/deb822/release_file.rs
index 5888728..355b246 100644
--- a/src/deb822/release_file.rs
+++ b/src/deb822/release_file.rs
@@ -342,7 +342,7 @@ impl TryFrom for ReleaseFile {
parsed.suite = value.suite;
parsed.version = value.version;
-parsed.aquire_by_hash = match value.extra_fields.get("Aquire-By-Hash")
{
+parsed.aquire_by_hash = match
value.extra_fields.get("Acquire-By-Hash") {
Some(val) => *val == "yes",
None => false,
};
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-offline-mirror 1/4] mirror: use xz multi decoder
Ubuntu's Packages.xz files require it, because they contain multiple
streams.
Signed-off-by: Fabian Grünbichler
---
src/mirror.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/mirror.rs b/src/mirror.rs
index 78a493b..14b0c6a 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -277,7 +277,7 @@ fn fetch_index_file(
&buf[..]
}
Some(CompressionType::Lzma) | Some(CompressionType::Xz) => {
-let mut xz = xz2::read::XzDecoder::new(raw);
+let mut xz = xz2::read::XzDecoder::new_multi_decoder(raw);
xz.read_to_end(&mut buf)?;
&buf[..]
}
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-apt 1/3] release: add Commands file reference type
used by command-not-found to lookup which package ships which command.
Signed-off-by: Fabian Grünbichler
---
Notes:
this is technically a breaking change, but the only user of this already
has a fallback match arm. I wonder whether we should mark this as
non-exhaustive?
src/deb822/release_file.rs | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/deb822/release_file.rs b/src/deb822/release_file.rs
index 6668450..2b7245b 100644
--- a/src/deb822/release_file.rs
+++ b/src/deb822/release_file.rs
@@ -51,6 +51,8 @@ pub type Component = String;
/// `Packages` and `Sources` will contain further reference to binary or
source package files.
/// These are handled in `PackagesFile` and `SourcesFile` respectively.
pub enum FileReferenceType {
+/// A `Commands` index listing command to package mappings
+Commands(Architecture, Option),
/// A `Contents` index listing contents of binary packages
Contents(Architecture, Option),
/// A `Contents` index listing contents of binary udeb packages
@@ -123,6 +125,20 @@ impl FileReferenceType {
Ok(FileReferenceType::Unknown)
}
}
+"cnf" => {
+if let Some(rest) = rest.strip_prefix("Commands-") {
+if let Some((arch, ext)) = rest.rsplit_once('.') {
+Ok(FileReferenceType::Commands(
+arch.to_owned(),
+
FileReferenceType::match_compression(ext).ok().flatten(),
+))
+} else {
+Ok(FileReferenceType::Commands(rest.to_owned(),
None))
+}
+} else {
+Ok(FileReferenceType::Unknown)
+}
+},
"dep11" => {
if let Some((_path, ext)) = rest.rsplit_once('.') {
Ok(FileReferenceType::Dep11(
@@ -198,7 +214,8 @@ impl FileReferenceType {
pub fn compression(&self) -> Option {
match *self {
-FileReferenceType::Contents(_, comp)
+FileReferenceType::Commands(_, comp)
+| FileReferenceType::Contents(_, comp)
| FileReferenceType::ContentsUdeb(_, comp)
| FileReferenceType::Packages(_, comp)
| FileReferenceType::Sources(comp)
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-offline-mirror 3/4] mirror: support acquiring indices by hash
requires proxmox-apt > 0.9.1, since earlier versions misdetect by-hash
support in the release file.
Signed-off-by: Fabian Grünbichler
---
src/mirror.rs | 47 ++-
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/src/mirror.rs b/src/mirror.rs
index f910e6a..6cbd680 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -233,6 +233,7 @@ fn fetch_index_file(
prefix: &Path,
reference: &FileReference,
uncompressed: &FileReference,
+by_hash: bool,
) -> Result {
let url = get_dist_url(&config.repository, &reference.path);
let path = get_dist_path(&config.repository, prefix, &reference.path);
@@ -252,14 +253,36 @@ fn fetch_index_file(
return Ok(FetchResult { data, fetched: 0 });
}
-let res = fetch_plain_file(
-config,
-&url,
-&path,
-reference.size,
-&reference.checksums,
-true,
-)?;
+let urls = if by_hash {
+let mut urls = Vec::new();
+if let Some((base_url, _file_name)) = url.rsplit_once('/') {
+if let Some(sha512) = reference.checksums.sha512 {
+urls.push(format!("{base_url}/by-hash/SHA512/{}",
hex::encode(sha512)));
+}
+if let Some(sha256) = reference.checksums.sha256 {
+urls.push(format!("{base_url}/by-hash/SHA256/{}",
hex::encode(sha256)));
+}
+}
+urls.push(url);
+urls
+} else {
+vec![url]
+};
+
+let res = urls
+.iter()
+.fold(None, |res, url| match res {
+Some(Ok(res)) => Some(Ok(res)),
+_ => Some(fetch_plain_file(
+config,
+&url,
+&path,
+reference.size,
+&reference.checksums,
+true,
+)),
+})
+.ok_or_else(|| format_err!("Failed to retrieve {}", reference.path))??;
let mut buf = Vec::new();
let raw = res.data_ref();
@@ -556,7 +579,13 @@ pub fn create_snapshot(
}
// this will ensure the uncompressed file will be written
locally
-let res = match fetch_index_file(&config, prefix, reference,
uncompressed_ref) {
+let res = match fetch_index_file(
+&config,
+prefix,
+reference,
+uncompressed_ref,
+release.aquire_by_hash,
+) {
Ok(res) => res,
Err(err) if !reference.file_type.is_package_index() => {
eprintln!(
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-offline-mirror 4/4] mirror: use new architecture helper
in order to avoid having a list of arch-specific references on two
places.
Signed-off-by: Fabian Grünbichler
---
requires proxmox-apt > 0.9.1 with the new helper
src/mirror.rs | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/mirror.rs b/src/mirror.rs
index 6cbd680..3370ca4 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -501,14 +501,14 @@ pub fn create_snapshot(
|| match &reference.file_type {
FileReferenceType::Ignored => true,
FileReferenceType::PDiff => true, // would require fetching
the patches as well
-FileReferenceType::Contents(arch, _)
-| FileReferenceType::ContentsUdeb(arch, _)
-| FileReferenceType::Packages(arch, _)
-| FileReferenceType::PseudoRelease(Some(arch)) => {
-!binary || !config.architectures.contains(arch)
-}
FileReferenceType::Sources(_) => !source,
-_ => false,
+_ => {
+if let Some(arch) = reference.file_type.architecture() {
+!binary || !config.architectures.contains(arch)
+} else {
+false
+}
+}
};
if skip {
println!("Skipping {}", reference.path);
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC PATCH access-control] loosen locking restriction for users without tfa configured
On 9/15/22 14:53, Thomas Lamprecht wrote: Am 15/09/2022 um 14:40 schrieb Dominik Csapak: On 9/15/22 12:43, Thomas Lamprecht wrote: Am 14/09/2022 um 15:42 schrieb Dominik Csapak: The downside is that we cannot authenticate users anymore without quorum (since locking requires write access to pmxcfs), even for users without tfa configured (and also for clusters without any tfa configured at all) question is more if we should disallow login on unquorate clusters for all but root@pam, as for all others you cannot be sure if they still got the permissions and for the pve realm the credentials are still correct, or if the non-existing TFA entry is still up-to-date (the quorate partition could have TFA configured for that user since cluster split). fair point, but then we'd also have to deny api requests in general for non root@pam users when the cluster is not quorate, otherwise they can continue making requests with existing tickets (aside creating a new one) That is something rather different and bound to the ticket life time, they won't be able to renew the ticket or create new ones, so while the session continues to be available for maximal 2h for GET calls it has already the same limitations as new logins. IOW. if you want to see that as the same thing you need to adapt tickets to have a cluster state first that can actively be revoked on a per-ticket/user basis, as otherwise it'll always be "valid ticket = valid login". we already do not allow requests for a user that gets disabled, so when a node loses connection to a cluster, the ticket is still valid there, while not valid on the remaining quorate nodes. i think this here is a similar scenario, since at the moment a node loses quorum, we cannot verify any property of the user anymore (since it could have changed) including the 'enabled/disabled' property, thus we'd have to prevent requests root@pam is a hard-coded super admin and verified via PAM, which normally should be pmxcfs, and thus quorum, independent, at least if nobody was crazy enough to link /etc/shadow to a file in pmxcfs or wrote a PAM that works on pmxcfs info in other ways. AFAICS a user can't do anything on a non quorate cluster that wouldn't be possible over ssh for example ? since most operations already need the cluster to be quorate (aside from things such as network configuration, which might be the exact reason why the cluster is not quorate, so the admin/user wants to fix it) so imho @pam could be allowed in general, but for the other realms it's very much a design decision. Standard pam accounts do not get their Proxmox VE access level if connected via SSH (or getting a shell in any other way), as the CLIHandler isn't user account aware, so not sure how that would be an argument for allowing all of pam (which may not even be allowed to SSH in or login (e.g., shell to /bin/false or the like, you just cannot (simply!) derive/know that ok, thats true ofc just to note: before accidentally breaking such logins by locking the tfa config, we always let users login, regardless of cluster quorum state yes I know, the question if that's right still stands, more secure is to not allow it, and iff, we could provide a datacenter.cfg option to allow such things with the (natural) warning that changes to that will only affected quorate notes. mhmm while that could work, i think it does not serve the right purpose, since i think the times you want to (rightfully) login to non-quorate nodes is when you try to repair them and there are 3 scenarios here: * have it default on (not as secure) * have it default off, but the user have it enabled all the time (also not as secure) * have it default off, users leave it disabled -> they cannot enable it when they'd need to (when they want to login to a non-quorate node) maybe we'd want to make it an option per user? that way there can be one (or more) admin user(s) that can always login (we could make that default true for root@pam). all that said, it still does not solve the problem when that user has tfa enabled, so maybe we should also consider building a check which type of tfa the user has (configured/provided) and only lock it when it's necessary (so far only recovery keys fall under that) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH FIXUP proxmox-offline-mirror] clippy fix
Signed-off-by: Fabian Grünbichler --- not really important, but can be folded into the patch since it's not yet applied ;) src/mirror.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mirror.rs b/src/mirror.rs index f5ee48e..a1fc1a0 100644 --- a/src/mirror.rs +++ b/src/mirror.rs @@ -280,7 +280,7 @@ fn fetch_index_file( Some(Ok(res)) => Some(Ok(res)), _ => Some(fetch_plain_file( config, -&url, +url, &path, reference.size, &reference.checksums, -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH follow-up proxmox-offline-mirror 8/7] mirror: handle indices which are only available compressed
there are repositories out there that not only skip serving the
uncompressed version, but not even reference it in their Release
file(s).
Signed-off-by: Fabian Grünbichler
---
best viewed with -w, the signal-desktop repository is one such example.
src/mirror.rs | 56 ---
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/src/mirror.rs b/src/mirror.rs
index 3370ca4..f5ee48e 100644
--- a/src/mirror.rs
+++ b/src/mirror.rs
@@ -232,25 +232,30 @@ fn fetch_index_file(
config: &ParsedMirrorConfig,
prefix: &Path,
reference: &FileReference,
-uncompressed: &FileReference,
+uncompressed: Option<&FileReference>,
by_hash: bool,
) -> Result {
let url = get_dist_url(&config.repository, &reference.path);
let path = get_dist_path(&config.repository, prefix, &reference.path);
-let uncompressed_path = get_dist_path(&config.repository, prefix,
&uncompressed.path);
-
-if config.pool.contains(&reference.checksums) &&
config.pool.contains(&uncompressed.checksums) {
-let data = config
-.pool
-.get_contents(&uncompressed.checksums, config.verify)?;
-
-// Ensure they're linked at current path
-config.pool.lock()?.link_file(&reference.checksums, &path)?;
-config
-.pool
-.lock()?
-.link_file(&uncompressed.checksums, &uncompressed_path)?;
-return Ok(FetchResult { data, fetched: 0 });
+
+if let Some(uncompressed) = uncompressed {
+let uncompressed_path = get_dist_path(&config.repository, prefix,
&uncompressed.path);
+
+if config.pool.contains(&reference.checksums)
+&& config.pool.contains(&uncompressed.checksums)
+{
+let data = config
+.pool
+.get_contents(&uncompressed.checksums, config.verify)?;
+
+// Ensure they're linked at current path
+config.pool.lock()?.link_file(&reference.checksums, &path)?;
+config
+.pool
+.lock()?
+.link_file(&uncompressed.checksums, &uncompressed_path)?;
+return Ok(FetchResult { data, fetched: 0 });
+}
}
let urls = if by_hash {
@@ -307,12 +312,15 @@ fn fetch_index_file(
};
let locked = &config.pool.lock()?;
-if !locked.contains(&uncompressed.checksums) {
-locked.add_file(decompressed, &uncompressed.checksums, config.sync)?;
-}
+if let Some(uncompressed) = uncompressed {
+if !locked.contains(&uncompressed.checksums) {
+locked.add_file(decompressed, &uncompressed.checksums,
config.sync)?;
+}
-// Ensure it's linked at current path
-locked.link_file(&uncompressed.checksums, &uncompressed_path)?;
+// Ensure it's linked at current path
+let uncompressed_path = get_dist_path(&config.repository, prefix,
&uncompressed.path);
+locked.link_file(&uncompressed.checksums, &uncompressed_path)?;
+}
Ok(FetchResult {
data: decompressed.to_owned(),
@@ -566,15 +574,13 @@ pub fn create_snapshot(
for basename in references {
println!("\tFetching '{basename}'..");
let files = release.files.get(basename).unwrap();
-let uncompressed_ref = files
-.iter()
-.find(|reference| reference.path == *basename)
-.ok_or_else(|| format_err!("Found derived reference without
base reference."))?;
+let uncompressed_ref = files.iter().find(|reference|
reference.path == *basename);
+
let mut package_index_data = None;
for reference in files {
// if both compressed and uncompressed are referenced, the
uncompressed file may not exist on the server
-if reference == uncompressed_ref && files.len() > 1 {
+if Some(reference) == uncompressed_ref && files.len() > 1 {
continue;
}
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
