Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Augeas can't parse systemd unit file entry ExecStartPre=+-command #839

Closed
rwmjones opened this issue Jul 31, 2024 · 4 comments · Fixed by #841
Closed

Augeas can't parse systemd unit file entry ExecStartPre=+-command #839

rwmjones opened this issue Jul 31, 2024 · 4 comments · Fixed by #841

Comments

@rwmjones
Copy link
Contributor

With the latest SSSD 2.10 was added a systemd unit file that contains:

ExecStartPre=+-/bin/chown -f sssd:sssd /etc/sssd/sssd.conf
ExecStartPre=+-/bin/chown -f -R sssd:sssd /etc/sssd/conf.d
ExecStartPre=+-/bin/chown -f -R sssd:sssd /etc/sssd/pki
ExecStart=/usr/sbin/sssd -i ${DEBUG_LOGGER}

The new ExecStartPre=+-... entries were added in this version and Augeas fails to parse them:

$ augtool print /augeas//error | grep sssd.service
/augeas/files/etc/systemd/system/multi-user.target.wants/sssd.service/error = "parse_failed"
/augeas/files/etc/systemd/system/multi-user.target.wants/sssd.service/error/pos = "413"
/augeas/files/etc/systemd/system/multi-user.target.wants/sssd.service/error/line = "14"
/augeas/files/etc/systemd/system/multi-user.target.wants/sssd.service/error/char = "0"
/augeas/files/etc/systemd/system/multi-user.target.wants/sssd.service/error/lens = "/usr/share/augeas/lenses/dist/inifile.aug:497.25-.43:"
/augeas/files/etc/systemd/system/multi-user.target.wants/sssd.service/error/message = "Get did not match entire input"
/augeas/files/lib/systemd/system/sssd.service/error = "parse_failed"
/augeas/files/lib/systemd/system/sssd.service/error/pos = "413"
/augeas/files/lib/systemd/system/sssd.service/error/line = "14"
/augeas/files/lib/systemd/system/sssd.service/error/char = "0"
/augeas/files/lib/systemd/system/sssd.service/error/lens = "/usr/share/augeas/lenses/dist/inifile.aug:497.25-.43:"
/augeas/files/lib/systemd/system/sssd.service/error/message = "Get did not match entire input"

I found through experimentation that the actual problem is the use of + and - characters together. This is valid for systemd, but Augeas only understands a single character here (eg. just + or just - works, but not two or more).

@rwmjones
Copy link
Contributor Author

FYI this is the downstream bug filed against RHEL: https://issues.redhat.com/browse/RHEL-50778

@tupyy
Copy link
Contributor

tupyy commented Aug 6, 2024

Hello,
This is a systemd issue not just sssd.

[Unit]
Description=OpenSSH server daemon
Documentation=man:sshd(8) man:sshd_config(5)
After=network.target sshd-keygen.target
Wants=sshd-keygen.target
# Migration for Fedora 38 change to remove group ownership for standard host keys
# See https://fedoraproject.org/wiki/Changes/SSHKeySignSuidBit
Wants=ssh-host-keys-migration.service

[Service]
Type=notify
EnvironmentFile=-/etc/sysconfig/sshd
ExecStart=+-/usr/sbin/sshd -D $OPTIONS
ExecReload=/bin/kill -HUP $MAINPID
KillMode=process
Restart=on-failure
RestartSec=42s

[Install]
WantedBy=multi-user.target

tupyy added a commit to tupyy/augeas that referenced this issue Aug 6, 2024
This commit allows the "+"(fullprivileges) command flag along with
"-" and "@" flags.

Fixes: hercules-team#839

Signed-off-by: [email protected]
Reported-by: [email protected]
tupyy added a commit to tupyy/augeas that referenced this issue Aug 6, 2024
This commit allows the "+"(fullprivileges) command flag along with
"-" and "@" flags.

Fixes: hercules-team#839

Signed-off-by: Cosmin Tupangiu <[email protected]>
Reported-by: Yongkui Guo <[email protected]>
tupyy added a commit to tupyy/augeas that referenced this issue Aug 14, 2024
This commit allows the "+"(fullprivileges) command flag along with
"-" and "@" flags.

Fixes: hercules-team#839

Signed-off-by: Cosmin Tupangiu <[email protected]>
Reported-by: Yongkui Guo <[email protected]>
@georgehansper
Copy link
Member

If the order of the flags is reversed, the existing lens will load, ie.

ExecStart=-+/usr/sbin/sshd -D $OPTIONS

becomes

/files/lib/systemd/system/sshd.service/Service/ExecStart
/files/lib/systemd/system/sshd.service/Service/ExecStart/ignoreexit
/files/lib/systemd/system/sshd.service/Service/ExecStart/command = "+/usr/sbin/sshd"
/files/lib/systemd/system/sshd.service/Service/ExecStart/arguments
/files/lib/systemd/system/sshd.service/Service/ExecStart/arguments/1 = "-D"
/files/lib/systemd/system/sshd.service/Service/ExecStart/arguments/2 = "$OPTIONS"

Pull Request #841 turns the '+' char into a separate path (flag) in augeas, but this it not strictly necessary as the '+' can be treated as part of the "value"

The fact that this is dependant on the order of the '+-' flags is not desirable

@rwmjones
Copy link
Contributor Author

rwmjones commented Aug 18, 2024

If the order of the flags is reversed, the existing lens will load, ie.

ExecStart=-+/usr/sbin/sshd -D $OPTIONS

But the config file that exists uses =+-. The config file that exists in Fedora's sssd cannot be parsed by augeas.

becomes

/files/lib/systemd/system/sshd.service/Service/ExecStart
/files/lib/systemd/system/sshd.service/Service/ExecStart/ignoreexit
/files/lib/systemd/system/sshd.service/Service/ExecStart/command = "+/usr/sbin/sshd"

And this parsing is wrong. + is a flag with a specific meaning to systemd: https://www.man7.org/linux/man-pages/man5/systemd.service.5.html#COMMAND_LINES (just like ignoreexit has a meaning to systemd). + isn't part of the command.

tupyy added a commit to tupyy/augeas that referenced this issue Aug 23, 2024
This commit allows the "+"(fullprivileges) command flag along with
"-" and "@" flags. Order does not matter.

Fixes: hercules-team#839

Signed-off-by: Cosmin Tupangiu <[email protected]>
Reported-by: Yongkui Guo <[email protected]>
tupyy added a commit to tupyy/augeas that referenced this issue Aug 23, 2024
This commit allows the "+"(fullprivileges) command flag along with
"-" and "@" flags. Order does not matter.

Fixes: hercules-team#839

Signed-off-by: Cosmin Tupangiu <[email protected]>
Reported-by: Yongkui Guo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants