packaging: Require account validation with pam_unix.so if PAM enabled

[ton31337]

With a current pam_rootok.so, it works only with root account. If the user is under frrvty, frr group, it gets the error:

% groups | grep -o -E "frrvty|frr"
frrvty
frr

% vtysh -c 'end'
vtysh_pam: Failed in account validation: Permission denied(6)

Checking the logs:

vtysh[23930]: pam_rootok(frr:account): root check failed

Let's require a valid user, instead of the root user only.

[donaldsharp]

Possible to get @eqvinox to look at this?

[eqvinox]

Hard NAK.

PAM is incomplete in FRR and should in fact be disabled. It would only work correctly if vtysh is setuid root. pam_unix.so requires root privileges, which you don't have when executing vtysh.

A possible reason to ship FRR with PAM enabled is so that users can enable it without rebuilding FRR, by first setting vtysh setuid root and then making a PAM config. However, we do not test this and it very likely does not work correctly and creates security holes.

OTOH the reason why we're not disabling PAM is probably that users and distro packagers would get confused ("why are you removing security?".) It's a bit of a stupid situation.

[eqvinox]

Actually the correct fix here is to replace pam_rootok with pam_permit.

Except then people will complain that we created a security hole on their system by opening something up.

sigh

[ton31337]

More context: #14745

[eqvinox]

More context: #14745

Actually it looks like we ignore the PAM result anyway. Which means maybe we can fix this properly because that means we can't be breaking someone's security (because it was already broken, lol.)

[eqvinox]

Ok. Yeah. So. This is what we're currently doing: https://github.com/FRRouting/frr/blob/master/vtysh/vtysh_user.c#L51-L74

• we run the auth check. If it fails, we refuse. Which is pointless for 2 reasons: (a) we already have a logged-in user, we're not trying to become a user, and (b) it defaults to pam_permit anyway. The only reason to do this is that we have to establish who we are for the acct check next (⇒ pam_permit is the correct thing! We're already logged in.)

• we run the acct check. If it fails, we print a message and continue anyway. (Note second_ret is overwritten with the result from pam_end.) In theory, this would be the place where some admin could put extra security settings. But it does not work for FRR.

• ultimately we connect() to the daemon VTY sockets. Whether we can do this depends solely on whether we are in the frrvty group or not. If we already are in the frrvty group before the entire PAM circus, we can always do it, and the user can also just use some other tool to directly open the daemon VTY sockets, e.g. socat. If we are not in the frrvty group before the PAM circus, PAM can't add it (because we're not root) and we wil fail anyway regardless of PAM.

⇒ the entirety of PAM in FRRouting is non-functional

Generally speaking, all pam_* functions must be executed with root privilege to work correctly. This is rather poorly documented, but inherent by how PAM operates — the PAM modules need to access files accessible only by root (e.g. /etc/shadow).

[ton31337]

Okay, so just to "hide" the warnings, we can use pam_permit.so? I can change that if this is the only change.

[eqvinox]

Okay, so just to "hide" the warnings, we can use pam_permit.so? I can change that if this is the only change.

Yes. In lieu of either disabling it completely, or completely reworking it to do something useful (which would involve making vtysh suid and/or sgid), using pam_permit is a good band-aid.

I would also add a comment to the /etc/pam.d/frr file saying that PAM in FRR is non-functional.

NB: /etc/pam.d/frr is likely not overwritten by package updates, i.e. we may need to make a note somewhere for people seeing this issue after FRR or system upgrades.