Verify the SMTP server's identity, with smtp.verify-server-identity to opt out

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - The plugin now reports a usage event — `startup` on enable — to the author's trace server so it is known which plugins are in use. Events carry the plugin name, the event name, and the plugin version; nothing about players or the server. Herald registers no commands, so there is no per-command event. Reporting runs off the main thread, never delays a tick, drops silently when the server is unreachable, and is turned off with `usage-reporting.enabled: false` in `config.yml`. The default config carries the plugin's key, so reporting is active out of the box unless turned off — including on servers upgraded from a version before the `usage-reporting` block existed, whose `config.yml` is never rewritten: the plugin reads the bundled defaults for any key the file lacks.
 - `smtp.implicit-tls` config option, which encrypts the connection to the SMTP server before the first SMTP command instead of upgrading a plain one with STARTTLS. This is what port `465` — SMTPS — expects, and what several hosted mail providers publish as their primary submission port; neither setting of `smtp.use-tls` could reach such a server before, since one sent `EHLO` in the clear to a port that would not read it and the other sent everything in the clear. It defaults to `false`, so an existing configuration keeps using STARTTLS on the port it was set up against. It is an alternative to `smtp.use-tls` rather than an addition to it: setting both to `true` is reported at startup and email notifications are skipped, rather than one mode being picked silently.
+- `smtp.verify-server-identity` config option, which controls whether the certificate the SMTP server presents is checked against the address in `smtp.server`. It defaults to `true`, and Herald now sets `mail.smtp.ssl.checkserveridentity` explicitly in both directions whenever `smtp.use-tls` or `smtp.implicit-tls` is on. The pinned `com.sun.mail:jakarta.mail:2.0.1` leaves that check off unless it is asked for and later versions of the library default it the other way, so until now whether the encrypted connection reached the host that was configured was decided by the dependency version rather than by Herald, in the direction of not checking. An installation whose relay presents a certificate that cannot name the address it is dialled at — by IP address, or under another hostname — will start failing its sends after this change with a certificate error in the log, and must set `smtp.verify-server-identity` to `false` to keep sending; doing so is reported at startup, since the connection then stays encrypted without establishing who answered it.
 - A startup warning when `smtp.port` and the encryption mode disagree on either of the two conventional ports — `465` without `smtp.implicit-tls`, or `smtp.implicit-tls` on `587`. Both combinations fail at the protocol level on the first player join, which reads as an unrecognised command or a handshake error rather than as a configuration mistake. It stays a warning rather than a refusal, since a relay may offer either mode on any port.
 
 - A `Dev Release` workflow, which republishes a rolling `dev` prerelease of `main` on every non-documentation push. This is what Dan's Plugin Manager's experimental channel installs from: `/dpm get herald --experimental` reads `releases/tags/dev`, so without it there is nothing for that command to download. The prerelease is unreleased, unreviewed code and is marked as such.

CONFIG.md

@@ -165,6 +165,8 @@ This is binding rather than best-effort: when it is `true`, a server that does n
 'smtp.username' is set but neither 'smtp.use-tls' nor 'smtp.implicit-tls' is true, so the SMTP username and password are sent over an unencrypted connection, along with every notification. Set 'smtp.use-tls' to true, or 'smtp.implicit-tls' to true on a port that expects SMTPS, unless the server genuinely has no encryption support.
 ```
 
+What this key covers is that the connection is encrypted, not who is on the other end of it; that is `smtp.verify-server-identity`, which is on by default and applies here.
+
 **Example:**
 
 ```yaml
@@ -186,6 +188,8 @@ The two encryption keys are alternatives rather than layers: `smtp.use-tls` star
 
 Leaving both `false` sends everything, credentials included, in plain text; see `smtp.use-tls` for what that costs.
 
+Like `smtp.use-tls`, this key governs only whether the connection is encrypted. Whether the certificate on the other end belongs to `smtp.server` is `smtp.verify-server-identity`, which is on by default and applies to this mode identically.
+
 **Example:**
 
 ```yaml
@@ -195,6 +199,29 @@ smtp:
   implicit-tls: true
 ```
 
+## smtp.verify-server-identity
+
+**Type:** boolean
+**Default:** `true`
+**Description:** Whether the certificate the SMTP server presents is checked against the address configured in `smtp.server`. This is a separate question from encryption, and neither `smtp.use-tls` nor `smtp.implicit-tls` answers it: those two establish that the connection is private, while this one establishes that it is private with the host that was asked for rather than with whoever answered. It applies to both encryption modes equally, and has no effect when neither is turned on, since there is then no certificate to check.
+
+Herald sets this explicitly in both directions rather than leaving it to the mail library. The pinned `com.sun.mail:jakarta.mail:2.0.1` does not verify the identity unless it is asked to, and later versions of the library default the other way, so leaving it unset would make the behaviour a property of the version that happens to be on the classpath.
+
+Setting it to `false` is a deliberate choice for a relay whose certificate cannot name the address it is reached at — one dialled by IP address, or one presenting a certificate issued for a different hostname. Herald warns at startup when it is turned off while an encryption mode is in use, rather than refusing to load, since mail is still delivered over an encrypted connection:
+
+```
+'smtp.verify-server-identity' is false, so the certificate the SMTP server presents is not checked against 'smtp.server'. The connection is still encrypted, but it is no longer established that it is encrypted with the host that was asked for. Set 'smtp.verify-server-identity' back to true unless the server is reached by an address the certificate cannot name.
+```
+
+Because the default is `true`, an installation that was sending mail to such a relay before this key existed will begin to fail on the first player join after upgrading, with a certificate error in the log. Setting this key to `false` restores the previous behaviour; pointing `smtp.server` at the hostname the certificate actually names is the better fix where it is available.
+
+**Example:**
+
+```yaml
+smtp:
+  verify-server-identity: true
+```
+
 ## email.enabled
 
 **Type:** boolean

USER_GUIDE.md

@@ -50,7 +50,8 @@ discord:
 4. Leave `email.enabled` set to `true`.
 5. Leave `smtp.use-tls` set to `true` unless your server has no STARTTLS support. It is required rather than attempted, so a server that cannot do STARTTLS reports a failure instead of sending in the clear.
 6. If your provider publishes port `465` instead, set `smtp.port` to `465`, `smtp.implicit-tls` to `true` and `smtp.use-tls` to `false`. That port encrypts the connection before the first command rather than upgrading it partway through, so exactly one of the two keys applies to any given server.
-7. Restart the server.
+7. Leave `smtp.verify-server-identity` set to `true`, so that whichever encryption mode you chose also confirms the certificate belongs to the host in `smtp.server`. Turn it off only for a relay dialled by IP address, or one presenting a certificate for a different name.
+8. Restart the server.
 
 ### Turning a Notification Channel Off
 
@@ -91,13 +92,15 @@ Herald reports what it loaded in the server log at startup:
 - `'smtp.username' is set but neither 'smtp.use-tls' nor 'smtp.implicit-tls' is true, ...` — email is configured and will be sent, but the SMTP credentials and every notification travel unencrypted. Turning on whichever encryption mode the server offers clears it.
 - `'smtp.use-tls' and 'smtp.implicit-tls' are both true, ...` — the two encryption modes are alternatives, so email notifications are skipped until exactly one of them is set to `true`.
 - `'smtp.port' is 465, which conventionally expects implicit TLS, ...` — or the reverse, implicit TLS on port `587`. Email notifications still load, because a relay may offer either mode on any port, but a send that fails with a protocol error after this warning is explained by it.
+- `'smtp.verify-server-identity' is false, ...` — email is configured and will be sent over an encrypted connection, but the certificate presented is no longer checked against `smtp.server`. Setting the key back to `true` clears it.
 
 If a notification fails to send later, Herald logs the failure with the reason and names the channel it was sent through (`Discord` or `email`) — for Discord, the reason includes the error message the webhook returned.
 
 Some email failures are worth recognising by sight:
 
 - A failure mentioning STARTTLS means `smtp.use-tls` is `true` but the server did not offer STARTTLS. Either point `smtp.server` and `smtp.port` at a port that does — usually `587` — or, if the server truly cannot, set `smtp.use-tls` to `false` and accept that the connection is then unencrypted. On port `465` the answer is neither: that port wants `smtp.implicit-tls` instead.
 - A failure mentioning an unrecognised command, or a handshake or SSL error, usually means the encryption mode and the port disagree — implicit TLS on a STARTTLS port, or the reverse. Herald warns about the two conventional ports at startup, so the startup log names it as well.
+- A failure naming the certificate, or reporting that the hostname does not match, means `smtp.verify-server-identity` is `true` and the certificate the server presented was issued for some other name than the one in `smtp.server`. Point `smtp.server` at the name the certificate carries where you can; where you cannot — a relay reached by IP address, for instance — set `smtp.verify-server-identity` to `false` and accept that the encrypted connection no longer proves who answered it.
 - A failure mentioning a connect timeout means no connection to `smtp.server` on `smtp.port` could be established within ten seconds — usually a wrong host or port, or a firewall dropping the packets.
 - A failure mentioning a read timeout means the connection was established but the server stopped answering, and Herald gave up after thirty seconds rather than waiting indefinitely. Either way the failure is reported, instead of the notification silently holding a task for every join.
 

src/main/java/com/dansplugins/herald/EmailNotifier.java

@@ -50,6 +50,7 @@ public class EmailNotifier implements Notifier {
     private final String emailSender;
     private final boolean useTLS;
     private final boolean implicitTLS;
+    private final boolean verifyServerIdentity;
     private final List<String> recipients;
     private final String subjectTemplate;
     private final String bodyTemplate;
@@ -96,13 +97,32 @@ public EmailNotifier(String smtpServer, int smtpPort, String smtpUsername,
     public EmailNotifier(String smtpServer, int smtpPort, String smtpUsername,
                          String smtpPassword, String emailSender, boolean useTLS, boolean implicitTLS,
                          List<String> recipients, String subjectTemplate, String bodyTemplate) {
+        this(smtpServer, smtpPort, smtpUsername, smtpPassword, emailSender, useTLS, implicitTLS, true,
+                recipients, subjectTemplate, bodyTemplate);
+    }
+
+    /**
+     * Create a notifier that can be told not to verify the SMTP server's identity.
+     * Encryption establishes that the connection is private; verification establishes
+     * that it is private with the host named in {@code smtp.server} rather than with
+     * whoever answered. The two are separate, and the pinned mail library leaves the
+     * second one off unless it is asked for, so Herald asks for it rather than
+     * inheriting whatever the pinned version happens to default to.
+     *
+     * @param verifyServerIdentity the configured {@code smtp.verify-server-identity}
+     */
+    public EmailNotifier(String smtpServer, int smtpPort, String smtpUsername,
+                         String smtpPassword, String emailSender, boolean useTLS, boolean implicitTLS,
+                         boolean verifyServerIdentity, List<String> recipients,
+                         String subjectTemplate, String bodyTemplate) {
         this.smtpServer = smtpServer;
         this.smtpPort = smtpPort;
         this.smtpUsername = smtpUsername;
         this.smtpPassword = smtpPassword;
         this.emailSender = emailSender;
         this.useTLS = useTLS;
         this.implicitTLS = implicitTLS;
+        this.verifyServerIdentity = verifyServerIdentity;
         this.recipients = recipients != null ? new ArrayList<>(recipients) : new ArrayList<>();
         this.subjectTemplate = templateOrDefault(subjectTemplate, DEFAULT_SUBJECT);
         this.bodyTemplate = templateOrDefault(bodyTemplate, DEFAULT_BODY);
@@ -204,6 +224,34 @@ public static String describeTlsModeConflict(boolean useTLS, boolean implicitTLS
                 + "usually on port " + IMPLICIT_TLS_PORT + ". Set exactly one of them to true.";
     }
 
+    /**
+     * Describe what turning off server identity verification costs.
+     * Reported as a warning rather than as a problem from
+     * {@link #validateConfiguration(List, String, int, String)} because the
+     * combination is deliberate on a relay reached by address, or one presenting a
+     * certificate for another name, and mail is still delivered over an encrypted
+     * connection; what that connection no longer establishes is who is on the other
+     * end of it. Silent when no encryption mode is in use, since there is then no
+     * certificate to check and {@link #describeCredentialExposure(String, boolean, boolean)}
+     * already names the larger problem.
+     *
+     * @param verifyServerIdentity the configured {@code smtp.verify-server-identity}
+     * @param useTLS               the configured {@code smtp.use-tls}
+     * @param implicitTLS          the configured {@code smtp.implicit-tls}
+     * @return the warning to log, or {@code null} when nothing is being skipped
+     */
+    public static String describeUnverifiedServerIdentity(boolean verifyServerIdentity, boolean useTLS,
+                                                          boolean implicitTLS) {
+        if (verifyServerIdentity || !(useTLS || implicitTLS)) {
+            return null;
+        }
+        return "'smtp.verify-server-identity' is false, so the certificate the SMTP server presents is not "
+                + "checked against 'smtp.server'. The connection is still encrypted, but it is no longer "
+                + "established that it is encrypted with the host that was asked for. Set "
+                + "'smtp.verify-server-identity' back to true unless the server is reached by an address the "
+                + "certificate cannot name.";
+    }
+
     /**
      * Describe an encryption mode that does not match the port it is pointed at.
      * Only the two conventional ports are judged, because a relay is free to offer
@@ -320,6 +368,14 @@ private boolean usesAuthentication() {
      * session cannot do both. Herald refuses the combination at startup, so the
      * precedence below only decides what a caller that built the notifier directly gets.
      *
+     * <p>{@code mail.smtp.ssl.checkserveridentity} is set explicitly whenever either mode
+     * is in use, in both directions, rather than being left to the mail library: the
+     * pinned {@code com.sun.mail:jakarta.mail:2.0.1} defaults it to {@code false}, and a
+     * later version defaults it the other way, so leaving it unset would make whether the
+     * certificate is checked against {@code smtp.server} a property of the dependency
+     * version rather than of Herald. It is left unset when neither mode is in use, since
+     * there is then no handshake for it to govern.
+     *
      * @return the SMTP properties for this notifier's configuration
      */
     Properties buildSessionProperties() {
@@ -338,6 +394,10 @@ Properties buildSessionProperties() {
             props.put("mail.smtp.starttls.required", "true");
         }
 
+        if (implicitTLS || useTLS) {
+            props.put("mail.smtp.ssl.checkserveridentity", String.valueOf(verifyServerIdentity));
+        }
+
         return props;
     }
 

src/main/java/com/dansplugins/herald/Herald.java

@@ -116,6 +116,11 @@ private void loadConfiguration() {
             // Defaults to false so that a config.yml written before the key existed keeps
             // using STARTTLS on the submission port it was set up against.
             boolean implicitTLS = getConfig().getBoolean("smtp.implicit-tls", false);
+            // Defaults to true, unlike the two keys above, because it tightens rather than
+            // switches: a config.yml written before the key existed was already asking for
+            // an encrypted connection to a named host, and checking the certificate against
+            // that name is what it was asking for.
+            boolean verifyServerIdentity = getConfig().getBoolean("smtp.verify-server-identity", true);
             String emailSubject = getConfig().getString("email.subject");
             String emailBody = getConfig().getString("email.body");
 
@@ -135,7 +140,7 @@ private void loadConfiguration() {
 
             if (tlsModeConflict == null && emailProblems.isEmpty()) {
                 notifiers.add(new EmailNotifier(smtpServer, smtpPort, smtpUsername, smtpPassword, emailSender, useTLS,
-                        implicitTLS, emailRecipients, emailSubject, emailBody));
+                        implicitTLS, verifyServerIdentity, emailRecipients, emailSubject, emailBody));
                 getLogger().info("Email notifications enabled");
 
                 // Warned rather than refused: the combination still delivers mail, and an
@@ -153,6 +158,15 @@ private void loadConfiguration() {
                 if (portMismatch != null) {
                     getLogger().warning(portMismatch);
                 }
+
+                // Warned for the same reason as the two above: the opt-out exists for relays
+                // whose certificate cannot name the address they are reached at, so it is a
+                // choice to be recorded in the log rather than a configuration to refuse.
+                String unverifiedIdentity = EmailNotifier.describeUnverifiedServerIdentity(
+                        verifyServerIdentity, useTLS, implicitTLS);
+                if (unverifiedIdentity != null) {
+                    getLogger().warning(unverifiedIdentity);
+                }
             } else if (emailPartiallyConfigured && !emailProblems.isEmpty()) {
                 // Logged even when a mode conflict was reported above, so that a config file
                 // with both problems names both at once instead of surfacing the second one

src/main/resources/config.yml

@@ -41,6 +41,11 @@ smtp:
   implicit-tls: false  # Set to true, with use-tls false, for a server that expects TLS before
                        # the first command - SMTPS, usually port 465. The two are alternatives:
                        # setting both to true is reported at startup and skips email.
+  # Whether the certificate the server presents is checked against the address in
+  # 'server' above. Applies to both encryption modes and is ignored when neither is
+  # on. Set to false only for a server whose certificate cannot name the address it
+  # is reached at; the connection stays encrypted, but with whoever answered.
+  verify-server-identity: true
 
 email:
   enabled: true        # Set to false to disable email notifications, keeping the settings above and below