Bug#824610: rkhunter: SSH PermitRootLogin is fragile and incomplete

[Christoph Anton Mitterer]

Package: rkhunter
Version: 1.4.2-5
Severity: normal
Tags: security upstream


Hi.

AFAIU, rkhunter does roughly the following to check for the value of PermitRootLogin.
Goes through SSH_CONFIG_DIR or /etc /etc/ssh /usr/local/etc /usr/local/etc/ssh and
looking for sshd_config, taking the first found.
Case-insensitively greps for "PermitRootLogin", only looking at the first result,
doing some further regexp playing to get the value out of that line.

1) There's no guarantee the file is called sshd_config and that seems not to be configurable.
2) The fallback with going through that dirs and testing only the first found match is a bit
   fragile IMHO. The fil could exist in multiple locations, but the one actually used could
   be form a later dir, which rkhunter woulnt' check anymore.
3) The parsing is IMHO a bit fragile. Nothing prevent's upstream from changing the syntax and
   semantics, especially that the "first" assignment wins could be easily changed.
   Newer sshd versions have the -T option, which can be used to give a standardised output of
   the effective configuration. That should be used, I'd say. (However, see later)
4) sshd_config syntax allows values to be enclosed in double quotes - AFAIK the parser doesn't
   handle this

most important an security relevant is IMHO:

5) It's not Match block aware.
   The Match blocks lead to different effective values (at runtime) for PermitRootLogin,
   depending on the match criteria.
   An sshd_config like:
   PermitRootLogin no
   Match User *
      PermitRootLogin yes

   would already trick rkhunter into believing it's "no", which it effectively is "yes".

   Unfortunately here this is where the nice -T fails... :-( ... while there is -C to, it
   cannot be used to "select" a certain match block (which we could parse for), but only to
   give the criteria (and it's difficult to set them up so that all Match blocks would get
   matched once).

   So in the end I'd say we should grep for something like:
   1) grep -i '^[[:space:]]*PermitRootLogin[[:space:]][[:space:]]*' "$SSHD_CONFIG"
      (not dropping any lines)
   2) remove the directive:
       sed 's/^[[:space:]]*PermitRootLogin[[:space:]][[:space:]]*//'
   3) remove double quotes must be done in an extra step, as we MUST only remove " if theres
      one at the beginning AND the end:
      sed 's/"\(.*\)"/\1/'
   4) sort -u the output

   If now multiple lines are left, it means we have different values either in Matchblocks
   or outside of match blocks.
   For both cases I'd say the rkhunter test should give a warning.
   If only one line is left, I'd continue to compare it to the expected value set in
   rkhunter.conf.


6) Oh and it seems current regexps assume one could write directive=value, but I don't think
   this is possible in the config syntax, or is it?


Cheers,
Chris