Expired accounts can login using netconf

[DineshReddyK]

Hi,

netconf password based authentication works even for expired accounts.
Is this expected and ignored?

Here test007 is expired already.

[root@k8s-1 /]# date
Tue Nov  3 10:48:05 UTC 2020

[root@k8s-1 /]# chage -l test007
Last password change                                    : Nov 03, 2020
Password expires                                        : never
Password inactive                                       : never
Account expires                                         : Nov 02, 2020
Minimum number of days between password change          : -1
Maximum number of days between password change          : -1
Number of days of warning before password expires       : -1

netopeper2-cli still able to successfully authenticate.

[root@k8s-1 /]# netopeer2-cli
> connect --login test007
Interactive SSH Authentication
Type your password:
Password:
> status
Current NETCONF session:
  ID          : 5
  Host        : 127.0.0.1
  Port        : 830
  Transport   : SSH
  Capabilities:
        urn:ietf:params:netconf:base:1.0

[michalvasko]

Users and their password are managed directly by libnetconf2 so this was simply not implemented. Could you please try this patch and provide some feedback? I tested it only briefly on current devel.

diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c
index 4f0c321..573e986 100644
--- a/src/session_server_ssh.c
+++ b/src/session_server_ssh.c
@@ -700,6 +700,9 @@ auth_password_get_pwd_hash(const char *username)
         if (!spwd) {
             VRB("Failed to retrieve the shadow entry for \"%s\".", username);
             return NULL;
+        } else if ((spwd->sp_expire > -1) && (spwd->sp_expire < time(NULL))) {
+            WRN("User \"%s\" account has expired.", username);
+            return NULL;
         }
 
         pass_hash = spwd->sp_pwdp;

[DineshReddyK]

Sure, I will apply this and test, but quickly looking at the change, are we comparing days with seconds?
sp_expire is in days, where as time(NULL) returns seconds.

Also, do you think we need to handle sp_inact ?
I haven't checked this part yet. Once done I will try to provide patch.

[DineshReddyK]

The following patch should work

diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c
index 29bd562..cc29398 100644
--- a/src/session_server_ssh.c
+++ b/src/session_server_ssh.c
@@ -700,6 +700,9 @@ auth_password_get_pwd_hash(const char *username)
         if (!spwd) {
             VRB("Failed to retrieve the shadow entry for \"%s\".", username);
             return NULL;
+        } else if ((spwd->sp_expire > -1) && (spwd->sp_expire <= (time(NULL) / (60 * 60 * 24)))) {
+            WRN("User \"%s\" account has expired.", username);
+            return NULL;
         }

2020-11-04 05:55:56,839 DEBG 'netopeer2-server' stdout output:
[WRN]: LN: User "netconf" account has expired.

2020-11-04 05:55:56,840 DEBG 'netopeer2-server' stdout output:
[INF]: LN: Failed user "netconf" authentication attempt (#1).

If you think sp_inact should be handled, then that is different issue.
But sample code should be something like this.

	long int curdays;
	curdays = (long int)(time(NULL) / (60 * 60 * 24));

	if ((curdays - spwd->sp_lstchg > spwd->sp_max)
	    && (curdays - spwd->sp_lstchg > spwd->sp_inact)
	    && (curdays - spwd->sp_lstchg > spwd->sp_max + spwd->sp_inact)
	    && (spwd->sp_max != -1) && (spwd->sp_inact != -1)) {
		WRN("User \"%s\" account expired.", username);
		return NULL;
	}
	if ((curdays - spwd->sp_lstchg > spwd->sp_max) && (spwd->sp_max != -1)) {
		WRN("User \"%s\" need a new password.", username);
		return NULL;
	}

[michalvasko]

Right, mixed those days and seconds. I committed the fixed patch and it seems to me sp_inact does not need to be handled based on the documentation.

[DineshReddyK]

Ok. At lease I see that as a security issue.
But since it is not related to the current issue being raised, I close this now.