Class-imposed login restrictions

[yuichiro-naito]

I want to add this feature to OpenSSH-portable. It is obtained from FreeBSD.
On FreeBSD, login.conf(4) has following fields to restrict users login.

1. host.allow
2. host.deny
3. times.allow
4. times.deny

I added a check logic in input_userauth_request function for that can
disconnect ssh session if a user is not allowed to login.

This function is executed in capability mode on FreeBSD. So, getting the class
structure must be proxied via the monitor process. It never be allowed to open
/etc/login.conf directly. I added the proxy functions mm_login_getpwclass
and mm_answer_login_getpwclass for this reason.

And checking allowed time to login, it is needed to refer the timezone database
that is also disallowed in capability mode. I added to call tzset(3) to cache
the timezone database just before entering capability mode. Tzset(3) is wrapped
by caph_cache_tzdata function in capsicum_helpers.h. caph_cache_tzdata is
a better name that means what to be done. capsicum_helper.h has been available
since FreeBSD 12.0.

Login_getpwclass(3) requires struct passwd data, so I added send/receive
functions. These seems to be upper level than sshbuf-getput-basic functions.
I added a new source file sshbuf-getput-passwd.c to include new functions.

Sending and receiving struct passwd are also used in mm_getpwnamallow and
mm_answer_pwnamallow. I changed to use my functions for consistent use.

And the GETPW and PUTPW macros are used only for sending/receiving
struct passwd. Now they are moved to ssh-getput-passwd.c so that never
affects other functions.

I believe this patch doesn't affect to NetBSD and OpenBSD for now.
If these BSDs implement login_getpwclass(3), auth_hostok(3) and auth_timeok(3) functions,
this patch will work as same as FreeBSD.

[daztucker]

I think this could be implemented less intrusively, and as a general policy when an authentication fails sshd should not leak information about why (ie it should do the packet_disconnect bit).

There's an existing hook that could be used for this (sys_auth_allowed_user, although I'd probably make it more like the other hooks in platform.c before reusing it).

[yuichiro-naito]

I think this could be implemented less intrusively,

Indeed. If I can close a connection in the monitor process, the implementation will be much easier.
When I tried to call ssh_packet_disconnect in getpwnamallow function, my ssh client got a following error.

Bad packet length 830298898.
ssh_dispatch_run_fatal: Connection to 192.168.3.3 port 222: Connection corrupted

So, I think ssh_packet_disconnect must be called in the authenticaion process.

If getpwnamallow returns a fakepw in the case of class imposed check was failed,
and if so, input_userauth_request closes a connection.

Is it a good idea?

and as a general policy when an authentication fails sshd should not leak information about why (ie it should do the packet_disconnect bit).

OK. Is "Authentication Failure" an acceptable message for this policy?

There's an existing hook that could be used for this (sys_auth_allowed_user, although I'd probably make it more like the other hooks in platform.c before reusing it).

I can see it makes allowed_user return false and getpwnamallow return NULL.
In this case, authentication is repeated until max_authtries times.
I think it is useless. It will never be recovered although how many times repeated.

[daztucker]

Indeed. If I can close a connection in the monitor process, the implementation will be much easier.

You can't call packet_disconnect in the monitor (because it's not directly connected to the client), but you shouldn't be doing that at all anyway.

So, I think ssh_packet_disconnect must be called in the authenticaion process.

right.

OK. Is "Authentication Failure" an acceptable message for this policy?

I don't think it should be generating any messages at all.

I can see it makes allowed_user return false and getpwnamallow return NULL.
In this case, authentication is repeated until max_authtries times.

Right, that's what should happen when the authentication is denied.

I think it is useless. It will never be recovered although how many times repeated.

That's what should happen when the authentication is denied. Behaving any differently leaks information about the authentication policy.

[yuichiro-naito]

I think it is useless. It will never be recovered although how many times repeated.
That's what should happen when the authentication is denied. Behaving any differently leaks information about the authentication policy.

OK. Thanks, I understand the policy of authentication failure.

I'm going to implement this feature in getpwnamallow.
It already calls login_getpwclass when HAVE_LOGIN_CAP.
I reuse the result of login_cap_t.

My implementation will be different from this PR.
I will close this PR and create new one after I finished re-implementation.