Page MenuHomeVyOS Platform

Control over which users have ssh access
Closed, ResolvedPublic


Would be nice to be able to prevent users from having ssh access, forcing them to be used via console only for disaster recovery scenarios. ( In our environment we don't want the vyos user to be used on a day-to-day basis but do want it for emergency console logins )

This can be done by adding all users to the AllowGroups option in sshd_config except for those that have a 'disable-ssh-access' flag set.


Difficulty level
Normal (likely a few hours)

Event Timeline

syncer triaged this task as Wishlist priority.Aug 8 2016, 4:52 PM
syncer added a subscriber: syncer.

Good point,
not sure if we can combine this with T110 task as they are both about SSH

Seems sensible to combine them both are similar changes and will be managed by same cfg scripts/templates.

Would this be a setting in the SSH service or rather system login. Because the former allows you to employ wildcards: VYOS-* while the latter feels more correct otherwise. Or you could have both, default the SSHd setting to no-one, and whitelist per user || employ the wildcard solution.

I was thinking of just updating the ssh service

I propose to introduce config nodes to create AllowUsers, AllowGroups, DenyUsers and DenyGroups settings in sshd_config. Additionally, I propose to introduce a sshd-option config node.

git commit
git pull request

I think T141 also wants to achieve something similar but with proper AAA. Unfortunately my network has not reached the critical mass to go for a TAC server. But we should keep this in mind!

Yeah. I ignored T141 by purpose for two reasons:

  • I do not know enough about what proper AAA support would exaclty mean.
  • Maybe doing this one step at a time would make incorporating changes easier instead of trying to achieve it all in one run?

Therefore, I just concentrated on T122 and tried to find a minimal invasive solution just for this.

@alainlamar thanks for sharing your thoughts. Regarding your MR, you add a node sshd_option where someone could add ANY option to sshd. I'm not a big fan of those "you can do everything nodes".

Whats your thought @dmbaturin @syncer @UnicronNL?

Tip:: We should have one new configuration command per Git commit. Usually Git commits represent exactly one feature. So best case would be 4 commits for this change as you introduce for new configuration nodes. Where I like the three for user / group access.

@c-po I re-arranged the code and the git commits and hope things are fine now so this ticket may come to its deserved end of life.

Yours truly!

@alainlamar your effort and support is tremendous and very much appreciated. I'm also super new to VyOS "development".

Reviewing your implementation and re-assembeleing a config in my mind produced

ssh {
    access-control {
        allow-groups sudo,admin,foo
        allow-users admin1,admin2,foo
        deny-groups lp,root,daemon
        deny-users syslog,uberadmin
    port 22

Which looks just like a plain wrapper to sshd_config. I doubt that this is the intention of the VyOS / Vyatta inventors. To me this implementation has one big drawback which is that you always have to alter the full configuration line which makes it kind of error prone. Some may easily forget a user/group by altering the configuration line.

Having a configuration like

ssh {
    access-control {
        allow {
            group admin
            group sudo
            user admin1
            user foo
            user bar
    port 22

will solve aboves problem and you will get TAB completion for free. Man(5) @ states:

This keyword can be followed by a list of user name patterns, separated by spaces. If specified, login is allowed only for user names that match one of the patterns.

When having a whitelist ACL for SSH users, is an explicit blacklist required at all?
As current interaction between VyOS and sshd_config is simple BASH-foo (basic sed operations on the config) my proposed configuration will not be that easy. But it would be a very good start on vyos-1x XML interface.

@alainlamar @c-po Multi-nodes should indeed be used whenever possible instead of making the user enter comma-separated values and the like.

I suggest that we improve and revamp this before officially announcing it in release notes.

It all works now, right? Closing then.