diff --git a/FEDORA.md b/FEDORA.md index 98d36b5d..7d893538 100644 --- a/FEDORA.md +++ b/FEDORA.md @@ -190,9 +190,10 @@ rights in the user file: ``` Each line of `/etc/newscache.users` is `user:hash:list:read:postto:allow`, -for example `alice:$6$...:*:*:*:read,post`; `openssl passwd -6` makes the -hash. Only `user:hash` is required: fields missing at the end add nothing, -like empty ones. The file is read at login, after NewsCache has switched to its +for example `alice-reader:$6$...:*:*:*:read,post`; `openssl passwd -6` +makes the hash. Only `user:hash` is required: fields missing at the end add +nothing, like empty ones. User names and passwords must have at least 10 +bytes: shorter ones are refused (`AuthMinLength`, default 10). The file is read at login, after NewsCache has switched to its `Username` (news), so that user must be able to read it. Until a client has logged in, it can only use `AUTHINFO`, `CAPABILITIES`, `MODE READER`, `HELP` and `QUIT`. diff --git a/changes/fix-auth-failure-backoff.md b/changes/fix-auth-failure-backoff.md new file mode 100644 index 00000000..e572bd94 --- /dev/null +++ b/changes/fix-auth-failure-backoff.md @@ -0,0 +1,31 @@ +Upgrading: +* Logins with a user name or password shorter than 10 bytes are refused + (481) after the upgrade: the new option AuthMinLength defaults to 10. + Give such users longer names and passwords, or set "AuthMinLength 0" + to keep them. The sample auth file and the examples in the + documentation now use names and passwords of at least 10 bytes. + +Behaviour changes: +* A failed login (AUTHINFO PASS) is answered with 481 after 1 second the + first time in a connection, after 2 seconds the second time and after + 4 seconds the third time; then the connection is closed, as before. A + successful login is answered at once. The count is per connection, so + a client that connects again starts at 1 second. AuthFailureDelay + keeps its name: 0 turns the wait off, any other value (1 to 3600, as + in older configuration files) turns it on; the default is 1, on. Up to + 1.3.22 the wait was AuthFailureDelay times the number of failures (15, + 30 and 45 s by default). +* A login with an unknown user, a locked or empty password field, or a + plain-text password field (--enable-plainpass) no longer does a dummy + password hash to take as long as a wrong password, so bots guessing + logins cost no CPU time for it. The time of a 481 may now tell whether + a user name exists. + +New options: +* AuthMinLength bytes (default 10, 0 to 255, 0 turns it off): a user name + or password shorter than this is refused with 481 when AUTHINFO PASS + arrives, after the wait of AuthFailureDelay and without a password + check, a lookup or a PAM call. It counts as a failed login. AUTHINFO + USER still answers 381. The log says "failed authinfo for user ... + (user name shorter than AuthMinLength)" or "(password shorter than + AuthMinLength)", without the password. newscache -p prints it. diff --git a/debian/README.Debian b/debian/README.Debian index 9a957f79..6479963a 100644 --- a/debian/README.Debian +++ b/debian/README.Debian @@ -63,7 +63,8 @@ and put one line per user into /etc/newscache.users: user:password-hash:*:*:*:read,post -The fields after the hash are the groups the user may list, read and post +User names and passwords must have at least 10 bytes: shorter ones +are refused (AuthMinLength, default 10). The fields after the hash are the groups the user may list, read and post to, and the extra permissions (see newscache.conf(5)); only user and hash are required, and fields missing at the end add nothing. Create the hash with "openssl passwd -6" (or -1), and make the file readable for the group news only: diff --git a/doc/NewsCache.texi b/doc/NewsCache.texi index afd387ae..0a9fb6be 100644 --- a/doc/NewsCache.texi +++ b/doc/NewsCache.texi @@ -402,19 +402,39 @@ ClientTimeout 900 @end example @cdindex AuthFailureDelay -After a failed login (@code{AUTHINFO PASS} with a wrong user name or -password), NewsCache waits @code{AuthFailureDelay} seconds before it -answers 481; after the third failure in a connection it closes the -connection. The default is 1 second; 0 answers at once. Values up to 3600 -are accepted for older configuration files, but the delay is at most 1 -second: a longer one only keeps a process busy per connection. (Up to -1.3.22 the delay was multiplied by the number of failed logins: 15, 30 -and 45 seconds by default.) +With @code{AuthFailureDelay} set to any value from 1 to 3600 (the +default is 1), a failed login (@code{AUTHINFO PASS} with a wrong user +name or password) is answered with 481 after 1 second the first time in +a connection, after 2 seconds the second time and after 4 seconds the +third time; then NewsCache closes the connection. A successful login is +answered at once. The count is per connection: a client that connects +again starts at 1 second. 0 turns the wait off: every 481 comes at once, +and the connection is still closed after the third failure. Values above +1 are accepted for older configuration files and work like 1. (Up to +1.3.22 the delay was the value times the number of failed logins: 15, 30 +and 45 seconds by default.) NewsCache does no dummy password check for +an unknown user or a locked password field, so the time of a 481 may +tell whether a user name exists. @example AuthFailureDelay 0 @end example +@cdindex AuthMinLength +@code{AuthMinLength} is the shortest user name and password a login may +have, counted in bytes. When @code{AUTHINFO PASS} arrives, a user name +or password shorter than this is refused with 481 after the wait of +@code{AuthFailureDelay}, without a password check, a lookup or a PAM +call; it counts as a failed login. @code{AUTHINFO USER} is answered with +381 as before. The default is 10, the range 0 to 255; 0 turns the check +off. Administrators must use user names and passwords of at least 10 +bytes; set @code{AuthMinLength 0} only to keep shorter ones that +already exist. + +@example +AuthMinLength 10 +@end example + @cdindex MaxArticleSize @cdindex MaxPostSize NewsCache holds an article in memory as a whole. @code{MaxArticleSize} @@ -1144,7 +1164,8 @@ rights of its entry from the start. An entry with only @code{allow authentication} and one of these logins still lets nobody read or post without a login, as in the example below, but because it gives no rights: the commands that need one get @samp{480} until the login. As RFC 4643 says, a failed login (wrong password, unknown user, -a stored hash that can't be checked) gets 481, @code{AUTHINFO PASS} +a stored hash that can't be checked, a user name or password shorter +than @code{AuthMinLength}) gets 481, @code{AUTHINFO PASS} without @code{AUTHINFO USER} gets 482, and @code{AUTHINFO} after a successful login, or where the entry has @code{Authentication none}, gets 502. Before the login, commands that need a right the client doesn't diff --git a/docker/newscache.conf b/docker/newscache.conf index 39705826..7b2254af 100644 --- a/docker/newscache.conf +++ b/docker/newscache.conf @@ -29,7 +29,8 @@ AccessList { # Or, instead of the Default entry below, let in anyone who logs in. The # entry allows only the login; the rights come from the user file, one # user:hash:list:read:postto:allow line per user, e.g. - # alice:$6$...:*:*:*:read,post (hash from "openssl passwd -6"; mount that + # alice-reader:$6$...:*:*:*:read,post (hash from "openssl passwd -6"; user + # names and passwords need at least 10 bytes, AuthMinLength; mount that # file too, readable by the news user). "allow read post" here would make # the login optional: clients that don't log in could read and post. #Default { diff --git a/etc/newscache.auth-dist b/etc/newscache.auth-dist index 85b65999..13473bf5 100644 --- a/etc/newscache.auth-dist +++ b/etc/newscache.auth-dist @@ -1,12 +1,14 @@ # This are commentar lines # You can use plaintext passwords, if the newscache is configured with # the option --enable-plainpass -user1:pass1:at.linux:at.linux:at.linux:post -user2:pass2:de.linux:de.linux:de.linux:post -user3:xtKlYxMWGiBYg:de.linux:de.linux:de.linux:post +# User names and passwords need at least 10 bytes (AuthMinLength +# in newscache.conf); shorter ones are refused. +reader-one:password-one:at.linux:at.linux:at.linux:post +reader-two:password-two:de.linux:de.linux:de.linux:post +reader-three:xtKlYxMWGiBYg:de.linux:de.linux:de.linux:post # Only user and password are required; missing fields add nothing, so -# user4 gets just the rights of its Client entry after logging in. -user4:pass4 +# reader-four gets just the rights of its Client entry after logging in. +reader-four:password-four # The default build recognizes raw MD5 as base64(MD5(password)); this is # unsalted and weak. Prefer a salted crypt(3) hash supported by this system. # htpasswd defaults to APR1 ($apr1$), which NewsCache does not support; diff --git a/etc/newscache.conf-dist b/etc/newscache.conf-dist index 42ece232..ab4524ad 100644 --- a/etc/newscache.conf-dist +++ b/etc/newscache.conf-dist @@ -146,7 +146,9 @@ Admin root@localhost # with newscache -t the client is matched by its address. # Login example: anyone may connect, but must log in first. The # entry allows only the login itself; the rights come from the user -# file (user:hash:list:read:postto:allow, e.g. alice:$6$...:*:*:*:read,post). +# file (user:hash:list:read:postto:allow, e.g. +# alice-reader:$6$...:*:*:*:read,post; user names and passwords need +# at least 10 bytes, see AuthMinLength). # With "allow read post" instead, the login would be optional and # clients that don't log in could read and post too. # Default { @@ -366,10 +368,17 @@ NewsServerList { # stop reading their replies, or stop sending in the middle of a POST) ClientTimeout 600 -# After a failed login, wait this many seconds before answering 481 -# (0 or 1; default 1). The connection is closed after the third failure. +# Wait 1, 2 and 4 s before answering the first, second and third failed +# login in a connection with 481; then the connection is closed. A +# successful login doesn't wait. 0 turns the wait off (default 1; values +# up to 3600 from older configurations work like 1). # AuthFailureDelay 1 +# Refuse user names and passwords shorter than this many bytes (481, a +# failed login, no password check). Use at least 10 bytes for both; +# 0 turns the check off (default 10, range 0 to 255). +# AuthMinLength 10 + # Serve ARTICLE and the like from the spool when it has the # article, using a history in SpoolDirectory/.history (default off) # History on diff --git a/man/newscache.conf.5.in b/man/newscache.conf.5.in index 21568396..bbb2dfe9 100644 --- a/man/newscache.conf.5.in +++ b/man/newscache.conf.5.in @@ -147,7 +147,10 @@ The default values are 3600 seconds for both parameter. Terminate the connection after t seconds inactivity. The default value is 1200 seconds. The same limit applies to each write to the client and, plus 10 seconds, to each read inside a command (the article of a POST). The whole article of a POST must arrive within t seconds after the 340 reply, however steadily it is sent: a client that stops reading its replies, or sends an article too slowly, is disconnected too, with 400 for the article. .TP .I "AuthFailureDelay seconds" -After a failed login (AUTHINFO PASS), wait this many seconds before answering 481; after the third failure the connection is closed. The default is 1 second; 0 answers at once. Values up to 3600 are accepted for older configuration files, but the delay is at most 1 second: a longer one only keeps a process busy per connection. (Up to 1.3.22 the delay was multiplied by the number of failed logins: 15, 30 and 45 seconds by default.) +With any value from 1 to 3600 (the default is 1), a failed login (AUTHINFO PASS) is answered with 481 after 1 second the first time in a connection, after 2 seconds the second time and after 4 seconds the third time; then the connection is closed. A successful login is answered at once. The count is per connection: a client that connects again starts at 1 second. 0 turns the wait off: every 481 comes at once, and the connection is still closed after the third failure. Values above 1 are accepted for older configuration files and work like 1. (Up to 1.3.22 the delay was the value times the number of failed logins: 15, 30 and 45 seconds by default.) NewsCache does no dummy password check for an unknown user or a locked password field, so the time of a 481 may tell whether a user name exists. +.TP +.I "AuthMinLength bytes" +The shortest user name and password a login may have, counted in bytes. When AUTHINFO PASS arrives, a user name or password shorter than this is refused with 481 after the wait of AuthFailureDelay, without a password check, a lookup or a PAM call; it counts as a failed login. AUTHINFO USER is answered with 381 as before. The default is 10, the range 0 to 255; 0 turns the check off. Administrators must use user names and passwords of at least 10 bytes; set AuthMinLength 0 only to keep shorter ones that already exist. .TP .I "History on|off" With on (or yes), keep a history of the articles in the spool (the directory .history in SpoolDirectory) and serve ARTICLE, HEAD, BODY, STAT and the message-ID forms of XHDR, XPAT and HDR from the spool when it has the article; see the manual, "Articles by Message-ID". The history counts toward SpoolSize; newscacheclean removes it when it alone takes more than half of SpoolSize, and when History is off. The default is off: every message-ID request goes to the news server. @@ -391,7 +394,7 @@ Only .B unix without fields makes the login required: until then every command but CAPABILITIES, MODE READER, AUTHINFO, HELP and QUIT gets 480, whatever the entry allows. With file:, pam (also without fields), pam+file and unix with fields the login is optional: the client has the rights of its entry from the start, and a login adds those of the user. An entry that allows only authentication still lets nobody in without a login, but for another reason: it gives no rights, so the commands that need one get 480 until the login (those that need none, such as DATE, work). -A failed login (wrong password, unknown user, a stored hash that can't be checked) is answered with 481, AUTHINFO PASS without AUTHINFO USER with 482, and AUTHINFO after a successful login, or where the entry has Authentication none, with 502 (RFC 4643). Possible values are: +A failed login (wrong password, unknown user, a stored hash that can't be checked, a user name or password shorter than AuthMinLength) is answered with 481, AUTHINFO PASS without AUTHINFO USER with 482, and AUTHINFO after a successful login, or where the entry has Authentication none, with 502 (RFC 4643). Possible values are: .IP none no further authentication necessary .IP file:/path/to/file @@ -399,7 +402,7 @@ formatdescription: .nf username:password:list:read:postto:allow .fi -Only username and password are required. Fields missing at the end add nothing, like empty ones: a line \fBalice:hash\fR only checks the login, and the client keeps the rights of its Client entry. A password field that is empty or starts with * or ! locks the user: no password matches it. +Only username and password are required. Fields missing at the end add nothing, like empty ones: a line \fBalice-reader:hash\fR only checks the login, and the client keeps the rights of its Client entry. A password field that is empty or starts with * or ! locks the user: no password matches it. The additional access specifications are added to the normal List, Read and PostTo parameters (see above and the examples). In the default build, NewsCache recognizes a raw-MD5 password value as base64-encoded MD5, 24 characters ending in \fB==\fR. This legacy format is unsalted and weak; do not use it for new passwords. Otherwise, NewsCache passes the stored value to the system's \fBcrypt(3)\fR; the supported algorithms vary by platform. Apache \fBhtpasswd\fR defaults to APR1 hashes (\fB$apr1$\fR), which are not supported by the system \fBcrypt(3)\fR implementations used with NewsCache. SHA-512 crypt hashes (\fB$6$\fR) work only where the system's \fBcrypt(3)\fR supports them; generate one with \fBopenssl passwd -6\fR or \fBmkpasswd -m sha-512\fR where available. With \fB--without-md5auto\fR, all stored values are checked as MD5, so \fBcrypt(3)\fR hashes cannot be used. Plaintext passwords are accepted only if NewsCache was built with \fB--enable-plainpass\fR. In the allow field, additional permissions can be specified (for example, allowing an authenticated user to post). .IP unix:list:read:postto:allow diff --git a/src/NewsCache.cc b/src/NewsCache.cc index a186b9cf..3950daca 100644 --- a/src/NewsCache.cc +++ b/src/NewsCache.cc @@ -641,12 +641,13 @@ class ClientData { bool tls; // the connection is TLS (TLSListenTo, -t) char auth_user[512]; bool auth_user_valid; // a USER is pending for one PASS - bool auth_refused; // the last AUTHINFO checked a password and refused it + bool auth_refused; // the last AUTHINFO PASS was refused (a failure) + const char *auth_refused_why; // why, if no password was checked, else NULL char auth_pass[512]; int auth_state; int auth_failures; int auth_max_failures; - int auth_failure_sleep_time; + bool auth_failure_delay; // wait 1, 2, 4 s after failed logins #ifdef PAM_AUTH pam_handle_t *pamh; @@ -664,9 +665,11 @@ class ClientData { grp(NULL), nbr(0), access_entry(NULL), tls(false), auth_user_valid(false), auth_refused(false), + auth_refused_why(NULL), auth_state(AUTH_OK), auth_failures(0), - auth_max_failures(3), auth_failure_sleep_time(Cfg.AuthFailureDelay), + auth_max_failures(3), + auth_failure_delay(Cfg.AuthFailureDelay != 0), stat_groups(0), stat_artingrp(0), stat_articles(0) { auth_user[0] = auth_pass[0] = '\0'; @@ -957,74 +960,6 @@ int auto_cryptcheck(const char *key, const char *pass) return r; } -/* - * Does the work of checking a password against a stored hash and throws - * the result away. A login for a user that does not exist (or whose - * password field is locked) returned at once, while one with a wrong - * password first computed a hash, so a remote client could tell existing - * user names by the time of the reply. Calling this for the user that - * isn't there makes both take about as long. hash is the setting to hash - * with: for "Authentication file" a hash from the same file (so that the - * cost is the one the administrator chose), else a fixed $6$ hash. - */ -static void dummy_password_check(const char *key, const char *hash) -{ - static const char fixed[] = "$6$ncdummysalt$kRpQqp7lXkZrlvI2YTvvoD8gQyPz4FcoZGD0UUmeCjxyAHweanwg9PP.vyX39TuZJREeQYHSxL/GTjyoHuXzM/"; - - if (hash == NULL || hash[0] == '\0') - hash = fixed; - bool md5 = false; -#ifdef MD5_CRYPT -#ifdef MD5_AUTO - md5 = strlen(hash) == 24 && hash[23] == '=' && hash[22] == '='; -#else - md5 = true; -#endif -#endif - if (md5) { -#ifdef MD5_CRYPT - free(md5crypt(key)); -#endif - } else { - (void) crypt(key, hash); - } -} - -/* - * A hash that looks usable from the first line of the auth file that has - * one (for dummy_password_check), or "" if there is none. - */ -static std::string auth_file_sample_hash(const char *filename) -{ - std::string res; - FILE *F = fopen(filename, "r"); - if (F == NULL) - return res; - char *buf = NULL; - size_t bufsize = 0; - while (getline(&buf, &bufsize, F) != -1) { - char *c1 = strchr(buf, ':'); - if (buf[0] == '#' || c1 == NULL) - continue; - char *h = c1 + 1; - char *c2 = strchr(h, ':'); - size_t n = c2 ? (size_t) (c2 - h) : strlen(h); - while (n > 0 && isspace((unsigned char) h[n - 1])) - n--; - if (n > 3 && h[0] == '$') { - res.assign(h, n); - break; - } - if (n == 24 && h[22] == '=' && h[23] == '=') { - res.assign(h, n); - break; - } - } - free(buf); - fclose(F); - return res; -} - #ifdef PLAIN_TEXT_PASSWORDS /* * Whether an auth-file password field may be compared as plain text: not @@ -1065,10 +1000,8 @@ int check_auth_unix (ClientData *clt) struct passwd *pw = NULL; pw = getpwnam(clt->auth_user); if (!pw || !(pw->pw_name) || !(pw->pw_passwd) - || !(pw->pw_passwd[0])) { - dummy_password_check(clt->auth_pass, NULL); + || !(pw->pw_passwd[0])) return -1; - } pass = pw->pw_passwd; if ((pass[0] == 'x') && (pass[1] == '\0')) { #ifdef SHADOW_PASSWORD @@ -1147,38 +1080,25 @@ int check_auth_file (ClientData *clt) int authc = 0; int passf=-1; - if (!authv) { - // no such user: same work as for a wrong password - dummy_password_check(clt->auth_pass, - auth_file_sample_hash(authfile.c_str()).c_str()); + if (!authv) return -1; - } // user:hash is enough; missing list, read, postto and allow fields // add nothing, like empty ones. while (authv[authc]) authc++; - // no password, or a locked or empty field, never matches (and takes - // as long as a wrong password, like an unknown user) + // no password, or a locked or empty field, never matches if (authc < 2 || authv[1][0] == '\0' || authv[1][0] == '*' - || authv[1][0] == '!') { - dummy_password_check(clt->auth_pass, - auth_file_sample_hash(authfile.c_str()).c_str()); + || authv[1][0] == '!') return -1; - } #ifdef PLAIN_TEXT_PASSWORDS // A field that is no hash is compared as plain text, without // crypt(): it can't use the field and logged an error for every - // login. It still does the work of a hash check (which logs - // nothing), so that a user with a plain field doesn't answer - // faster than an unknown one. A hash is never compared as plain - // text (the plain comparison came first and accepted a hash as its - // own password). - if (plain_password_field(authv[1])) { - dummy_password_check(clt->auth_pass, - auth_file_sample_hash(authfile.c_str()).c_str()); + // login. A hash is never compared as plain text (the plain + // comparison came first and accepted a hash as its own password). + if (plain_password_field(authv[1])) passf = hashes_differ(clt->auth_pass, authv[1]); - } else + else #endif passf = auto_cryptcheck(clt->auth_pass, authv[1]); if (passf) @@ -1722,6 +1642,7 @@ static void clear_password(char *p) int ns_authinfo(ClientData * clt, int argc, char *argv[]) { clt->auth_refused = false; + clt->auth_refused_why = NULL; // No AUTHINFO after a login (RFC 4643 2.2: 502); a second login // added that user's rights to the first one's. Entries without an // Authentication start out in AUTH_OK: no login for them either. @@ -1779,7 +1700,21 @@ int ns_authinfo(ClientData * clt, int argc, char *argv[]) } strcpy(clt->auth_pass, argv[2]); clear_password(argv[2]); - int checked = check_authentication(clt); + // AuthMinLength: a user name or password that is too short is + // refused without a lookup, password check or PAM call, and + // counts as a failure. (Only here, not at USER, so that USER + // tells nothing; bytes, not characters.) + int checked; + if (strlen(clt->auth_user) < (size_t) Cfg.AuthMinLength) { + clt->auth_refused_why = + "user name shorter than AuthMinLength"; + checked = -1; + } else if (strlen(clt->auth_pass) < (size_t) Cfg.AuthMinLength) { + clt->auth_refused_why = + "password shorter than AuthMinLength"; + checked = -1; + } else + checked = check_authentication(clt); clear_password(clt->auth_pass); // Keep the name for the failed-login audit log, but require another // USER before a subsequent PASS (RFC 4643 2.3.2). @@ -1787,11 +1722,17 @@ int ns_authinfo(ClientData * clt, int argc, char *argv[]) if (checked < 0) { clt->auth_refused = true; - // the same short delay after each failure; it grew with - // the failures (15, 30, 45 s by default), which only held - // the session's process and memory + // 1, 2 and 4 s after the first, second and third failure + // in this session (then it is closed); a successful login + // doesn't wait. Per session only: a client that connects + // again starts at 1 s. ++clt->auth_failures; - sleep(clt->auth_failure_sleep_time); + if (clt->auth_failure_delay) { + int n = clt->auth_failures < 1 ? 1 + : clt->auth_failures > 3 ? 3 + : clt->auth_failures; + sleep(1u << (n - 1)); + } (*clt-> co) << FAILED_SEQUENCE << " Bad authinfo.\r\n"; return -1; @@ -4461,11 +4402,14 @@ void nnrpd(int fd, bool tls) client_logname << " failed " << oreq << "\n"; else if (clt.auth_refused) - // (the user name only when its password was - // checked: not for 482, 501, 502 or too long) + // (the user name only for a refused login: not + // for 482, 501, 502 or too long) slog.p(Logger::Notice) << clt. client_logname << " failed authinfo for user " << - clt.auth_user << "\n"; + clt.auth_user << + (clt.auth_refused_why ? " (" : "") << + (clt.auth_refused_why ? clt.auth_refused_why : "") << + (clt.auth_refused_why ? ")" : "") << "\n"; else slog.p(Logger::Notice) << clt. client_logname << " failed authinfo\n"; diff --git a/src/NewsCacheConfig.cc b/src/NewsCacheConfig.cc index 2604f9a5..87a61e72 100644 --- a/src/NewsCacheConfig.cc +++ b/src/NewsCacheConfig.cc @@ -71,6 +71,7 @@ void Config::init(void) ClientTimeout = 1200; AuthFailureDelay = 1; + AuthMinLength = 10; HistoryEnabled = 0; MaxConnections = 32; MaxConnectionsPerHost = 0; @@ -223,10 +224,24 @@ void Config::read(const char *fn, bool lookupHostname) throw SyntaxError(lex, "AuthFailureDelay expects seconds, 0 to 3600", ERROR_LOCATION); - // at most 1 s: a longer (and, before, growing) sleep - // only kept a process and its memory busy per guessing - // connection; values up to 3600 are still accepted + // 0 turns the wait after failed logins off, any other + // value on: 1, 2 and 4 s after the first, second and + // third failure in a session. Values up to 3600 (seconds + // times the failures in 1.3.22) are still accepted. AuthFailureDelay = v > 1 ? 1 : (int) v; + } else if (tok == "AuthMinLength") { + char *e; + long v; + + a1 = lex.getToken(); + errno = 0; + v = strtol(a1.c_str(), &e, 10); + if (!isdigit((unsigned char) a1[0]) || *e || errno + || v > 255) + throw SyntaxError(lex, + "AuthMinLength expects bytes, 0 to 255", + ERROR_LOCATION); + AuthMinLength = (int) v; } else if (tok == "MaxArticleSize" || tok == "MaxPostSize") { char *e; long v; @@ -346,6 +361,7 @@ void Config::printParameters (ostream *pOut) } *pOut << "ClientTimeout " << ClientTimeout << endl; *pOut << "AuthFailureDelay " << AuthFailureDelay << endl; + *pOut << "AuthMinLength " << AuthMinLength << endl; *pOut << "History " << (HistoryEnabled ? "on" : "off") << endl; *pOut << "MaxConnections " << MaxConnections << endl; *pOut << "MaxConnectionsPerHost " << MaxConnectionsPerHost << endl; diff --git a/src/NewsCacheConfig.h b/src/NewsCacheConfig.h index 894a9841..7d8e5f08 100644 --- a/src/NewsCacheConfig.h +++ b/src/NewsCacheConfig.h @@ -54,7 +54,8 @@ class Config { char PidFile[MAXPATHLEN]; int ClientTimeout; - int AuthFailureDelay; // seconds after each failed login, 0 or 1 + int AuthFailureDelay; // 0: no wait after failed logins, 1: 1, 2, 4 s + int AuthMinLength; // bytes: shortest user name and password, 0: off int HistoryEnabled; // History on: the message-ID history in the spool int MaxConnections; int MaxConnectionsPerHost; // 0: no limit diff --git a/tests/e2e/auth_test.go b/tests/e2e/auth_test.go index e48b1f8b..d48b24af 100644 --- a/tests/e2e/auth_test.go +++ b/tests/e2e/auth_test.go @@ -1,7 +1,9 @@ package e2e import ( + "crypto/md5" "debug/elf" + "encoding/base64" "fmt" "os" "os/exec" @@ -66,55 +68,6 @@ func authEnv(t *testing.T, rights string, lines ...string) *env { return e } -// TestAuthUnknownUserTiming: an unknown user was refused at once, while a -// wrong password first computed the hash, so the time of the reply told -// whether a user name exists. With a costly hash in the auth file (SHA-512, -// 600000 rounds, some 0.3 s) the unknown user's reply must take about as -// long: at least half of the wrong password's, a generous bound (the -// unfixed code needs a few milliseconds). The same for a locked line. -func TestAuthUnknownUserTiming(t *testing.T) { - if runtime.GOOS == "darwin" { - t.Skip("macOS crypt() has no SHA-512 ($6$)") - } - const slow = "$6$rounds=600000$ncslowsalt$UzZarigsENPa9S.5Y2jR3OczOpyLqiOROQovOdMlqMP5Z..Q8R/wyv.AsdE/gb1JHc3.wDy2PFQrL6c0EPKvA." - e := authFileEnv(t, "tester:"+slow+":*:*:*:", "locked:*") - e.Extra = "AuthFailureDelay 0" - // Interleave the three cases so a temporary change in host load does not - // affect every sample of just one user. Keep each user's shortest of - // three, as before, to limit scheduling delays in the comparison. - users := []string{"tester", "nobody-here", "locked"} - best := make(map[string]time.Duration, len(users)) - for _, user := range users { - best[user] = time.Duration(1 << 62) - } - for i := 0; i < 3; i++ { - for _, user := range users { - c := e.session() - if code, line := c.Cmd("AUTHINFO USER %s", user); code != 381 { - t.Fatalf("AUTHINFO USER %s: %q, want 381", user, line) - } - start := time.Now() - code, line := c.Cmd("AUTHINFO PASS wrong") - d := time.Since(start) - if code != 481 { - t.Fatalf("AUTHINFO PASS for %s: %q, want 481", user, line) - } - if d < best[user] { - best[user] = d - } - } - } - wrong := best["tester"] - if wrong < 50*time.Millisecond { - t.Skipf("the costly hash took only %v here; can't measure", wrong) - } - for _, u := range []string{"nobody-here", "locked"} { - if d := best[u]; d < wrong/2 { - t.Errorf("reply for %s took %v, a wrong password for an existing user %v: the time tells whether the user exists", u, d, wrong) - } - } -} - // TestAuthFileShortLine: an auth-file line with only user, password and the // List field is valid. The Read, PostTo and flags fields are missing, and // reading them must not run past the end of the field list. @@ -471,28 +424,203 @@ func TestAuthFileBadHash(t *testing.T) { } } -// TestAuthFailureDelay: after each failed login NewsCache waits -// AuthFailureDelay seconds (at most 1) before it answers; a later correct -// password still logs in. The delay was a fixed 15 seconds, then (1.3.22) -// AuthFailureDelay times the number of failures. -func TestAuthFailureDelay(t *testing.T) { +// failLogin sends AUTHINFO USER user (381 expected) and AUTHINFO PASS pass, +// expects 481 and returns how long the PASS took. +func failLogin(t *testing.T, c *client.Conn, user, pass string) time.Duration { + t.Helper() + if code, line := c.Cmd("AUTHINFO USER %s", user); code != 381 { + t.Fatalf("AUTHINFO USER %s: %q, want 381", user, line) + } + start := time.Now() + code, line := c.Cmd("AUTHINFO PASS %s", pass) + d := time.Since(start) + if code != 481 { + t.Fatalf("AUTHINFO PASS for %s: %q, want 481", user, line) + } + return d +} + +// expectClosed reads the 400 after the third failed login and checks that +// the connection is closed then. +func expectClosed(t *testing.T, c *client.Conn) { + t.Helper() + code, line, err := c.ReadStatus() + if err != nil || code != 400 { + t.Fatalf("after the third failure: %q, %v; want 400", line, err) + } + if l, err := c.ReadLine(); err == nil { + t.Fatalf("after the 400: %q, want the connection closed", l) + } +} + +// checkBackoff fails if the n-th failed login (from 1) didn't wait about +// 1, 2 or 4 s: at least that minus a little, at most 3 s more. +func checkBackoff(t *testing.T, n int, d time.Duration) { + t.Helper() + want := time.Second << (n - 1) + if d < want-100*time.Millisecond || d > want+3*time.Second { + t.Errorf("failure %d: answered after %v, want about %v", n, d.Round(time.Millisecond), want) + } +} + +// TestAuthFailureBackoff: with the wait on (AuthFailureDelay 1, the +// default, or a larger value from an older configuration) the first, +// second and third failed login of a session are answered after 1, 2 and +// 4 s, and then the connection is closed. Every failure waited the same +// 1 s before; in 1.3.22 the wait was AuthFailureDelay times the failures. +func TestAuthFailureBackoff(t *testing.T) { + for _, extra := range []string{"", "AuthFailureDelay 1", "AuthFailureDelay 15"} { + name := extra + if name == "" { + name = "default" + } + t.Run(name, func(t *testing.T) { + t.Parallel() + e := authFileEnv(t, "tester:"+md5Hash+":*:*:*:") + e.Extra = extra + c := e.session() + c.Deadline = 20 * time.Second + for n := 1; n <= 3; n++ { + checkBackoff(t, n, failLogin(t, c, "tester", "wrong")) + } + expectClosed(t, c) + }) + } +} + +// TestAuthSuccessNoDelay: with the wait on, a successful login is answered +// at once, also after a failed one in the same session. +func TestAuthSuccessNoDelay(t *testing.T) { t.Parallel() e := authFileEnv(t, "tester:"+md5Hash+":*:*:*:") - e.Extra = "AuthFailureDelay 1" - c := e.session() - c.Deadline = 20 * time.Second - for n := 1; n <= 2; n++ { - c.Cmd("AUTHINFO USER tester") + timedLogin := func(c *client.Conn, what string) { + t.Helper() + if code, line := c.Cmd("AUTHINFO USER tester"); code != 381 { + t.Fatalf("AUTHINFO USER: %q, want 381", line) + } start := time.Now() - if code, line := c.Cmd("AUTHINFO PASS wrong"); code != 481 { - t.Fatalf("failure %d: AUTHINFO PASS wrong: %q, want 481", n, line) + if code, line := c.Cmd("AUTHINFO PASS secret"); code != 281 { + t.Fatalf("AUTHINFO PASS: %q, want 281", line) } - want := time.Second - if d := time.Since(start); d < want || d > want+2*time.Second { - t.Errorf("failure %d: answered after %v, want about %v", n, d.Round(time.Millisecond), want) + if d := time.Since(start); d > 500*time.Millisecond { + t.Errorf("successful login %s answered after %v, want no wait", what, d.Round(time.Millisecond)) } } - login(t, c) + timedLogin(e.session(), "as the first command") + c := e.session() + c.Deadline = 20 * time.Second + checkBackoff(t, 1, failLogin(t, c, "tester", "wrong")) + timedLogin(c, "after a failure") +} + +// TestAuthFailureNoDelayOff: AuthFailureDelay 0 turns the wait off: three +// failed logins are answered at once, and the connection is still closed +// after the third. +func TestAuthFailureNoDelayOff(t *testing.T) { + e := authFileEnv(t, "tester:"+md5Hash+":*:*:*:") + e.Extra = "AuthFailureDelay 0" + c := e.session() + c.Deadline = 10 * time.Second + for n := 1; n <= 3; n++ { + if d := failLogin(t, c, "tester", "wrong"); d > 500*time.Millisecond { + t.Errorf("failure %d with AuthFailureDelay 0: answered after %v, want no wait", n, d.Round(time.Millisecond)) + } + } + expectClosed(t, c) +} + +// ncMD5 is NewsCache's MD5 auth-file hash of pass: the MD5 digest in +// base64, as md5Hash is for "secret". +func ncMD5(pass string) string { + sum := md5.Sum([]byte(pass)) + return base64.StdEncoding.EncodeToString(sum[:]) +} + +// TestAuthMinLength: a user name or password shorter than AuthMinLength +// (default 10 bytes) is refused with 481 when AUTHINFO PASS arrives, +// without checking it: the right password doesn't help. It counts as a +// failure, with the wait (1, 2, 4 s) and the close after the third. USER +// still answers 381. A 10-byte name and password log in; AuthMinLength 0 +// accepts short ones. +func TestAuthMinLength(t *testing.T) { + const ( + user10 = "tester0123" + pass10 = "secret0123" + user9 = "tester012" + pass9 = "secret012" + ) + lines := []string{ + user10 + ":" + ncMD5(pass10) + ":*:*:*:", + user9 + ":" + ncMD5(pass10) + ":*:*:*:", + "shortpass1:" + ncMD5(pass9) + ":*:*:*:", + } + t.Run("10 bytes log in with the default", func(t *testing.T) { + e := authFileEnv(t, lines...) + e.AuthMinLength = "" + c := e.session() + if code, line := c.Cmd("AUTHINFO USER %s", user10); code != 381 { + t.Fatalf("AUTHINFO USER: %q, want 381", line) + } + if code, line := c.Cmd("AUTHINFO PASS %s", pass10); code != 281 { + t.Fatalf("AUTHINFO PASS: %q, want 281", line) + } + }) + t.Run("shorter refused with the default", func(t *testing.T) { + t.Parallel() + e := authFileEnv(t, lines...) + e.AuthMinLength = "" + p, c := e.sessionProcess() + c.Deadline = 20 * time.Second + // the right passwords: refused without a check + checkBackoff(t, 1, failLogin(t, c, user9, pass10)) + checkBackoff(t, 2, failLogin(t, c, "shortpass1", pass9)) + checkBackoff(t, 3, failLogin(t, c, user9, pass10)) + expectClosed(t, c) + if err := p.Wait(); err != nil { + t.Fatalf("session exit: %v", err) + } + log := e.logFile() + for _, want := range []string{ + "failed authinfo for user " + user9 + " (user name shorter than AuthMinLength)", + "failed authinfo for user shortpass1 (password shorter than AuthMinLength)", + } { + if !strings.Contains(log, want) { + t.Errorf("the log lacks %q:\n%s", want, log) + } + } + for _, pass := range []string{pass9, pass10} { + if strings.Contains(log, pass) { + t.Errorf("the log has the password %q:\n%s", pass, log) + } + } + }) + t.Run("AuthMinLength 10 counts bytes", func(t *testing.T) { + // "t\u00e9ster012" is 10 bytes but 9 characters + u := "t\u00e9ster012" + e := authFileEnv(t, u+":"+ncMD5(pass10)+":*:*:*:") + e.AuthMinLength = "10" + e.Extra = "AuthFailureDelay 0" + c := e.session() + if code, line := c.Cmd("AUTHINFO USER %s", u); code != 381 { + t.Fatalf("AUTHINFO USER: %q, want 381", line) + } + if code, line := c.Cmd("AUTHINFO PASS %s", pass10); code != 281 { + t.Fatalf("AUTHINFO PASS: %q, want 281", line) + } + }) + t.Run("AuthMinLength 0", func(t *testing.T) { + for _, up := range [][2]string{{user9, pass10}, {"shortpass1", pass9}} { + e := authFileEnv(t, lines...) + e.AuthMinLength = "0" + c := e.session() + if code, line := c.Cmd("AUTHINFO USER %s", up[0]); code != 381 { + t.Fatalf("AUTHINFO USER: %q, want 381", line) + } + if code, line := c.Cmd("AUTHINFO PASS %s", up[1]); code != 281 { + t.Fatalf("AUTHINFO PASS for %s with AuthMinLength 0: %q, want 281", up[0], line) + } + } + }) } // TestLoginOnlyEntry: an entry with "allow authentication" and nothing @@ -1363,29 +1491,6 @@ func TestLogControlBytes(t *testing.T) { } } -// TestAuthFailureDelayFixed: each failed login waits the same short delay, -// at most 1 s, also with an AuthFailureDelay from an older configuration. -// The delay was AuthFailureDelay times the number of failures (15, 30 and -// 45 s by default, up to 3 h with 3600), which only held the session's -// process and memory for each guessing connection. -func TestAuthFailureDelayFixed(t *testing.T) { - e := authFileEnv(t, "tester:"+md5Hash+":*:*:*:") - e.Extra = "AuthFailureDelay 15" - c := e.session() - for i := 1; i <= 2; i++ { - if code, line := c.Cmd("AUTHINFO USER tester"); code != 381 { - t.Fatalf("AUTHINFO USER: %q, want 381", line) - } - start := time.Now() - if code, line := c.Cmd("AUTHINFO PASS wrong"); code != 481 { - t.Fatalf("AUTHINFO PASS (wrong) #%d: %q, want 481", i, line) - } - if d := time.Since(start); d < 900*time.Millisecond || d > 3*time.Second { - t.Errorf("failed login #%d answered after %v, want about 1 s", i, d.Round(10*time.Millisecond)) - } - } -} - // TestAuthMaxFailures: after the third failed login the connection is // closed (400), with every LogStyle. With "LogStyle strict-inn" the check // was skipped (it sat in the block that writes NewsCache's own log lines), diff --git a/tests/e2e/config_test.go b/tests/e2e/config_test.go index f1fcd56b..565165ee 100644 --- a/tests/e2e/config_test.go +++ b/tests/e2e/config_test.go @@ -658,7 +658,8 @@ func TestAccessPrefixInvalid(t *testing.T) { } // TestAuthFailureDelayConfig: AuthFailureDelay accepts 0 to 3600 seconds; -// values above 1 are capped at 1, the default printed by newscache -p. +// 0 turns the wait after failed logins off, any other value on (1, 2, 4 s), +// printed as 1 by newscache -p, the default. func TestAuthFailureDelayConfig(t *testing.T) { for _, c := range []struct { extra, want string @@ -697,6 +698,48 @@ func TestAuthFailureDelayConfig(t *testing.T) { } } +// TestAuthMinLengthConfig: AuthMinLength accepts 0 to 255 bytes, default +// 10, and newscache -p prints it (next to AuthFailureDelay). +func TestAuthMinLengthConfig(t *testing.T) { + for _, c := range []struct { + value string // "" = no AuthMinLength line + want string // "" = a config error + }{ + {"", "AuthMinLength 10"}, + {"0", "AuthMinLength 0"}, + {"10", "AuthMinLength 10"}, + {"255", "AuthMinLength 255"}, + {"256", ""}, + {"-1", ""}, + {"ten", ""}, + {"10b", ""}, + } { + t.Run(c.value, func(t *testing.T) { + e := newEnv(t) + e.AuthMinLength = c.value + e.writeConf() + out, err := exec.Command(e.Bin, "-c", e.Conf, "-p").CombinedOutput() + if c.want == "" { + if err == nil { + t.Fatalf("newscache -p accepted AuthMinLength %s:\n%s", c.value, out) + } + if !strings.Contains(string(out), "AuthMinLength expects bytes, 0 to 255") { + t.Errorf("AuthMinLength %s: error %q", c.value, strings.TrimSpace(string(out))) + } + return + } + if err != nil { + t.Fatalf("newscache -p: %v\n%s", err, out) + } + for _, want := range []string{c.want, "AuthFailureDelay 1"} { + if !strings.Contains("\n"+string(out), "\n"+want+"\n") { + t.Errorf("newscache -p output lacks %q:\n%s", want, out) + } + } + }) + } +} + // TestIntegerOptionsConfig: the integer options were read with atoi, // unchecked. "MaxConnections ten" became 0 and turned every client away, // "ClientTimeout 20m" became 20 seconds, "Retries -1" became 4294967295 diff --git a/tests/e2e/harness_test.go b/tests/e2e/harness_test.go index 4e95a669..59caad86 100644 --- a/tests/e2e/harness_test.go +++ b/tests/e2e/harness_test.go @@ -66,6 +66,7 @@ type env struct { // Settings written into newscache.conf; change before Start*. MaxConnections int ClientTimeout int + AuthMinLength string // AuthMinLength value, "" = no line (the default, 10) ServerOptions string // Options line of the upstream server, "" = default ServerHost string // upstream host for NewsCache, "" = the fake upstream ServerPort string // upstream port, "" = the fake upstream's @@ -93,6 +94,7 @@ func newEnv(t *testing.T) *env { Store: fakeserver.NewStore(), MaxConnections: 8, ClientTimeout: 60, + AuthMinLength: "0", // logins such as tester/secret are shorter than 10 GroupTimeout: 0, AccessList: DefaultAccessList, } @@ -255,6 +257,8 @@ var confTmpl = template.Must(template.New("conf").Parse(`# generated by tests/e2 {{end -}} {{if not (index .ExtraKeys "maxconnections")}}MaxConnections {{.MaxConnections}} {{end -}} +{{if and .AuthMinLength (not (index .ExtraKeys "authminlength"))}}AuthMinLength {{.AuthMinLength}} +{{end -}} {{.AccessList}} NewsServerList { Server {{.UpHost}} {{.UpPort}} { @@ -348,6 +352,7 @@ func (e *env) writeConf() { "LogDir": e.LogDir, "ClientTimeout": e.ClientTimeout, "MaxConnections": e.MaxConnections, + "AuthMinLength": e.AuthMinLength, "UpHost": firstNonEmpty(e.ServerHost, e.Upstream.Host()), "UpPort": firstNonEmpty(e.ServerPort, fmt.Sprint(e.Upstream.Port())), "ServerLines": e.ServerLines,