Mailing List Archive: OpenSSH: Dev

AllowUsers "logic" and failure to indicate bad configuration

Index | Next | Previous | View Threaded

mpatton at inforelay

Aug 9, 2012, 7:32 PM

Post #1 of 2 (228 views)
Permalink

AllowUsers "logic" and failure to indicate bad configuration

I smacked into this previously reported bug today whereby an invalid
keyword in the Match{} stanza did not throw an error on configuration
reload. Are there any plans to fix this? Likewise the penchant for some
fields to be comma separated and others to be spaces is just asking for
mistakes. Why not support both and be done with it? There was no response
(that I saw in the archives) to this post
http://marc.info/?l=openssh-unix-dev&m=132311628508429&w=2

Like him, I'm using 5.3p1 as packaged in CentOS 6.3.

Secondly the Allow/Deny logic is downright tortured. I looked back and
again didn't come across any good discussion as to why it was written that
way. It should not be necessary for AllowUsers to be the superset of
AllowGroups. As Spock would say "it is illogical." If you had to write PF
rules like that you'd go crazy. That's why most people use first-match
logic.

Per the manpage, if the logic is DenyUsers > AllowUsers > DenyGroups >
AllowGroups, then there has to be a immediate stop to the logic chain at
each stage.

if $user ~= %DenyUsers; then ( deny; return )
if $user ~= %AllowUsers; then { allow; return )
if $user member of %DenyGroups; then (deny; return )
if $user member of %AllowGroups; then (allow; return )
if (%AllowUsers != null || %AllowGroups != null); then deny

Anything else makes no sense. Can we expect a change in the future? Or if
I were to submit a patch to fix this, would it be accepted?

The other flaw I see here is the treatment of 'root'. There are already
special keywords controlling that UID, so it makes no sense to have to tag
'root' into this logic either. PermitRootLogin is supposed to be
sufficient to dictate that particular outcome.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

mpatton at inforelay

Aug 9, 2012, 8:38 PM

Post #2 of 2 (212 views)
Permalink

Re: AllowUsers "logic" and failure to indicate bad configuration [In reply to]

> way. It should not be necessary for AllowUsers to be the superset of
> AllowGroups.

Sorry, I meant to say AllowGroups should not have to include at least one
GID from each of AllowUsers.

I forgot to add that a case could be made for the Allow/Deny directives to
be valid within all Match{} stanzas as well. Right now there is only one
global rule. However, it makes perfect sense to want to selectively
override it.

At first blush I thought that maybe the rules specified in Match{} would
just be used to "flip" the logic arrived at from the main body; eg.
AllowUsers could effectively negate any rejection reached by the main body
logic.

Then I thought that maybe it would be better to treat it as a complete
substitution of the matching global rule directive. To me, this seems to
be the most elegant but will have to be clearly spelled out in
documentation.

The 3rd option would call for ignoring the entire global rule in favor of
building a new one based strictly off the directives contained within
Match{}. Yes, in most cases they'll be a lot of cut/pasting from the
global rule and tweaking them to taste, but the intent of the stanza is
unambiguous.

If we were to implement option 2, specifying all 4 directives would
achieve the same result as option 3.

Thoughts?

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev [at] mindrot
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads