[pve-devel] RE : pve-devel Digest, Vol 132, Issue 53

[pve-devel] RE : pve-devel Digest, Vol 132, Issue 53

[wb]

Hi Dietmar,

Thank you for your feedback.

However, since I am starting on a new installation, I am surprised to get this kind of answer.
« Your cluster fs is not working (pmxcfs). See you run on a broken installation. »
Or 
« You need a working PVE installation before doing any API calls... »

With the following command, I have the process up!

ps aux | grep pmxcfs


I think I have enough knowledge about SAML and Perl to do it, however, the support of a dev would be ideal at least on the lock part.

I'm trying to implement a new api so that Proxmox authentication works with SAMLv2.

I would have preferred to have more info on the following part :
# this is just a readonly copy, the relevant one is in status.c from pmxcfs
# observed files are the one we can get directly through IPCC, they are cached
# using a computed version and only those can be used by the cfs_*_file methods

To try to bring a little more element, I added a file to the following list in the PVE::Cluster file
my $observed = {
    'request.tmp' => 1,

Still in the PVE::Cluster file, It is well in the following part that it blocks :


If I take the error message from the first email,
«  error during cfs-locked \'file-request_tmp\' operation: pve cluster filesystem not online /etc/pve/priv/lock. »
If I test the dir /etc/pve/priv/lock, it exists!

Do the files we add in PVE::Cluster file need to be listed in /var/lib/pve-cluster/config.db, if so, any spec please?

Thanking you in advance, 

Sincerely,

Julien BLAIS


De : pve-devel-request@lists.proxmox.com
Envoyé le :lundi 24 mai 2021 12:00
À : pve-devel@lists.proxmox.com
Objet :pve-devel Digest, Vol 132, Issue 53

Send pve-devel mailing list submissions to
	pve-devel@lists.proxmox.com

To subscribe or unsubscribe via the World Wide Web, visit
	https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
or, via email, send a message with subject or body 'help' to
	pve-devel-request@lists.proxmox.com

You can reach the person managing the list at
	pve-devel-owner@lists.proxmox.com

When replying, please edit your Subject line so it is more specific
than "Re: Contents of pve-devel digest..."


Today's Topics:

   1. cfs-locked 'authkey' operation: pve cluster filesystem not
      online (wb)
   2. Re: cfs-locked 'authkey' operation: pve cluster filesystem
      not online (Dietmar Maurer)


----------------------------------------------------------------------

Message: 1
Date: Sun, 23 May 2021 23:23:23 +0200
From: wb <webmaster@jbsky.fr>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: [pve-devel] cfs-locked 'authkey' operation: pve cluster
	filesystem not online
Message-ID:
	<fb0ddc9e61de4c98f1498ff4375b9689@mwinf5d62.me-wanadoo.net>
Content-Type: text/plain; charset="utf-8"

Hello to all.

I have the plan to implement the SSO authentication feature with the SAML protocol.
However, I have an error that prevents me from validating the authentication process.
It is about the locks.
The first step is to store the request_saml_id. If I try to create a file by your libraries, I get an 500 error with msg:
error during cfs-locked \'file-request_tmp\' operation: pve cluster filesystem not online /etc/pve/priv/lock.
https://github.com/jbsky/proxmox-saml2-auth/commit/d75dc621aae719c8fdd251859af9641cda0e526b
Ok, I can make a temp workaround.

2nd step?:
When I try to create a ticket with the function create_ticket in package PVE::API2::AccessControl;
I've got this error :
authentication failure; rhost=127.0.0.1 user=admin@DOM msg=error during cfs-locked 'authkey' operation: pve cluster filesystem not online /etc/pve/priv/lock
src : https://github.com/jbsky/proxmox-saml2-auth/commit/93b02727d2e172968c14c4ce3a7c27e8d5c0feb0

I have really bad luck with these locks!
Can you help me to understand the prerequisites to make the lock work?


If you want init a redirect to an identity provider(IdP, ex: Keycloak), use this url :
https://pve/api2/html/access/saml?realm=DOM

After an authentication side IdP, the IdP post to pve at https://pve/api2/html/access/saml.


I'm sorry to work on a separate repository, it's because I don't know your components very well.

I would be grateful if you could tell me how to debug these locks.

Thanking you in advance, 

Sincerely,

Julien BLAIS


------------------------------

Message: 2
Date: Mon, 24 May 2021 09:45:15 +0200 (CEST)
From: Dietmar Maurer <dietmar@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	wb <webmaster@jbsky.fr>
Subject: Re: [pve-devel] cfs-locked 'authkey' operation: pve cluster
	filesystem not online
Message-ID: <606562427.786.1621842315013@webmail.proxmox.com>
Content-Type: text/plain; charset=UTF-8

Hi Julien,


> Hello to all.
> 
> I have the plan to implement the SSO authentication feature with the SAML protocol.
> However, I have an error that prevents me from validating the authentication process.
> It is about the locks.
> The first step is to store the request_saml_id. If I try to create a file by your libraries, I get an 500 error with msg:
> error during cfs-locked \'file-request_tmp\' operation: pve cluster filesystem not online /etc/pve/priv/lock.

Your cluster fs is not working (pmxcfs). See you run on a broken installation.

> https://github.com/jbsky/proxmox-saml2-auth/commit/d75dc621aae719c8fdd251859af9641cda0e526b
> Ok, I can make a temp workaround.
> 
> 2nd step?:
> When I try to create a ticket with the function create_ticket in package PVE::API2::AccessControl;
> I've got this error :
> authentication failure; rhost=127.0.0.1 user=admin@DOM msg=error during cfs-locked 'authkey' operation: pve cluster filesystem not online /etc/pve/priv/lock

Again, the pmxcfs is not online.

> src : https://github.com/jbsky/proxmox-saml2-auth/commit/93b02727d2e172968c14c4ce3a7c27e8d5c0feb0
> 
> I have really bad luck with these locks!
> Can you help me to understand the prerequisites to make the lock work?

You need a working PVE installation before doing any API calls...




------------------------------

Subject: Digest Footer

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


------------------------------

End of pve-devel Digest, Vol 132, Issue 53
******************************************

Re: [pve-devel] RE : pve-devel Digest, Vol 132, Issue 53

[Thomas Lamprecht]

On 24.05.21 23:45, wb wrote:
> However, since I am starting on a new installation, I am surprised to get this kind of answer.
> « Your cluster fs is not working (pmxcfs). See you run on a broken installation. »
> Or 
> « You need a working PVE installation before doing any API calls... »
> 
> With the following command, I have the process up!
> 
> ps aux | grep pmxcfs
> 

running does not mean working...

What's the output/status of:

# systemctl status pve-cluster 
# touch /etc/pve/foo
# findmnt /etc/pve

> 
> I think I have enough knowledge about SAML and Perl to do it, however, the support of a dev would be ideal at least on the lock part.
> 

Nobody questioned that..

> I'm trying to implement a new api so that Proxmox authentication works with SAMLv2.

Yes, as you stated in the initial mail..

> 
> I would have preferred to have more info on the following part :
> # this is just a readonly copy, the relevant one is in status.c from pmxcfs
> # observed files are the one we can get directly through IPCC, they are cached
> # using a computed version and only those can be used by the cfs_*_file methods
> 

I'd suggest ignoring the pmxcfs internal optimized cache-using part, you do not need
that for a start, just use the common file_get_content / file_set_content helper from
the PVE::Tools module, you could do everything with those for now and only then
migrate to a optimized cfs_*_{read,write} helper.

> To try to bring a little more element, I added a file to the following list in the PVE::Cluster file
> my $observed = {
>     'request.tmp' => 1,
> 
> Still in the PVE::Cluster file, It is well in the following part that it blocks :
> 
> 
> If I take the error message from the first email,
> «  error during cfs-locked \'file-request_tmp\' operation: pve cluster filesystem not online /etc/pve/priv/lock. »
> If I test the dir /etc/pve/priv/lock, it exists!

Existence is not a problem, pmxcfs is a clustered realtime configuration filesystem,
it either may not be mounted (and again, running is not always a 100% guarantee that
it is still mounted) or in a cluster (or thinking that's in a cluster due to
`/etc/corosync/corosync.conf` and/or `/etc/pve/corosync.conf` existing) but has no
quorum, i.e., read-only

> 
> Do the files we add in PVE::Cluster file need to be listed in /var/lib/pve-cluster/config.db, if so, any spec please?

no, that's the backing DB, I'd heavily recommend not modifying that one directly if
unsure. Those files get always created on the FUSE VFS layer (besides the very barebone
initial one we create with a small helper).

Note: you need the correct permissions in your service, it must be in www-data group
to be able to read/test directory existance and run as root for writing.

[pve-devel] RE : RE : pve-devel Digest, Vol 132, Issue 53

[wb]

> running does not mean working...
Ok, but starting from an installation, I doubt :).




De : Thomas Lamprecht
Envoyé le :mardi 25 mai 2021 08:11
À : Proxmox VE development discussion; wb
Objet :Re: [pve-devel] RE : pve-devel Digest, Vol 132, Issue 53

On 24.05.21 23:45, wb wrote:
> However, since I am starting on a new installation, I am surprised to get this kind of answer.
> « Your cluster fs is not working (pmxcfs). See you run on a broken installation. »
> Or 
> « You need a working PVE installation before doing any API calls... »
> 
> With the following command, I have the process up!
> 
> ps aux | grep pmxcfs
> 

running does not mean working...

What's the output/status of:

# systemctl status pve-cluster 
# touch /etc/pve/foo
# findmnt /etc/pve

> 
> I think I have enough knowledge about SAML and Perl to do it, however, the support of a dev would be ideal at least on the lock part.
> 

Nobody questioned that..

> I'm trying to implement a new api so that Proxmox authentication works with SAMLv2.

Yes, as you stated in the initial mail..

> 
> I would have preferred to have more info on the following part :
> # this is just a readonly copy, the relevant one is in status.c from pmxcfs
> # observed files are the one we can get directly through IPCC, they are cached
> # using a computed version and only those can be used by the cfs_*_file methods
> 

I'd suggest ignoring the pmxcfs internal optimized cache-using part, you do not need
that for a start, just use the common file_get_content / file_set_content helper from
the PVE::Tools module, you could do everything with those for now and only then
migrate to a optimized cfs_*_{read,write} helper.

> To try to bring a little more element, I added a file to the following list in the PVE::Cluster file
> my $observed = {
>     'request.tmp' => 1,
> 
> Still in the PVE::Cluster file, It is well in the following part that it blocks :
> 
> 
> If I take the error message from the first email,
> «  error during cfs-locked \'file-request_tmp\' operation: pve cluster filesystem not online /etc/pve/priv/lock. »
> If I test the dir /etc/pve/priv/lock, it exists!

Existence is not a problem, pmxcfs is a clustered realtime configuration filesystem,
it either may not be mounted (and again, running is not always a 100% guarantee that
it is still mounted) or in a cluster (or thinking that's in a cluster due to
`/etc/corosync/corosync.conf` and/or `/etc/pve/corosync.conf` existing) but has no
quorum, i.e., read-only

> 
> Do the files we add in PVE::Cluster file need to be listed in /var/lib/pve-cluster/config.db, if so, any spec please?

no, that's the backing DB, I'd heavily recommend not modifying that one directly if
unsure. Those files get always created on the FUSE VFS layer (besides the very barebone
initial one we create with a small helper).

Note: you need the correct permissions in your service, it must be in www-data group
to be able to read/test directory existance and run as root for writing.

Re: [pve-devel] RE : RE : pve-devel Digest, Vol 132, Issue 53

[Thomas Lamprecht]

Hi,

On 25.05.21 22:50, wb wrote:
> Sorry, the first mail went a little too fast
> 
>> running does not mean working...
> Ok, but starting from an installation, I doubt :).
> 

But you already started to patch around, so we just cannot know for sure - was just
mentioning that to cover more possibilities :-)


> systemctl status pve-cluster 
> 
> 
> 
> touch /etc/pve/foo
> findmnt /etc/pve
> 

Look OK, but please paste such things inline the next time, I and most others here
on this list find pictures of plain text rather needlessly hard to deal with.

> 
> I finally found the problem that comes from the read permission on the /etc/pve/priv/authkey.key file, probably because www-data is reading instead of root.
> 

I mean yes, it still is a filesystem so read/write need to adhere to the standard
POSIX file permissions.

Quoting from our documentation:

> All files and directories are owned by user root and have group www-data.
> Only root has write permissions, but group www-data can read most files.
> Files below the following paths:> 
> /etc/pve/priv/
> /etc/pve/nodes/${NAME}/priv/
> 
> are only accessible by root.

-- https://pve.proxmox.com/pve-docs/chapter-pmxcfs.html#_file_access_rights

If a API method needs to access the `priv` path it needs to set the
`protected => 1` "flag" in the API-method schema definition, that way it will proxied
to the local-only API daemon running as root.


> By moving it to /etc/pve/authkey.key, I finally manage to create the ticket and thus, to do SSO.
> An analysis of the problem in my code would be welcome.
> 
> There will still be 2 points to be clean (not counting the anomaly due to the right in reading) :
> ➢ First point, create links in the login page instead of putting saml authentication realm in the list.
> ➢ Check the saml request id properly in the authentication part of the SAML class, (this is really not the case)
> 
> Thank you in advance,
> 
> Attached is the patch.

This is a combined patch over multiple repositories, further having it as attachment
makes replying (reviewing) hard to do on the list.

Please check out the following resources:
* Developer docs, especially the CLA, and preparing/sending patches section:
  https://pve.proxmox.com/wiki/Developer_Documentation

* Our coding style guides, didn't looked to close to actually see violations so rather
  mentioning it for completeness sake:
  - https://pve.proxmox.com/wiki/Perl_Style_Guide
  - https://pve.proxmox.com/wiki/Javascript_Style_Guide

  Note, we're well aware that not all of the existing code holds up to those standards
  (yet), that's still no real excuse for new code to adapt to it, as much as the surrounding
  code style allows.

Just two short comments regarding the patch, as I did not checked that out in full:

1. I'm not sure why you need to duplicate API calls, especially the `post_saml` one.
2. It also seems you missed including the diff for the new `PVE::Auth::SAML` module.

If you want a real review I'd suggest to send out what you have now as real patche
series via `git send-email`, ideally adding some reasoning for why things are done
that way - making review much easier - thanks!

cheers,
Thomas

[pve-devel] RE : RE : RE : pve-devel Digest, Vol 132, Issue 53

[wb]

Hello Thomas,

Thanks for all these clarifications, I was able to move forward and put a V0.

https://github.com/jbsky/proxmox-saml2-auth/commit/75c9f089170aacf02370ab86bfa8c75fe0c76a6a.diff

Instead of creating a POST /access/saml endpoint, I finally reused the POST /access/ticket (thanks for your comment ;)

The important points remaining:
➢ For the login page, we need to create links to each SAML "realm" create, we should have <a href ...> links, there, there is nothing.

➢ Correctly check the saml request id in the authentication part of the SAML class from the user cookie instead of storing it directly on the server (that's what I did).
I had no problem creating a user/client side cookie (e.g. chrome), however, I have trouble retrieving it and providing it to the endpoints as a parameter.
Is it possible to pass a cookie from the client to an endpoint? 
That would be much cleaner, we would be at the target of what is recommended.

➢ I created an attribute realm_proxmox on the IdP side to provide the realm to the endpoint "/access/ticket" , same as the previous point, if the client could provide it with these cookies.

I will prepare the git-mail for you tomorrow or the day after at the earliest.

Sincelery,

Julien BLAIS

De : Thomas Lamprecht
Envoyé le :mercredi 26 mai 2021 08:22
À : wb; Proxmox VE development discussion
Objet :Re: RE : [pve-devel] RE : pve-devel Digest, Vol 132, Issue 53

Hi,

On 25.05.21 22:50, wb wrote:
> Sorry, the first mail went a little too fast
> 
>> running does not mean working...
> Ok, but starting from an installation, I doubt :).
> 

But you already started to patch around, so we just cannot know for sure - was just
mentioning that to cover more possibilities :-)


> systemctl status pve-cluster 
> 
> 
> 
> touch /etc/pve/foo
> findmnt /etc/pve
> 

Look OK, but please paste such things inline the next time, I and most others here
on this list find pictures of plain text rather needlessly hard to deal with.

> 
> I finally found the problem that comes from the read permission on the /etc/pve/priv/authkey.key file, probably because www-data is reading instead of root.
> 

I mean yes, it still is a filesystem so read/write need to adhere to the standard
POSIX file permissions.

Quoting from our documentation:

> All files and directories are owned by user root and have group www-data.
> Only root has write permissions, but group www-data can read most files.
> Files below the following paths:> 
> /etc/pve/priv/
> /etc/pve/nodes/${NAME}/priv/
> 
> are only accessible by root.

-- https://pve.proxmox.com/pve-docs/chapter-pmxcfs.html#_file_access_rights

If a API method needs to access the `priv` path it needs to set the
`protected => 1` "flag" in the API-method schema definition, that way it will proxied
to the local-only API daemon running as root.


> By moving it to /etc/pve/authkey.key, I finally manage to create the ticket and thus, to do SSO.
> An analysis of the problem in my code would be welcome.
> 
> There will still be 2 points to be clean (not counting the anomaly due to the right in reading) :
> ➢ First point, create links in the login page instead of putting saml authentication realm in the list.
> ➢ Check the saml request id properly in the authentication part of the SAML class, (this is really not the case)
> 
> Thank you in advance,
> 
> Attached is the patch.

This is a combined patch over multiple repositories, further having it as attachment
makes replying (reviewing) hard to do on the list.

Please check out the following resources:
* Developer docs, especially the CLA, and preparing/sending patches section:
  https://pve.proxmox.com/wiki/Developer_Documentation

* Our coding style guides, didn't looked to close to actually see violations so rather
  mentioning it for completeness sake:
  - https://pve.proxmox.com/wiki/Perl_Style_Guide
  - https://pve.proxmox.com/wiki/Javascript_Style_Guide

  Note, we're well aware that not all of the existing code holds up to those standards
  (yet), that's still no real excuse for new code to adapt to it, as much as the surrounding
  code style allows.

Just two short comments regarding the patch, as I did not checked that out in full:

1. I'm not sure why you need to duplicate API calls, especially the `post_saml` one.
2. It also seems you missed including the diff for the new `PVE::Auth::SAML` module.

If you want a real review I'd suggest to send out what you have now as real patche
series via `git send-email`, ideally adding some reasoning for why things are done
that way - making review much easier - thanks!

cheers,
Thomas