Skip to content

Commit

Permalink
mail, node: Improve handling of various edge cases.
Browse files Browse the repository at this point in the history
* modules: Don't attempt STARTTLS if io_tls isn't loaded, since it will fail.
* net_imap: Emit debug instead of warning log if cached UIDVALIDITY is empty.
* pty.c, socket.c: Adjust debug and warning log levels.
  • Loading branch information
InterLinked1 committed Dec 22, 2024
1 parent 99e3bf5 commit bcbb2b1
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 7 deletions.
2 changes: 1 addition & 1 deletion bbs/pty.c
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ void *pty_master(void *varg)
* if it's running in raw mode and not exiting gracefully, but terminals don't
* account for that sort of thing and neither should we.
*/
bbs_debug(3, "Received ^%c, forwarding it to child process\n", sigchar);
bbs_debug(10, "Received ^%c, forwarding it to child process\n", sigchar);
} else {
/* Now, if we're not executing a child process, things are very different.
* In this case, it actually makes sense to do some processing of these signals ourselves, too,
Expand Down
4 changes: 2 additions & 2 deletions bbs/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -1565,10 +1565,10 @@ int bbs_multi_poll(struct pollfd pfds[], int numfds, int ms)
return __bbs_multi_poll(NULL, pfds, numfds, ms);
}

/* This is not an assertion, since it can legitimately happen sometimes and that would be overkill. */
/* This is not an assertion, or even a warning, since it can legitimately happen right after a node disconnects and that would be overkill. */
#define REQUIRE_SLAVE_FD(node) \
if (node->slavefd == -1) { \
bbs_warning("Node %d has no active slave fd\n", node->id); \
bbs_debug(1, "Node %d has no active slave fd\n", node->id); \
return -1; \
}

Expand Down
4 changes: 4 additions & 0 deletions modules/mod_smtp_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ int bbs_smtp_client_starttls(struct bbs_smtp_client *restrict smtpclient)
bbs_error("Can't do STARTTLS, connection is already secure\n");
return -1;
}
if (!ssl_available()) {
bbs_error("Can't do STARTTLS, TLS module is not loaded\n");
return -1;
}
if (smtpclient->caps & SMTP_CAPABILITY_STARTTLS) {
bbs_smtp_client_send(smtpclient, "STARTTLS\r\n");
SMTP_CLIENT_EXPECT_FINAL(smtpclient, 2500, "220");
Expand Down
7 changes: 6 additions & 1 deletion modules/mod_smtp_delivery_external.c
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,12 @@ static int __attribute__ ((nonnull (2, 3, 9, 17))) try_send(struct smtp_session
if (!use_implicit_tls) {
if (allow_starttls) {
if (smtpclient.caps & SMTP_CAPABILITY_STARTTLS) {
if (bbs_smtp_client_starttls(&smtpclient)) {
if (!ssl_available() && require_starttls_out) {
bbs_warning("Encryption is mandatory, but TLS module is not loaded. Delivery failed.\n");
snprintf(buf, len, "TLS subsystem is unavailable");
res = -2;
goto cleanup;
} else if (bbs_smtp_client_starttls(&smtpclient)) {
res = -2;
goto cleanup; /* Abort if we were told STARTTLS was available but failed to negotiate. */
}
Expand Down
2 changes: 1 addition & 1 deletion modules/mod_smtp_fetchmail.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ static int fetch_single_host(const char *hostname, struct stringlist *domains)
bbs_warning("SMTP server %s does not support ETRN, unable to fetch mail\n", hostname);
goto cleanup;
}
if (smtpclient.caps & SMTP_CAPABILITY_STARTTLS) {
if (smtpclient.caps & SMTP_CAPABILITY_STARTTLS && ssl_available()) {
if (bbs_smtp_client_starttls(&smtpclient)) {
goto cleanup; /* Abort if we were told STARTTLS was available but failed to negotiate. */
}
Expand Down
4 changes: 3 additions & 1 deletion nets/net_imap.c
Original file line number Diff line number Diff line change
Expand Up @@ -3012,7 +3012,9 @@ static int handle_remote_move(struct imap_session *imap, char *dest, const char
goto cleanup;
}
/* Could get an untagged EXISTS at this point */
IMAP_CLIENT_EXPECT_EVENTUALLY(&destclient->client, 5, tagged_resp); /* tagged OK */
if (imap_client_wait_response(destclient, -1, SEC_MS(5))) { /* tagged OK */
goto cleanup;
}
}
} else {
if (destfd != -1) {
Expand Down
4 changes: 3 additions & 1 deletion nets/net_imap/imap_client_status.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,9 @@ static int status_size_fetch_incremental(struct imap_client *client, const char
/* Compare MESSAGES and UIDNEXT from the old and new responses */
if (parse_status_item(old, "UIDVALIDITY", &oldv) || parse_status_item(new, "UIDVALIDITY", &newv)) {
if (strlen_zero(old)) {
bbs_warning("Empty UIDVALIDITY in comparison, cannot rely on cache (%s / %s)\n", old, new);
/* This will happen the first time a particular remote IMAP folder is accessed...
* we obviously won't have any UIDVALIDITY cached for it. */
bbs_debug(5, "Empty UIDVALIDITY in comparison, cannot rely on cache (%s / %s)\n", old, new);
} else {
bbs_warning("UIDVALIDITY parsing error, cannot rely on cache (%s / %s)\n", old, new);
}
Expand Down

0 comments on commit bcbb2b1

Please sign in to comment.