From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id AC4887319F for ; Thu, 27 May 2021 00:28:20 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 92A19181A5 for ; Thu, 27 May 2021 00:28:20 +0200 (CEST) Received: from smtp.smtpout.orange.fr (smtp02.smtpout.orange.fr [80.12.242.124]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 42C701818D for ; Thu, 27 May 2021 00:28:18 +0200 (CEST) Received: from dovecot.localdomain ([90.118.15.232]) by mwinf5d37 with ME id 9aUB2500250Qfqq03aUBPV; Thu, 27 May 2021 00:28:11 +0200 X-ME-Helo: dovecot.localdomain X-ME-Auth: anVsaWVuLmJsYWlzNUBvcmFuZ2UuZnI= X-ME-Date: Thu, 27 May 2021 00:28:11 +0200 X-ME-IP: 90.118.15.232 Message-ID: <6a8cb87017fd0e74c5deaf7eb1be57be@mwinf5d37.me-wanadoo.net> MIME-Version: 1.0 To: Thomas Lamprecht , Proxmox VE development discussion From: wb Date: Thu, 27 May 2021 00:28:11 +0200 Importance: normal X-Priority: 3 In-Reply-To: References: <02c8e0ce3079939415c742edf16c0966@mwinf5d78.me-wanadoo.net> X-SPAM-LEVEL: Spam detection results: 1 AWL -0.180 Adjusted score from AWL reputation of From: address HTML_MESSAGE 0.001 HTML included in message KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods KAM_NUMSUBJECT 0.5 Subject ends in numbers excluding current years POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_NONE -0.0001 Sender listed at https://www.dnswl.org/, no trust RCVD_IN_MSPIKE_H2 -0.001 Average reputation (+2) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: [pve-devel] =?utf-8?b?UkXCoDogUkXCoDogIFJFwqA6IHB2ZS1kZXZlbCBE?= =?utf-8?q?igest=2C_Vol_132=2C_Issue_53?= X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 May 2021 22:28:20 -0000 Hello Thomas, Thanks for all these clarifications, I was able to move forward and put a V= 0. https://github.com/jbsky/proxmox-saml2-auth/commit/75c9f089170aacf02370ab86= bfa8c75fe0c76a6a.diff Instead of creating a POST /access/saml endpoint, I finally reused the POST= /access/ticket (thanks for your comment ;) The important points remaining: =E2=9E=A2 For the login page, we need to create links to each SAML "realm" = create, we should have links, there, there is nothing. =E2=9E=A2 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 parame= ter. Is it possible to pass a cookie from the client to an endpoint?=20 That would be much cleaner, we would be at the target of what is recommende= d. =E2=9E=A2 I created an attribute realm_proxmox on the IdP side to provide t= he 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 earlie= st. Sincelery, Julien BLAIS De=C2=A0: Thomas Lamprecht Envoy=C3=A9 le=C2=A0:mercredi 26 mai 2021 08:22 =C3=80=C2=A0: wb; Proxmox VE development discussion Objet=C2=A0:Re: RE=C2=A0: [pve-devel] RE=C2=A0: 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 >=20 >> running does not mean working... > Ok, but starting from an installation, I doubt :). >=20 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=20 >=20 >=20 >=20 > touch /etc/pve/foo > findmnt /etc/pve >=20 Look OK, but please paste such things inline the next time, I and most othe= rs here on this list find pictures of plain text rather needlessly hard to deal wit= h. >=20 > I finally found the problem that comes from the read permission on the /e= tc/pve/priv/authkey.key file, probably because www-data is reading instead = of root. >=20 I mean yes, it still is a filesystem so read/write need to adhere to the st= andard 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:>=20 > /etc/pve/priv/ > /etc/pve/nodes/${NAME}/priv/ >=20 > 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 =3D> 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 tick= et and thus, to do SSO. > An analysis of the problem in my code would be welcome. >=20 > There will still be 2 points to be clean (not counting the anomaly due to= the right in reading)=C2=A0: > =E2=9E=A2 First point, create links in the login page instead of putting = saml authentication realm in the list. > =E2=9E=A2 Check the saml request id properly in the authentication part o= f the SAML class, (this is really not the case) >=20 > Thank you in advance, >=20 > Attached is the patch. This is a combined patch over multiple repositories, further having it as a= ttachment 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 violation= s 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 thos= e standards (yet), that's still no real excuse for new code to adapt to it, as much a= s 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_s= aml` 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 a= re done that way - making review much easier - thanks! cheers, Thomas