hercules-team / augeas

A configuration editing tool and API
http://augeas.net/
GNU Lesser General Public License v2.1
483 stars 199 forks source link

ignoring case for sshd_config #837

Open xkcd386at opened 2 months ago

xkcd386at commented 2 months ago

Hi

I'm just starting to learn augeas and it looks very slick; thank you.

The problem I ran into was that my existing sshd_config had lowercase entries (e.g., passwordauthentication no instead of PasswordAuthentication no). Sshd does allow it, and in fact when you run sshd -T -- which outputs the "effective configuration to stdout" -- everything is lowercase.

So, is there a way to tell autool to detect existing lines case-INsensitively?

thanks again

tupyy commented 3 weeks ago

Hi.

I don't think so. For every type of file, there's a lens file that defines how that file is parsed. In your case is lens/sshd.aug.

georgehansper commented 2 weeks ago

Firstly, let me see if I have understood your question properly

The existing lens, sshd.aug, does not have the keyword "PasswordAuthentication" hard-coded into it

It will accept the both the lower-case and mixed-case variants of this keyword.

So a hypothetical sshd_config file:

# Sample sshd_config
PasswordAuthentication yes
passwordauthentication no

generates the tree

/files/etc/ssh/sshd_config/#comment = "Sample sshd_config"
/files/etc/ssh/sshd_config/PasswordAuthentication = "yes"
/files/etc/ssh/sshd_config/passwordauthentication = "no"

If you know that the file you are editting has either of these as an existing line, the following set-command would suffice:

set /files/etc/ssh/sshd_config/*[label()="PasswordAuthentication" or label()="passwordauthentication"] yes

There is no (existing) case-insensitive path expression that could be used, other than a rather cumbersome regexp like this:

set /files/etc/ssh/sshd_config/*[label()=~regexp("^[Pp][Aa][Ss][Ss][Ww][Oo][Rr][Dd][Aa][Uu][Tt][Hh][Ee][Nn][Tt][Ii][Cc][Aa][Tt][Ii][Oo][Nn]$")] yes

However, either of the above statements hits a problem if there is no existing line with either "PasswordAuthentication" or "passwordauthentication" in the file

This stems from a fundamental problem with Augeas path-expressions. These are very good at selecting an existing item in the file, and making idempotent changes to this item They are not so good at adding new items to a file in an idempotent way The problem is that, if there is no existing path to match, there is no existing label to assign to the new node

Consider the (idempotent) set statement:

set /files/etc/ssh/sshd_config/PasswordAuthentication yes

If there is no existing line with "PasswordAuthentication", Augeas knows to create a new node with the label "PasswordAuthentication" The new node "PasswordAuthentication" is appended within the "context". In this case, the new line is appended to the file at the end

If there is an existing line, and instead we were to use

set /files/etc/ssh/sshd_config/*[label()="PasswordAuthentication"] yes

Augeas would find the existing node that matches "PasswordAuthentication", and that is OK, too.

If there is NO matching node, the nodeset is empty, and the '*' has taken the place of the label "PasswordAuthentication" in the previous example. So Augeas has no idea what label to assign, if it were to create a new node The end result is that the set command fails with an error message

It would be nice to be able to create a path-expression with a default label if none is found, eg *[ expr else new("somelabel") ] At the moment there is no such functionality

What IS possible, although again it is a little cumbersome, is to use the "else" operator to select an alternate node, like this

set "/files/etc/ssh/sshd_config/*[label()="PasswordAuthentication" or label()="passwordauthentication"] else /files/etc/ssh/sshd_config/PasswordAuthentication" yes

Note the quotation marks around the whole path-expression. These are required by augtool or the API call aug_srun() If you are using Augeas via another API call, then the outermost quotation marks should be omitted

The above statement can be made shorter by using a context, eg in augtool

context /files/etc/ssh/sshd_config
set "*[label()="PasswordAuthentication" or label()="passwordauthentication"] else PasswordAuthentication" yes

Though personally, I prefer the longer form, as it presumes less prior knowledge of Augeas on the part of the reader

I hope this helps for now

xkcd386at commented 2 weeks ago

Thank you. I think I understand most of what you said, though my knowledge of augeas needs to go up quite a bit to grok the whole thing.

I notice within sshd.aug there are many expressions with "/i" appended (e.g., let deny_users = array_entry /DenyUsers/i "DenyUsers"). If that /i means "case-insensitive" (as it does in perl for example) then it means that case-insensitivity is only handled on a keyword-by-keyword basis.

This in turn means that this (in your comment):

These are very good at selecting an existing item in the file, and making idempotent changes to this item

is only true if by "file" you meant "sshd.aug" not "/etc/ssh/sshd_config" (i.e., that directive was "known" to augeas).

For unknown keywords, it is not able to work with a config file whose syntax explicitly ignores case altogether (and, as I mentioned earler, even produces all lowercase when you run sshd -T). I should add that, though this doesn't happen in practice, sshd actually accepts any case (e.g., PassWordAuthentication -- note the "W" capitalised), so enumerating won't help -- it will have to be that huge regex you showed in your comment.

Anyway, I have fallen back on plain old perl because, except for match blocks, sshd_config is largely just a dump of directives with no sequencing issues, so I can safely drop one at the top of the file if it doesn't already exist.

georgehansper commented 2 weeks ago

In the comment that you are referring to, I meant that Augeas is good at changing, say, the "yes" to a "no" in-situ, but we needed to add the the "else" expression in order to add the line if it was absent altogether

Within the lens file sshd.aug, it is possible to use case-insensitive regular expressions, as you have noticed. But this does not extend to Augeas path-expressions (as might be used in augtool).

I think this has highlighted that is room for an enhancement here: namely to add case-insensitive regular expressions to the path-expression syntax.

Right now, I am thinking that the easiest way to achieve this in a backwards-compatible way, would be a create another function regexpi() alongside of the existing regexp() function.

Augeas still has some usability issues, which are gradually being worked on

Thanks for taking the trouble to raise this issue