Skip to content

Commit

Permalink
mod_smtp_delivery_external: Fix return value if STARTTLS fails.
Browse files Browse the repository at this point in the history
If STARTTLS failed, the return value was not reset to -1 after
a previous call that set the same variable, resulting in mail
getting lost after a failed STARTTLS since 0 was returned.
Fix this by resetting the return value to -1 to indicate the failure.

Related fixes:

net_smtp: Don't advertise STARTTLS capability if client is exempt
  from TLS.
socket.c: Fix inconsistent return value in bbs_poll_read.
  • Loading branch information
InterLinked1 committed Apr 27, 2024
1 parent 6a28bd6 commit 4abeff2
Show file tree
Hide file tree
Showing 7 changed files with 11 additions and 6 deletions.
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ Most common terminal emulators should work fine. The emulator's terminal type is
Some emulators are particularly good. Of all the well-known ones, these three terminal emulators are particularly recommended for BBSing on Windows:

* **SyncTERM** - Works well, looks nice. You **must** use the `newer 1.2 version <https://github.com/bbs-io/syncterm-windows/releases/tag/dev>`_. The more commonly downloaded 1.1 version has major bugs.
* **qodem** - Initial configuration slightly unintuitive, but otherwise works very well, with excellent support for non-standard display sizes.
* **qodem** - Initial configuration slightly unintuitive, but otherwise works very well, with excellent support for non-standard display sizes. Set :code:`doorway_mode_on_connect = mixed` in :code:`%userprofile%\Documents\qodem\prefs\qodemrc.txt`.
* **PuTTY** (and forks, like KiTTY) - Works well, no known issues. Not "retro" at all, but does the job fine.

Most other terminal emulators tested tend to have various setup, compatibility, or runtime issues. In particular:
Expand Down
2 changes: 1 addition & 1 deletion bbs/socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -1805,7 +1805,7 @@ ssize_t bbs_poll_read(int fd, int ms, char *buf, size_t len)
{
ssize_t res = bbs_poll(fd, ms);
if (res <= 0) {
return res - 1;
return -1;
}
res = read(fd, buf, len);
if (res <= 0) {
Expand Down
2 changes: 1 addition & 1 deletion bbs/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ int __bbs_pthread_join(pthread_t thread, void **retval, const char *file, const
/* Seems that after using pthread_timedjoin_np, you can't do a blocking pthread_join anymore? So loop */
while (res && res == ETIMEDOUT) {
bbs_debug(9, "Thread %lu not yet joined after %lus\n", thread, ts.tv_sec);
bbs_safe_sleep(50);
bbs_safe_sleep(250);
ts.tv_sec = 1; /* XXX Even this doesn't seem to make it work right */
res = pthread_timedjoin_np(thread, retval ? retval : &tmp, &ts);
}
Expand Down
2 changes: 1 addition & 1 deletion include/mod_smtp_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

#define SMTP_EXPECT(smtpclient, ms, str) \
res = bbs_tcp_client_expect(&(smtpclient)->client, "\r\n", 1, ms, str); \
if (res) { bbs_warning("Expected '%s', got: %s\n", str, (smtpclient)->client.rldata.buf); goto cleanup; } else { bbs_debug(9, "Found '%s': %s\n", str, (smtpclient)->client.rldata.buf); }
if (res) { bbs_warning("Expected '%s', returned %d, got: %s\n", str, res, (smtpclient)->client.rldata.buf); goto cleanup; } else { bbs_debug(9, "Found '%s': %s\n", str, (smtpclient)->client.rldata.buf); }

#define bbs_smtp_client_send(smtpclient, fmt, ...) bbs_tcp_client_send(&(smtpclient)->client, fmt, ## __VA_ARGS__); bbs_debug(3, " => " fmt, ## __VA_ARGS__);

Expand Down
6 changes: 5 additions & 1 deletion io/io_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ static void *zlib_thread(void *varg)
}
pfds[1].revents = 0;
}
if (pfds[0].revents || pfds[1].revents) {
bbs_debug(3, "poll returned %s\n", poll_revent_name(pfds[0].revents ? pfds[0].revents : pfds[1].revents));
break;
}
}
bbs_debug(4, "zlib thread exiting\n");
PIPE_CLOSE(z->wpfd);
Expand Down Expand Up @@ -298,7 +302,7 @@ static void cleanup(struct bbs_io_transformation *tran)
deflateEnd(z->compressor);
inflateEnd(z->decompressor);
z->done = 1;
pthread_kill(z->thread, SIGUSR1); /* Signal zlib_thread (shutdown(z->wpfd[1], SHUT_RDWR) does not work*/
pthread_kill(z->thread, SIGUSR1); /* Signal zlib_thread (shutdown(z->wpfd[1], SHUT_RDWR) does not work */
PIPE_CLOSE(z->rpfd);
PIPE_CLOSE(z->wpfd);
bbs_pthread_join(z->thread, NULL);
Expand Down
1 change: 1 addition & 0 deletions modules/mod_smtp_delivery_external.c
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ static int try_send(struct smtp_session *smtp, struct smtp_tx_data *tx, const ch
if (!secure) {
if (smtpclient.caps & SMTP_CAPABILITY_STARTTLS) {
if (bbs_smtp_client_starttls(&smtpclient)) {
res = -1;
goto cleanup; /* Abort if we were told STARTTLS was available but failed to negotiate. */
}
} else if (require_starttls_out) {
Expand Down
2 changes: 1 addition & 1 deletion nets/net_smtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ static int handle_helo(struct smtp_session *smtp, char *s, int ehlo)
smtp_reply0_nostatus(smtp, 250, "SIZE %u", max_message_size); /* RFC 1870 */
smtp_reply0_nostatus(smtp, 250, "8BITMIME"); /* RFC 6152 */
smtp_reply0_nostatus(smtp, 250, "ETRN"); /* RFC 1985 */
if (!smtp->node->secure && ssl_available()) {
if (!smtp->node->secure && ssl_available() && !exempt_from_starttls(smtp)) {
smtp_reply0_nostatus(smtp, 250, "STARTTLS");
}
if (bbs_user_is_registered(smtp->node->user)) {
Expand Down

0 comments on commit 4abeff2

Please sign in to comment.