shadow-maint / shadow

Upstream shadow tree
Other
305 stars 232 forks source link

terminal message is truncated #252

Open wrjiafang opened 4 years ago

wrjiafang commented 4 years ago

Hi, all I use terminal server to connect to my board ttyS0 via serial cable. After I use command to "chage -E 2020-05-09 a" to disable the user "a", when login to my board, then the message is like this: " login: a Password: Your account has ex " The normal print should be: " login: a Password: Your account has expired; please contact your system administrator

Authentication failure

" It seems like the message is truncated. The board's booting up message is normal, and when use ssh to login, the message is also normal. I doubt it is because the login is exit too quick then the output is disabled.

From src/login.c, the context is here: [ / Check the account validity / retcode = pam_acct_mgmt (pamh, 0); if (retcode == PAM_NEW_AUTHTOK_REQD) { retcode = pam_chauthtok (pamh, PAM_CHANGE_EXPIRED_AUTHTOK); } PAM_FAIL_CHECK;

define PAM_FAIL_CHECK if (retcode != PAM_SUCCESS) { \

    fprintf(stderr,"\n%s\n",pam_strerror(pamh, retcode)); \
    SYSLOG((LOG_ERR,"%s",pam_strerror(pamh, retcode))); \
    (void) pam_end(pamh, retcode); \
    exit(1); \

}](url)

The "Your account has expired; please contact your system administrator" belongs to pam_acct_mgmt(). and the "Authentication failure" should be from "fprintf(stderr..)"

Do you have any suggestions about this issue ? Should I add some delay before "exit(1)" in PAM_FAIL_CHECK ?

Thanks

hallyn commented 4 years ago

Rather than a delay, an fflush(stderr) might be more appropriate? Can you easily build+test that, and open a PR if it works?

wrjiafang commented 4 years ago

@hallyn Thanks for your reply. I tried the fflush(stderr) and fflush(stdout) like this: fflush(stdout); fflush(stderr); PAM_FAIL_CHECK; and

define PAM_FAIL_CHECK if (retcode != PAM_SUCCESS) { \

    fprintf(stderr,"\n%s\n",pam_strerror(pamh, retcode)); \
    fflush(stderr); \

Unfortunately, it did not work :(

hallyn commented 4 years ago

Could you try adding fclose(stderr) before exit?

wrjiafang commented 4 years ago

Changed as: " (void) pam_end(pamh, retcode); \ fclose(stderr); \
exit(1); \ " Log: " login: a Password: Your account has expired; plea " It did not work, till now the usleep(10000) before exit() worked.

hallyn commented 4 years ago

I did mean both fflush and fclose.

I do believe it still won't work, but let's make sure.

It's possible that switching to not using streams (i,e., sprintf to a string and then write(2, buf, strlen(buf)) will work.

Note I can't reproduce it easily here. Are you doing this in a VM, or on physical hardware?

hallyn commented 4 years ago

I also wonder whether adding "--delay 10" to your agetty spawn line would help. It tells agetty to wait before opening the console. It's possible that output can continue to flow until it's re-opened for a new session, I'm not sure.

I'll aim to also test this in a vm soon, but it probably won't be today.

wrjiafang commented 4 years ago

1) code: (void) pam_end(pamh, retcode); \ fflush(stderr); \ fclose(stderr); \
exit(1); \ It does not help.

2) after the login exit, we will restart the agetty which is actually a ttyS0 service of systemd. Before restart, I tried to add 5s, but it also does not help. 3) I failed to reproduce on our qemu system. So actually, I only have this issue on the hardware, ppc hardware, x86 hardware and so on. I guess if you use the libpam and shadow/login, on any hardware with ttyS0 console, it should have the same issue, but I'm not so sure, you can try on your side first, hope you can also test on actual machine.

hallyn commented 4 years ago

Hi,

Actually, yes. We should wait for FAIL_DELAY seconds in the PAM_FAIL_CHECK, as well as in the other expired account path (around line 1188). Can you propose a patch to do that?

wrjiafang commented 4 years ago

OK, I will cook one patch. the expired account patch, do you mean in the function of this expire (pwd, spwd) : 1177 if (NULL != spwd) { / check for age of password / 1178 if (expire (pwd, spwd)) {
1179 / The user updated her password, get the new 1180 entries. 1181 Use the x variants because we need to keep the 1182 entry for a long time, and there might be other 1183 getxxyyy in between. 1184 / 1185 pw_free (pwd);