Skip to content

Commit

Permalink
reject attempts at starttls for smtp & imap when no tls config is pre…
Browse files Browse the repository at this point in the history
…sent

we didn't announce starttls as capability, but clients can still try them. we
would try to do a handshake with a nil certificate, which would cause a
goroutine panic (which is handled gracefully, shutting down the connection).

found with code that was doing starttls unconditionally.
  • Loading branch information
mjl- committed Sep 15, 2024
1 parent 0977b7a commit a7bdc41
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 0 deletions.
3 changes: 3 additions & 0 deletions imapserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,9 @@ func (c *conn) cmdStarttls(tag, cmd string, p *parser) {
if c.tls {
xsyntaxErrorf("tls already active") // ../rfc/9051:1353
}
if c.tlsConfig == nil {
xsyntaxErrorf("starttls not announced")
}

conn := c.conn
if n := c.br.Buffered(); n > 0 {
Expand Down
3 changes: 3 additions & 0 deletions smtpserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,9 @@ func (c *conn) cmdStarttls(p *parser) {
if c.account != nil {
xsmtpUserErrorf(smtp.C503BadCmdSeq, smtp.SeProto5BadCmdOrSeq1, "cannot starttls after authentication")
}
if c.tlsConfig == nil {
xsmtpUserErrorf(smtp.C503BadCmdSeq, smtp.SeProto5BadCmdOrSeq1, "starttls not offered")
}

// We don't want to do TLS on top of c.r because it also prints protocol traces: We
// don't want to log the TLS stream. So we'll do TLS on the underlying connection,
Expand Down

0 comments on commit a7bdc41

Please sign in to comment.