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

Remove blank lines in rendered ssh_config and sshd_config #148

Open
johnnybubonic opened this issue Jan 29, 2019 · 6 comments
Open

Remove blank lines in rendered ssh_config and sshd_config #148

johnnybubonic opened this issue Jan 29, 2019 · 6 comments

Comments

@johnnybubonic
Copy link

The conf files you generate are either broken or render to eyesores such as this:

# The contents of the original sshd_config are kept on the bottom for
# quick reference.
# See the sshd_config(5) manpage for details

Port 22
Protocol 2
HostKey /etc/ssh/ssh_host_rsa_key
HostKey /etc/ssh/ssh_host_ed25519_key
UsePrivilegeSeparation sandbox
SyslogFacility AUTHPRIV
LogLevel INFO
ClientAliveInterval 1
ClientAliveCountMax 300
LoginGraceTime 120
PermitRootLogin prohibit-password
StrictModes yes
MaxAuthTries 6
MaxSessions 5
PubkeyAuthentication yes
AuthorizedKeysFile %h/.ssh/authorized_keys
ChallengeResponseAuthentication no
AuthenticationMethods publickey
PasswordAuthentication no
Banner /etc/ssh/banner
AcceptEnv XMODIFIERS
Subsystem sftp /usr/lib/ssh/sftp-server
UsePAM yes

AllowUsers root


AllowGroups root

KexAlgorithms curve25519-sha256@libssh.org,diffie-hellman-group-exchange-sha256
Ciphers chacha20-poly1305@openssh.com,aes256-gcm@openssh.com,aes128-gcm@openssh.com,aes256-ctr,aes192-ctr,aes128-ctr
MACs hmac-sha2-512-etm@openssh.com,hmac-sha2-256-etm@openssh.com,umac-128-etm@openssh.com,hmac-sha2-512,hmac-sha2-256,umac-128@openssh.com

Match Group sftpusers #sftp_chroot
    AllowTcpForwarding no

    ChrootDirectory /sftp-chroot/%u

    ForceCommand internal-sftp

    X11Forwarding no


Yes, including trailing whitespace.

I'd remove the blank lines between UsePAM/AllowUsers/AllowGroups, and remove the blank lines between all items in the Match section as well as the trailing blank lines. These blank lines make the config harder to visually parse, as they are separating logical sections that should be together.

@myii
Copy link
Member

myii commented Jan 29, 2019

@johnnybubonic I haven't made any contributions to this repo so I can only speak in general. My inclination is to agree with the points you are raising but I've recently been made aware of a situation which complicates things. It started with an issue and PR in the postgres-formula:

The problem was that a fix for some is a whitespace bomb for others -- as I mentioned in my comment regarding the fix.

This led back to a stale issue in the main repo. After various discussions, that issue has been re-opened, so hopefully we can attain consistency when making fixes such as the ones you are suggesting.

@johnnybubonic
Copy link
Author

@myii Thanks for bringing that upstream issue to my attention re: per-file trimming etc. I've subscribed to it to follow. It's a good thing SSH doesn't have whitespace-dependent parsing! I can only imagine the headache for things that do.

I think a lot of it - at least for this issue, for openssh-formula - can be addressed by just getting rid of the majority if not all of the macros. They're superfluous - many times they're used where simple for-loops could just be used instead with raw whitespacing in the template rather than trying to macro out the whitespacing/indentation and linebreaking. It seems to largely be an unnecessary complication. Alternatively, changing to something like a raw py template instead would probably be a lot more elegant.

@johnnybubonic
Copy link
Author

or one could just use the augeas state instead.

@alxwr
Copy link
Member

alxwr commented Feb 12, 2019

I think a lot of it - at least for this issue, for openssh-formula - can be addressed by just getting rid of the majority if not all of the macros. They're superfluous - many times they're used where simple for-loops could just be used instead with raw whitespacing in the template rather than trying to macro out the whitespacing/indentation and linebreaking. It seems to largely be an unnecessary complication. Alternatively, changing to something like a raw py template instead would probably be a lot more elegant.

@johnnybubonic If the resulting template is generally easier to understand, I'll be happy to merge your PR. Please stick to SLS. While it may not always be as elegant as Python, it lowers the bar for contributions.

@johnnybubonic
Copy link
Author

sure - i'll try to get a PR in. not sure when that'll be exactly (i'm.. in the middle of converting our infra over to salt, obviously...), but i'll keep it to SLS. would you be OK with using the augeaus state in addition/instead? it should be part of salt core.

@alxwr
Copy link
Member

alxwr commented Feb 13, 2019

@johnnybubonic AFAIK augeas needs additional python libraries on the minion. Therefore I (and I think others) favor SLS templates.

Have a look at https://github.com/saltstack-formulas/systemd-formula/blob/master/TOFS_pattern.md too, if you want to use alternative template files.

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

No branches or pull requests

3 participants