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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id EB3D890B2C for ; Mon, 12 Feb 2024 14:35:11 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CE76119EE6 for ; Mon, 12 Feb 2024 14:34:41 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 12 Feb 2024 14:34:41 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id CD15E478E2 for ; Mon, 12 Feb 2024 14:34:40 +0100 (CET) Date: Mon, 12 Feb 2024 14:34:21 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20240205175419.1271680-1-m.carrara@proxmox.com> <20240205175419.1271680-12-m.carrara@proxmox.com> In-Reply-To: <20240205175419.1271680-12-m.carrara@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1707744739.1xd0sl17x9.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.086 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH v2 pve-manager 11/11] fix #4759: debian/postinst: configure ceph-crash.service and its key 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: Mon, 12 Feb 2024 13:35:12 -0000 On February 5, 2024 6:54 pm, Max Carrara wrote: > This commit adds the `set_ceph_crash_conf` function, which dynamically > adapts the host's Ceph configuration in order to allow the Ceph crash > module's daemon to run without elevated privileges. >=20 > This adaptation is only performed if: > * Ceph is installed > * Ceph is configured ('/etc/pve/ceph.conf' exists) > * Connection to RADOS is successful >=20 > If the above conditions are met, the function will ensure that: > * Ceph possesses a key named 'client.crash' > * The key is saved to '/etc/pve/ceph/ceph.client.crash.keyring' > * A section for 'client.crash' exists in '/etc/pve/ceph.conf' > * The 'client.crash' section has a key named 'keyring' which > references '/etc/pve/ceph/ceph.client.crash.keyring' >=20 > Furthermore, if a key named 'client.crash' already exists within the > cluster, it shall be reused and not regenerated. Also, the > configuration is not altered if the conditions above are already met. >=20 > This way the keyring file is available as read-only in > '/etc/pve/ceph/' for the `www-data` group (due to how pmxcfs works). > Because the `ceph` user has been made part of said `www-data` group > [0], it may access the file without requiring any additional > privileges. >=20 > Thus, the configuration for the Ceph crash daemon is safely adapted as > expected by PVE tooling and also shared via pmxcfs across one's > cluster. I still don't think this is a good idea, even a simple perl -e '..' invocation or two (or a small helper script that doesn't live in $PATH) for doing the two steps we want (initialize key if missing, lock+modify config if key was missing) would be better (although compared to the "hidden" or regular command approach, it has the downside that somebody might miss the calls here when refactoring), among other things the code below - doesn't lock /etc/pve/ceph.conf but modifies it - implements yet another broken parser for ceph.conf (e.g., it doesn't handle the stuff you fix in the perl variant in this series!) - duplicates constants from the perl code that risk running out of sync, like paths or the key profile - still has issues that you fixed in the perl code between v1 and v2 (restarting services) I haven't reviewed the bash code in detail for that reason! another issue - IMHO this should be version-guarded, since any new setup would already gain it when setting up a monitor, and we avoid access to pmxcfs in the upgrade hot path which can cause problems (cluster non-quorate, ..). >=20 > [0]: https://git.proxmox.com/?p=3Dceph.git;a=3Dcommitdiff;h=3Df72c698a559= 05d93e9a0b7b95674616547deba8a >=20 > Signed-off-by: Max Carrara > --- > Changes v1 --> v2: > * fix 'keyring' key being appended to 'client.crash' section even > if it already exists and configured correctly >=20 > debian/postinst | 113 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 113 insertions(+) >=20 > diff --git a/debian/postinst b/debian/postinst > index 6138ef6d..267a62ae 100755 > --- a/debian/postinst > +++ b/debian/postinst > @@ -110,6 +110,118 @@ migrate_apt_auth_conf() { > fi > } > =20 > +set_ceph_crash_conf() { > + PVE_CEPH_CONFFILE=3D'/etc/pve/ceph.conf' > + PVE_CEPH_CONFDIR=3D'/etc/pve/ceph' > + PVE_CEPH_CRASH_KEY=3D"${PVE_CEPH_CONFDIR}/ceph.client.crash.keyring" > + PVE_CEPH_CRASH_KEY_REF=3D"${PVE_CEPH_CONFDIR}/\$cluster.\$name.keyri= ng" > + > + # ceph isn't installed -> nothing to do > + if ! which ceph > /dev/null 2>&1; then > + return 0 > + fi > + > + # ceph isn't configured -> nothing to do > + if test ! -f "${PVE_CEPH_CONFFILE}"; then > + return 0 > + fi > + > + CEPH_AUTH_RES=3D"$(ceph auth get-or-create client.crash mon 'profile= crash' mgr 'profile crash' 2>&1 || true)" > + > + # ceph is installed and possibly configured, but no connection to RA= DOS > + # -> assume no monitor was created, nothing to do > + if echo "${CEPH_AUTH_RES}" | grep -i -q 'RADOS object not found'; th= en > + return 0 > + fi > + > + SECTION_RE=3D'^\[\S+\]$' > + CRASH_SECTION_RE=3D'^\[client\.crash\]$' > + > + if echo "${CEPH_AUTH_RES}" | grep -q -E "${CRASH_SECTION_RE}"; then > + DO_RESTART_UNIT=3D0 > + CRASH_KEY=3D"$(echo "${CEPH_AUTH_RES}" | grep 'key' | sed -E 's/= ^\s+key\s+=3D\s+//')" > + > + if test ! -d "${PVE_CEPH_CONFDIR}"; then > + mkdir -p "${PVE_CEPH_CONFDIR}" > + fi > + > + # keyring file doesn't exist or contains wrong key > + if test ! -f "${PVE_CEPH_CRASH_KEY}" || ! grep -q "${CRASH_KEY}"= "${PVE_CEPH_CRASH_KEY}"; then > + echo "Saving key for 'client.crash' as '${PVE_CEPH_CRASH_KEY= }'" > + echo "${CEPH_AUTH_RES}" > "${PVE_CEPH_CRASH_KEY}" > + DO_RESTART_UNIT=3D1 > + fi > + > + # 'client.crash' section is in conf file > + if grep -q -E "${CRASH_SECTION_RE}" "${PVE_CEPH_CONFFILE}"; then > + IFS=3D'' > + NEW_PVE_CEPH_CONFFILE=3D'' > + IN_CRASH_SECTION=3D0 > + HAS_KEYRING=3D0 > + REPLACED_KEYRING=3D0 > + > + # look for 'keyring' key in 'client.crash' section > + # -> replace it if it points to the wrong location > + while read -r LINE; do > + if test "${IN_CRASH_SECTION}" =3D "1"; then > + if echo "${LINE}" | grep -q -E "${SECTION_RE}"; then > + IN_CRASH_SECTION=3D0 > + elif echo "${LINE}" | grep -q -E '\s+keyring'; then > + HAS_KEYRING=3D1 > + > + if ! echo "${LINE}" | grep -q "${PVE_CEPH_CRASH_= KEY_REF}"; then > + echo "Replacing keyring value in section 'cl= ient.crash' of '${PVE_CEPH_CONFFILE}'" > + LINE=3D"$(printf '\t keyring =3D %s' "${PVE_= CEPH_CRASH_KEY_REF}")" > + REPLACED_KEYRING=3D1 > + fi > + fi > + elif echo "${LINE}" | grep -q -E "${CRASH_SECTION_RE}"; = then > + IN_CRASH_SECTION=3D1 > + fi > + > + NEW_PVE_CEPH_CONFFILE=3D"${NEW_PVE_CEPH_CONFFILE}${LINE}= \n" > + done < "${PVE_CEPH_CONFFILE}" > + > + unset IFS > + > + if test "${HAS_KEYRING}" =3D "1"; then > + # 'keyring' key was replaced -> write to file > + if test "${REPLACED_KEYRING}" =3D "1"; then > + echo "${NEW_PVE_CEPH_CONFFILE}" > "${PVE_CEPH_CONFFI= LE}" > + DO_RESTART_UNIT=3D1 > + fi > + > + # client.crash section exists, but contained no 'keyring' ke= y > + # -> put 'keyring' key into 'client.crash' section > + else > + sed -i -E "s#(${CRASH_SECTION_RE})#\1\n\t keyring =3D ${= PVE_CEPH_CRASH_KEY_REF}#" \ > + "${PVE_CEPH_CONFFILE}" > + DO_RESTART_UNIT=3D1 > + fi > + > + # 'client.crash' section doesn't exist -> add it > + else > + echo "Adding section for key in '${PVE_CEPH_CONFFILE}'" > + printf '[client.crash]\n\tkeyring =3D %s\n\n' "${PVE_CEPH_CR= ASH_KEY_REF}" \ > + >> "${PVE_CEPH_CONFFILE}" > + DO_RESTART_UNIT=3D1 > + fi > + > + if test "${DO_RESTART_UNIT}" =3D "1"; then > + UNIT=3D'ceph-crash.service' > + > + if systemctl -q is-enabled "${UNIT}"; then > + echo "Restarting ceph-crash.service" > + deb-systemd-invoke restart "${UNIT}" > + fi > + fi > + > + else > + echo "WARNING: Ceph: Unable to retrieve key for 'client.crash' -= output:" > + printf '%s\n\n' "${CEPH_AUTH_RES}" > + fi > +} > + > case "$1" in > triggered) > # We don't print a status message here, as dpkg already said > @@ -189,6 +301,7 @@ case "$1" in > fi > =20 > set_lvm_conf > + set_ceph_crash_conf > =20 > if test ! -e /proxmox_install_mode; then > # modeled after code generated by dh_start > --=20 > 2.39.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20