Skip to content

Commit

Permalink
mod_mail: Log remote IMAP server error when LOGIN fails.
Browse files Browse the repository at this point in the history
Fix an off-nominal path to display the error sent by
a remote IMAP server (for IMAP client proxy) when
the IMAP LOGIN command fails. Previously, we were using
IMAP_CLIENT_EXPECT, which would be expected to fail and
thus goto the end of the function, rather than logging
the error first, making this a confusing error to follow.

This bug was initially discovered due to logins to Microsoft
accounts failing intermittently. This was initially presumed
to be a bug in capability parsing, but is actually (even by
their own admission) a bug in Microsoft's email servers:

"Users [using] Basic Authentication may experience recurring
password prompts... this is a known issue."

No workaround will be implemented to accomodate this since
this is a substantial (and, frankly, inexcusable for anything
calling itself a mail server) bug in Microsoft's software,
which should NOT be handled by other applications.

Additionally, Microsoft is disabling basic authentication in
three days (September 16, 2024), so this will be moot
very shortly. In any case, the LOGIN error sent by the server
is now logged properly to alert users to this happening.

LBBS-62 #close
  • Loading branch information
InterLinked1 committed Sep 14, 2024
1 parent d865645 commit 9a1918b
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions modules/mod_mail.c
Original file line number Diff line number Diff line change
Expand Up @@ -1687,7 +1687,7 @@ int maildir_parse_uid_from_filename(const char *filename, unsigned int *uid)
return 0;
}

static int imap_client_capability(struct bbs_tcp_client *client, int *capsptr)
static int imap_client_parse_capabilities(struct bbs_tcp_client *client, int *capsptr)
{
char *cur, *capstring;
int caps = 0;
Expand Down Expand Up @@ -1812,7 +1812,7 @@ int imap_client_login(struct bbs_tcp_client *client, struct bbs_url *url, struct
}
if (STARTS_WITH(client->buf, "* CAPABILITY")) {
ok_had_caps = 1; /* Don't need to parse again until authenticated */
if (imap_client_capability(client, capsptr)) { /* Parse unauthenticated capabilities */
if (imap_client_parse_capabilities(client, capsptr)) { /* Parse unauthenticated capabilities */
return -1;
}
}
Expand All @@ -1831,7 +1831,7 @@ int imap_client_login(struct bbs_tcp_client *client, struct bbs_url *url, struct
IMAP_CLIENT_SEND(client, "a0 CAPABILITY");
IMAP_CLIENT_EXPECT(client, "* CAPABILITY ");
}
if (imap_client_capability(client, capsptr)) { /* Parse unauthenticated capabilities */
if (imap_client_parse_capabilities(client, capsptr)) { /* Parse unauthenticated capabilities */
return -1;
}
if (!ok_had_caps) {
Expand Down Expand Up @@ -1890,7 +1890,7 @@ int imap_client_login(struct bbs_tcp_client *client, struct bbs_url *url, struct
return -1;
}
if (STARTS_WITH(client->buf, "* CAPABILITY")) {
if (imap_client_capability(client, capsptr)) {
if (imap_client_parse_capabilities(client, capsptr)) {
return -1;
}
IMAP_CLIENT_EXPECT(client, "a1 OK");
Expand All @@ -1900,7 +1900,8 @@ int imap_client_login(struct bbs_tcp_client *client, struct bbs_url *url, struct
/* It failed, send an empty response to get the error message */
IMAP_CLIENT_SEND(client, "");
}
IMAP_CLIENT_EXPECT(client, "a1 OK"); /* Won't get it, but at least see what the server had to say */
/* Won't get it, but at least see what the server had to say */
bbs_tcp_client_expect(client, "\r\n", 1, 2000, "a1 OK"); /* Don't use IMAP_CLIENT_EXPECT, or we'll bypass the warning below when it fails */
bbs_warning("Login failed, got '%s'\n", client->buf);
return -1;
}
Expand All @@ -1910,7 +1911,7 @@ int imap_client_login(struct bbs_tcp_client *client, struct bbs_url *url, struct
IMAP_CLIENT_SEND(client, "a2 CAPABILITY");
IMAP_CLIENT_EXPECT(client, "* CAPABILITY ");
}
if (imap_client_capability(client, capsptr)) { /* Parse authenticated capabilities */
if (imap_client_parse_capabilities(client, capsptr)) { /* Parse authenticated capabilities */
return -1;
}
if (!ok_had_caps) {
Expand Down

0 comments on commit 9a1918b

Please sign in to comment.