From 1bccd34c1195934871713101b8873756b70fd758 Mon Sep 17 00:00:00 2001 From: Simhadri Saratchandra Date: Wed, 15 Jul 2026 19:16:21 +0530 Subject: [PATCH] feat: support STARTTLS on plaintext listeners Listeners with TLS config now take a mode field: "implicit" (default, unchanged behavior) wraps the connection in TLS immediately, while "starttls" accepts plaintext and upgrades in-session when the client issues STARTTLS (RFC 3207), as used on submission port 587. The previous STARTTLS path was dead code and broken in three ways: the parser required a CRLF the read loop had already stripped, main.rs never provided the session a TLS acceptor, and the upgrade duplicated the raw fd (unsafe from_raw_fd), creating two owners of the socket. - Add smtp::MaybeTlsStream, which upgrades plain->TLS in place via mem::replace; no unsafe, no recursive session handling. The acceptor travels with the stream, so each listener decides independently whether to offer STARTTLS. - Advertise 250-STARTTLS per stream capability; reject with 502 when unavailable or already on TLS, and 503 mid-transaction. - Discard bytes pipelined after the STARTTLS command before the handshake to prevent plaintext command injection, and fully reset the session afterwards (client must EHLO and authenticate again). - Bound the TLS handshake with cmd_timeout on both the STARTTLS upgrade and the implicit-TLS accept, so silent clients cannot hold connection permits indefinitely. - Fix parse of STARTTLS on CRLF-stripped lines; parameters still rejected. - Send a single 221 on QUIT: handle_client no longer writes a second Bye on clean termination (previously caused broken-pipe errors for fast-closing clients, and a stray 221 after timeout's 421). - Remove the now-unused SmtpServer::with_tls and the smtp crate's rustls/rustls-pemfile dependencies; update example configs and docs. Verified with cargo test (153 tests) and a live 10-case end-to-end run covering plain/implicit/STARTTLS listeners, injection, and timeouts. --- Cargo.lock | 2 - config.example.huml | 5 + config.example.toml | 10 +- docs/PRODUCTION_HARDENING.md | 6 +- docs/src/pages/configuration.md | 17 +- .../pages/reference/example-configuration.md | 9 +- docs/src/pages/reference/huml-example.md | 5 + smtp-server/src/config.rs | 13 + smtp-server/src/main.rs | 46 +++- smtp/Cargo.toml | 2 - smtp/src/lib.rs | 228 ++++++++++++------ smtp/src/parser.rs | 29 ++- 12 files changed, 265 insertions(+), 107 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a487fb9..4892a6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3144,8 +3144,6 @@ dependencies = [ "memchr", "miette", "nom 8.0.0", - "rustls", - "rustls-pemfile", "thiserror 1.0.69", "tokio", "tokio-rustls", diff --git a/config.example.huml b/config.example.huml index d47be8b..dbb205d 100644 --- a/config.example.huml +++ b/config.example.huml @@ -17,11 +17,16 @@ server:: addr: "0.0.0.0:25" - :: addr: "0.0.0.0:587" + tls:: + cert_path: "/etc/ssl/certs/smtp.crt" + key_path: "/etc/ssl/private/smtp.key" + mode: "starttls" - :: addr: "0.0.0.0:465" tls:: cert_path: "/etc/ssl/certs/smtp.crt" key_path: "/etc/ssl/private/smtp.key" + mode: "implicit" - :: addr: "127.0.0.1:2525" max_retries: 5 diff --git a/config.example.toml b/config.example.toml index d026485..89f798c 100644 --- a/config.example.toml +++ b/config.example.toml @@ -38,16 +38,22 @@ bind = "0.0.0.0:8080" [[server.listeners]] addr = "0.0.0.0:25" -# SMTP submission port (plaintext) +# SMTP submission port (STARTTLS: starts plaintext, upgrades when the +# client issues STARTTLS) [[server.listeners]] addr = "0.0.0.0:587" +[server.listeners.tls] +cert_path = "/etc/ssl/certs/smtp.crt" +key_path = "/etc/ssl/private/smtp.key" +mode = "starttls" -# SMTPS port (TLS) +# SMTPS port (implicit TLS: the connection is TLS from the first byte) [[server.listeners]] addr = "0.0.0.0:465" [server.listeners.tls] cert_path = "/etc/ssl/certs/smtp.crt" key_path = "/etc/ssl/private/smtp.key" +mode = "implicit" # default when omitted # Local testing port (plaintext) [[server.listeners]] diff --git a/docs/PRODUCTION_HARDENING.md b/docs/PRODUCTION_HARDENING.md index af4a548..698f17e 100644 --- a/docs/PRODUCTION_HARDENING.md +++ b/docs/PRODUCTION_HARDENING.md @@ -82,9 +82,11 @@ Production deployments should set `server.helo_hostname` to the public FQDN for --- -### 8. STARTTLS is broken on plaintext listeners +### 8. ~~STARTTLS is broken on plaintext listeners~~ → Addressed -**Problem:** Two issues: +**Status:** Addressed. Listeners now take `mode = "starttls"` in their TLS config to accept plaintext connections and upgrade via STARTTLS (RFC 3207). The upgrade happens in place through `smtp::MaybeTlsStream` (no raw-FD tricks), any bytes pipelined after the STARTTLS command are discarded to prevent plaintext injection, and the SMTP session is fully reset after the handshake. + +**Problem (original):** Two issues: 1. `main.rs` never calls `SmtpServer::with_tls()`, so the SMTP session never has a `tls_acceptor` and STARTTLS is never advertised on plaintext listeners. 2. `upgrade_to_tls()` in `smtp/src/lib.rs` uses `unsafe { TcpStream::from_raw_fd() }` which creates two owners of the same FD — a use-after-free risk. diff --git a/docs/src/pages/configuration.md b/docs/src/pages/configuration.md index 2ba0802..52b7bd3 100644 --- a/docs/src/pages/configuration.md +++ b/docs/src/pages/configuration.md @@ -48,16 +48,29 @@ For new or low-volume senders, keep `min_idle` and `max_size` small so strict re ```toml [[server.listeners]] -addr = "0.0.0.0:25" # Bind address and port +addr = "0.0.0.0:465" # Bind address and port # Optional TLS configuration [server.listeners.tls] cert_path = "/path/to/cert.pem" key_path = "/path/to/key.pem" +mode = "implicit" # "implicit" (default) or "starttls" [[server.listeners]] -addr = "127.0.0.1:2525" # Second listener without TLS +addr = "0.0.0.0:587" # STARTTLS listener: accepts plaintext and +[server.listeners.tls] # upgrades to TLS when the client asks +cert_path = "/path/to/cert.pem" +key_path = "/path/to/key.pem" +mode = "starttls" + +[[server.listeners]] +addr = "127.0.0.1:2525" # Listener without TLS ``` +Each listener can negotiate TLS in one of two ways: + +- `mode = "implicit"` (the default): the TLS handshake happens as soon as the connection opens, before any SMTP traffic. Use this for SMTPS (port 465). +- `mode = "starttls"`: the connection starts in plaintext and the server advertises `STARTTLS` in its EHLO response; the session is upgraded to TLS when the client issues the `STARTTLS` command (RFC 3207). Use this for submission (port 587). + ## Authentication (`[[server.auth]]`) ```toml diff --git a/docs/src/pages/reference/example-configuration.md b/docs/src/pages/reference/example-configuration.md index 98c9e51..89d6a88 100644 --- a/docs/src/pages/reference/example-configuration.md +++ b/docs/src/pages/reference/example-configuration.md @@ -25,11 +25,18 @@ max_size = 10 # Multiple listeners [[server.listeners]] -addr = "0.0.0.0:25" +addr = "0.0.0.0:465" # Implicit TLS (default mode) [server.listeners.tls] cert_path = "/etc/hedwig/server.crt" key_path = "/etc/hedwig/server.key" +[[server.listeners]] +addr = "0.0.0.0:587" # Plaintext with STARTTLS upgrade +[server.listeners.tls] +cert_path = "/etc/hedwig/server.crt" +key_path = "/etc/hedwig/server.key" +mode = "starttls" + [[server.listeners]] addr = "127.0.0.1:2525" # Local plaintext listener diff --git a/docs/src/pages/reference/huml-example.md b/docs/src/pages/reference/huml-example.md index 84e0a55..ea06419 100644 --- a/docs/src/pages/reference/huml-example.md +++ b/docs/src/pages/reference/huml-example.md @@ -28,11 +28,16 @@ server:: addr: "0.0.0.0:25" - :: addr: "0.0.0.0:587" + tls:: + cert_path: "/etc/ssl/certs/smtp.crt" + key_path: "/etc/ssl/private/smtp.key" + mode: "starttls" - :: addr: "0.0.0.0:465" tls:: cert_path: "/etc/ssl/certs/smtp.crt" key_path: "/etc/ssl/private/smtp.key" + mode: "implicit" - :: addr: "127.0.0.1:2525" max_retries: 5 diff --git a/smtp-server/src/config.rs b/smtp-server/src/config.rs index a4b3050..89fddb2 100644 --- a/smtp-server/src/config.rs +++ b/smtp-server/src/config.rs @@ -141,6 +141,19 @@ pub struct CfgListener { pub struct CfgTls { pub cert_path: String, pub key_path: String, + /// How TLS is negotiated on this listener: "implicit" wraps the + /// connection in TLS immediately (e.g. port 465), "starttls" accepts + /// plaintext and upgrades when the client issues STARTTLS (e.g. port 587). + #[serde(default)] + pub mode: TlsMode, +} + +#[derive(Debug, Deserialize, Clone, Copy, Default, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum TlsMode { + #[default] + Implicit, + Starttls, } #[derive(Debug, Deserialize, Clone)] diff --git a/smtp-server/src/main.rs b/smtp-server/src/main.rs index 8162c6c..55970ff 100644 --- a/smtp-server/src/main.rs +++ b/smtp-server/src/main.rs @@ -4,7 +4,7 @@ use futures::StreamExt; use miette::{bail, Context, IntoDiagnostic, Result}; use mta_sts::refresher; use rustls::pki_types::CertificateDer; -use smtp::{SmtpServer, SmtpStream}; +use smtp::{MaybeTlsStream, SmtpServer, SmtpStream}; use std::sync::Arc; use storage::{fs_storage::FileSystemStorage, sqlite_storage::SqliteStorage, Status, Storage}; use subtle::ConstantTimeEq; @@ -294,10 +294,10 @@ async fn run_server(config_path: &str) -> Result<()> { .into_diagnostic() .wrap_err_with(|| format!("Failed to bind to address: {}", listener_config.addr))?; - let tls_status = if listener_config.tls.is_some() { - "TLS" - } else { - "plaintext" + let tls_status = match &listener_config.tls { + Some(tls) if tls.mode == config::TlsMode::Starttls => "STARTTLS", + Some(_) => "TLS", + None => "plaintext", }; info!( storage_type = cfg.storage.storage_type, @@ -310,6 +310,11 @@ async fn run_server(config_path: &str) -> Result<()> { for (listener, acceptor_index) in listeners { let server_clone = smtp_server.clone(); let tls_acceptor = tls_acceptors[acceptor_index].clone(); + let tls_mode = cfg.server.listeners[acceptor_index] + .tls + .as_ref() + .map(|tls| tls.mode) + .unwrap_or_default(); let shutdown = shutdown_token.clone(); let listener_addr = cfg.server.listeners[acceptor_index].addr.clone(); let conn_semaphore = Arc::clone(&conn_semaphore); @@ -349,11 +354,18 @@ async fn run_server(config_path: &str) -> Result<()> { // Hold the permit for the lifetime of this connection. let _permit = permit; - let mut boxed_socket: Box = - if let Some(acceptor) = tls_acceptor { - match acceptor.accept(socket).await { - Ok(tls_stream) => Box::new(tls_stream), - Err(e) => { + let mut boxed_socket: Box = match tls_acceptor { + // Implicit TLS: the handshake happens before any SMTP traffic. + // Bounded so a silent client can't hold a connection permit forever. + Some(acceptor) if tls_mode == config::TlsMode::Implicit => { + match tokio::time::timeout( + cmd_timeout, + acceptor.accept(socket), + ) + .await + { + Ok(Ok(tls_stream)) => Box::new(tls_stream), + Ok(Err(e)) => { if e.kind() == std::io::ErrorKind::UnexpectedEof { debug!("TLS handshake failed: {}", e); } else { @@ -362,10 +374,18 @@ async fn run_server(config_path: &str) -> Result<()> { return; } + Err(_) => { + debug!("TLS handshake timed out"); + return; + } } - } else { - Box::new(socket) - }; + } + // STARTTLS: start in plaintext, upgrade when the client asks. + Some(acceptor) => { + Box::new(MaybeTlsStream::Plain(socket, Some(acceptor))) + } + None => Box::new(socket), + }; if let Err(e) = server_clone.handle_client(&mut boxed_socket).await { error!("Error handling client: {:#}", e); diff --git a/smtp/Cargo.toml b/smtp/Cargo.toml index bbb2a3f..a3060ee 100644 --- a/smtp/Cargo.toml +++ b/smtp/Cargo.toml @@ -11,7 +11,5 @@ thiserror = { workspace = true } tokio = { workspace = true } async-trait = { workspace = true } tokio-rustls = "0.26.1" -rustls = { version = "0.23", features = ["ring"] } -rustls-pemfile = "2.2.0" bytes = "1.10.0" memchr = "2.7.4" diff --git a/smtp/src/lib.rs b/smtp/src/lib.rs index 669bed5..9cdbe42 100644 --- a/smtp/src/lib.rs +++ b/smtp/src/lib.rs @@ -5,17 +5,14 @@ use base64::prelude::*; use bytes::BytesMut; use memchr::memchr; use miette::{bail, Context, Diagnostic, IntoDiagnostic, Result, SourceSpan}; -use std::os::unix::io::AsRawFd; // or windows equivalent -use std::os::unix::io::FromRawFd; +use std::pin::Pin; use std::sync::Arc; +use std::task::Poll; use std::time::Duration; use thiserror::Error; -use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt, ReadBuf}; use tokio::net::TcpStream; -use rustls::pki_types::CertificateDer; -use std::path::Path; -use tokio_rustls::rustls::{self, ServerConfig}; use tokio_rustls::{server::TlsStream, TlsAcceptor}; pub mod parser; @@ -28,30 +25,116 @@ pub trait SmtpStream: AsyncRead + AsyncWrite + Unpin + Send { Ok(()) } - async fn upgrade_to_tls( - &mut self, - _acceptor: &TlsAcceptor, - ) -> Result>> { - Ok(None) + /// Whether this stream can be upgraded to TLS via STARTTLS. + fn supports_starttls(&self) -> bool { + false + } + + /// Upgrades the stream to TLS in place. Only valid when + /// `supports_starttls()` returns true. + async fn upgrade_to_tls(&mut self) -> Result<()> { + bail!("STARTTLS not supported on this stream") } } #[async_trait] -impl SmtpStream for TcpStream { - async fn upgrade_to_tls( - &mut self, - acceptor: &TlsAcceptor, - ) -> Result>> { - let fd = self.as_raw_fd(); - let stream = unsafe { TcpStream::from_std(std::net::TcpStream::from_raw_fd(fd)) } - .into_diagnostic()?; - let tls_stream = acceptor.accept(stream).await.into_diagnostic()?; - Ok(Some(tls_stream)) +impl SmtpStream for TcpStream {} + +#[async_trait] +impl SmtpStream for TlsStream {} + +/// A connection that starts out as plain TCP and may be upgraded to TLS +/// mid-session via STARTTLS. Carrying the acceptor with the stream lets each +/// listener decide independently whether to offer STARTTLS. +pub enum MaybeTlsStream { + Plain(TcpStream, Option), + Tls(Box>), + /// Transient state while the TLS handshake runs; only observable if the + /// handshake fails, after which the connection is unusable. + Upgrading, +} + +impl AsyncRead for MaybeTlsStream { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + match self.get_mut() { + MaybeTlsStream::Plain(s, _) => Pin::new(s).poll_read(cx, buf), + MaybeTlsStream::Tls(s) => Pin::new(s).poll_read(cx, buf), + MaybeTlsStream::Upgrading => Poll::Ready(Err(upgrading_io_error())), + } } } +impl AsyncWrite for MaybeTlsStream { + fn poll_write( + self: Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + buf: &[u8], + ) -> Poll> { + match self.get_mut() { + MaybeTlsStream::Plain(s, _) => Pin::new(s).poll_write(cx, buf), + MaybeTlsStream::Tls(s) => Pin::new(s).poll_write(cx, buf), + MaybeTlsStream::Upgrading => Poll::Ready(Err(upgrading_io_error())), + } + } + + fn poll_flush( + self: Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> Poll> { + match self.get_mut() { + MaybeTlsStream::Plain(s, _) => Pin::new(s).poll_flush(cx), + MaybeTlsStream::Tls(s) => Pin::new(s).poll_flush(cx), + MaybeTlsStream::Upgrading => Poll::Ready(Err(upgrading_io_error())), + } + } + + fn poll_shutdown( + self: Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> Poll> { + match self.get_mut() { + MaybeTlsStream::Plain(s, _) => Pin::new(s).poll_shutdown(cx), + MaybeTlsStream::Tls(s) => Pin::new(s).poll_shutdown(cx), + MaybeTlsStream::Upgrading => Poll::Ready(Err(upgrading_io_error())), + } + } +} + +fn upgrading_io_error() -> std::io::Error { + std::io::Error::new( + std::io::ErrorKind::NotConnected, + "connection unusable after failed TLS upgrade", + ) +} + #[async_trait] -impl SmtpStream for TlsStream {} +impl SmtpStream for MaybeTlsStream { + fn supports_starttls(&self) -> bool { + matches!(self, MaybeTlsStream::Plain(_, Some(_))) + } + + async fn upgrade_to_tls(&mut self) -> Result<()> { + match std::mem::replace(self, MaybeTlsStream::Upgrading) { + MaybeTlsStream::Plain(tcp, Some(acceptor)) => { + let tls_stream = acceptor + .accept(tcp) + .await + .into_diagnostic() + .wrap_err("TLS handshake failed during STARTTLS upgrade")?; + *self = MaybeTlsStream::Tls(Box::new(tls_stream)); + Ok(()) + } + other => { + *self = other; + bail!("STARTTLS not supported on this stream") + } + } + } +} #[derive(Debug, Error, Diagnostic)] pub enum SmtpError { @@ -157,9 +240,6 @@ pub struct SmtpServer { // Indicates whether authentication is enabled for this server. auth_enabled: bool, - // TLS acceptor for handling encrypted connections. - tls_acceptor: Option, - // Maximum message size in bytes. Enforced during DATA and advertised via SIZE in EHLO. max_message_size: usize, @@ -184,7 +264,6 @@ impl SmtpServer { SmtpServer { callbacks: Arc::new(callbacks), auth_enabled, - tls_acceptor: None, max_message_size: DEFAULT_MAX_MESSAGE_SIZE, cmd_timeout: DEFAULT_CMD_TIMEOUT, data_timeout: DEFAULT_DATA_TIMEOUT, @@ -206,36 +285,6 @@ impl SmtpServer { self } - pub fn with_tls( - mut self, - cert_path: impl AsRef, - key_path: impl AsRef, - ) -> Result { - let cert_file = std::fs::File::open(cert_path) - .into_diagnostic() - .wrap_err("Failed to open certificate file")?; - let key_file = std::fs::File::open(key_path) - .into_diagnostic() - .wrap_err("Failed to open private key file")?; - - let certs: Vec> = - rustls_pemfile::certs(&mut std::io::BufReader::new(cert_file)) - .collect::>>() - .into_diagnostic()?; - - let key = rustls_pemfile::private_key(&mut std::io::BufReader::new(key_file)) - .into_diagnostic()? - .ok_or_else(|| miette::miette!("No private key found"))?; - - let config = ServerConfig::builder() - .with_no_client_auth() - .with_single_cert(certs, key) - .into_diagnostic()?; - - self.tls_acceptor = Some(TlsAcceptor::from(Arc::new(config))); - Ok(self) - } - /// Handles a client connection. /// /// This method processes SMTP commands from the client and manages the SMTP session. @@ -273,8 +322,9 @@ impl SmtpServer { _ => Ok(()), } } else { - // Handle successful connection termination - socket.write_line(b"221 Bye\r\n").await + // Clean termination: QUIT already answered with 221, and on + // EOF/timeout the client is gone — nothing more to write. + Ok(()) } } @@ -370,6 +420,9 @@ impl SmtpServer { let line = buf.split_to(cr + 2); // Remove CRLF. let line = &line[..line.len().saturating_sub(2)]; + // Deliberate leniency: surrounding whitespace is trimmed + // before parsing, so padded commands from sloppy clients + // (e.g. "STARTTLS \r\n") are accepted. let command = std::str::from_utf8(line) .map_err(|err| SmtpError::ParseError { message: format!("Invalid UTF-8 sequence: {}", err), @@ -379,6 +432,44 @@ impl SmtpServer { .to_string(); match parse_command(&command, &session.state) { + // STARTTLS is handled here rather than in handle_command + // because the upgrade must also discard any bytes the + // client pipelined after the command (RFC 3207: possible + // plaintext injection) — and those live in `buf`. + Ok(SmtpCommand::StartTls) => { + if !stream.supports_starttls() { + stream.write_line(b"502 STARTTLS not supported\r\n").await?; + } else if matches!( + session.state, + SessionState::ReceivingMailFrom | SessionState::ReceivingRcptTo + ) { + stream + .write_line(b"503 STARTTLS not allowed during mail transaction\r\n") + .await?; + } else { + stream.write_line(b"220 Ready to start TLS\r\n").await?; + buf.clear(); + data_buffer.clear(); + // Bound the handshake so a client that goes + // silent after STARTTLS can't hold the + // connection (and its permit) forever. + match tokio::time::timeout( + self.cmd_timeout, + stream.upgrade_to_tls(), + ) + .await + { + Ok(result) => result?, + // The handshake never completed; the stream + // is unusable, so just drop the connection. + Err(_) => return Ok(()), + } + // RFC 3207: the session is reset to its initial + // state; the client must EHLO again. + session.email = Email::default(); + session.state = SessionState::Connected; + } + } Ok(cmd) => { if self.handle_command(session, cmd, stream).await? { return Ok(()); @@ -410,7 +501,7 @@ impl SmtpServer { self.callbacks.on_ehlo(&domain).await?; let mut response = String::from("250-localhost\r\n"); response.push_str(&format!("250-SIZE {}\r\n", self.max_message_size)); - if self.tls_acceptor.is_some() { + if stream.supports_starttls() { response.push_str("250-STARTTLS\r\n"); } // Add AUTH support if enabled. @@ -474,25 +565,6 @@ impl SmtpServer { .await?; session.state = SessionState::ReceivingData; } - (_, SmtpCommand::StartTls) => { - if let Some(acceptor) = &self.tls_acceptor { - stream.write_line(b"220 Ready to start TLS\r\n").await?; - - if let Some(tls_stream) = stream.upgrade_to_tls(acceptor).await? { - session.state = SessionState::Connected; - let mut new_stream = Box::new(tls_stream) as Box; - // Wrap the recursive call in Box::pin - Box::pin(self.handle_connection(session, &mut new_stream)).await?; - return Ok(true); - } else { - stream - .write_line(b"454 TLS not available due to temporary reason\r\n") - .await?; - } - } else { - stream.write_line(b"502 STARTTLS not supported\r\n").await?; - } - } (_, SmtpCommand::Quit) => { stream.write_line(b"221 Bye\r\n").await?; return Ok(true); diff --git a/smtp/src/parser.rs b/smtp/src/parser.rs index ee7f682..0c6f512 100644 --- a/smtp/src/parser.rs +++ b/smtp/src/parser.rs @@ -89,8 +89,7 @@ fn parse_normal_command(input: &str) -> IResult<&str, SmtpCommand> { parse_mail_from, parse_rcpt_to, parse_simple_command, - // Changed this line to be exact - map(tag_no_case("STARTTLS\r\n"), |_| SmtpCommand::StartTls), + parse_starttls, )) .parse(input) } @@ -210,6 +209,19 @@ fn parse_rcpt_to(input: &str) -> IResult<&str, SmtpCommand> { .parse(input) } +/// Parses STARTTLS. RFC 3207 forbids parameters, so nothing may follow the +/// command word (the read loop has already stripped the CRLF, but accept a +/// trailing CRLF too for callers that pass raw lines). +fn parse_starttls(input: &str) -> IResult<&str, SmtpCommand> { + map( + verify(preceded(tag_no_case("STARTTLS"), rest), |r: &str| { + r.is_empty() || r == "\r\n" + }), + |_| SmtpCommand::StartTls, + ) + .parse(input) +} + fn parse_simple_command(input: &str) -> IResult<&str, SmtpCommand> { alt(( map(tag_no_case("DATA"), |_| SmtpCommand::Data), @@ -648,13 +660,20 @@ mod tests { SmtpCommand::StartTls ); - // Should fail without CRLF - assert!(parse_command("STARTTLS", &SessionState::Connected).is_err()); + // The read loop strips CRLF before parsing, so the bare word must parse + assert_eq!( + parse_command("STARTTLS", &SessionState::Connected).unwrap(), + SmtpCommand::StartTls + ); // Should fail with parameters assert!(parse_command("STARTTLS param\r\n", &SessionState::Connected).is_err()); + assert!(parse_command("STARTTLS param", &SessionState::Connected).is_err()); - // Should fail with extra spaces + // The parser itself rejects surrounding whitespace. Note that the + // server's read loop trims each line before parsing, so whitespace- + // padded commands (e.g. "STARTTLS \r\n") are normalized and accepted + // there — these assertions only cover direct parser callers. assert!(parse_command("STARTTLS \r\n", &SessionState::Connected).is_err()); assert!(parse_command(" STARTTLS\r\n", &SessionState::Connected).is_err()); }