diff --git a/docs/content/docs/guides/mailbox-import.mdx b/docs/content/docs/guides/mailbox-import.mdx index 0d7da16c9..237aa4010 100644 --- a/docs/content/docs/guides/mailbox-import.mdx +++ b/docs/content/docs/guides/mailbox-import.mdx @@ -410,6 +410,7 @@ Failures are grouped by cause, each with the fix. The same keys appear as `cause | `auth_refused` | The server refused the sign-in | | `host_not_found` | The server name does not exist | | `host_unreachable` | The server could not be reached | +| `private_host` | The server is on a private network | | `port_blocked` | The mail port looks blocked | | `tls_failed` | The secure connection failed | | `cleartext_refused` | Encryption is required | diff --git a/docs/content/docs/guides/mailboxes.mdx b/docs/content/docs/guides/mailboxes.mdx index c468ccf07..005612c74 100644 --- a/docs/content/docs/guides/mailboxes.mdx +++ b/docs/content/docs/guides/mailboxes.mdx @@ -151,7 +151,7 @@ The same holds for a server that answers but refuses what Warmbly asked. The dra With two-factor authentication on, generate an app password in your provider's security settings and use that. -Warmbly signs in with whichever method your server offers, preferring LOGIN, then PLAIN, and using CRAM-MD5 only when the server offers neither. Every one of them is sent over the established TLS connection, never before it: a server that offers to sign in without encryption is refused with an error that says so, except a relay on the worker's own machine with the **None** mode. Servers that accept only one of these, which includes Microsoft 365 relays and most appliance relays, work without any setting to change. +Warmbly signs in with whichever method your server offers, preferring LOGIN, then PLAIN, and using CRAM-MD5 only when the server offers neither. Every one of them is sent over the established TLS connection, never before it: a server that offers to sign in without encryption is refused with an error that says so and asks for the server's SSL/TLS port (usually 465) or a port that offers STARTTLS (usually 587), except a relay on the worker's own machine with the **None** mode. A server that offers none of the three methods is refused with the methods it does offer, so you can tell whether another host or port is needed. Servers that accept only one of these, which includes Microsoft 365 relays and most appliance relays, work without any setting to change. ### Servers with their own certificate @@ -161,6 +161,7 @@ When that happens the form offers **Trust this server's own certificate**. It is - A server whose certificate does verify keeps normal checking even when you ticked the box: the choice only applies to a server that needs it, and it switches itself off if the server later gets a certificate from an authority - Changing the password or username of the same server keeps the choice; moving to another host name asks again +- A connected mailbox whose server stops presenting a verifiable certificate (expired, or for another host name) shows **Server certificate not verified** in the drawer. Correct the host name with **Update credentials**, or trust the server's own certificate when the test offers it - When the certificate changes, for example when it is renewed with a new key, the mailbox stops connecting and shows **Server certificate changed** in the drawer. It is not deactivated and its password is not marked wrong. Open **Settings > Server certificates** on the mailbox and choose **Re-confirm**: Warmbly tests the servers, shows what they present now, and pins the new fingerprints once you confirm - **Settings > Server certificates** always shows, per server, whether it is verified by a certificate authority or which fingerprint it is trusted on @@ -224,7 +225,7 @@ Both are about the same thing: the credentials never leave the machine, so there Set the Bridge's own username and password, which it shows in its interface, not your Proton account password. The same applies to any relay on the machine: a Dovecot sharing a host with its worker, or a local sink used for testing. -More generally, hosted Warmbly connects only to mail servers at public addresses, checked against the address each connection actually reaches. A self-hosted instance can reach servers on its own network, and an operator can refuse those too with [`MAIL_ALLOW_PRIVATE_HOSTS=false`](/development/configuration/#workers). +More generally, hosted Warmbly connects only to mail servers at public addresses, checked against the address each connection actually reaches. A self-hosted instance can reach servers on its own network, and an operator can refuse those too with [`MAIL_ALLOW_PRIVATE_HOSTS=false`](/development/configuration/#workers). A refused server fails the connection test, and a connected mailbox shows **Server address is not public** in the drawer, both saying the host name points to a private network address. Use the host name your provider gives for connecting from outside your network; on a self-hosted instance the message also says the operator can allow private addresses. diff --git a/internal/app/email/certificates.go b/internal/app/email/certificates.go index b97c2e202..8c5c5c238 100644 --- a/internal/app/email/certificates.go +++ b/internal/app/email/certificates.go @@ -2,6 +2,7 @@ package email import ( "context" + "strings" "github.com/google/uuid" "github.com/rs/zerolog/log" @@ -107,6 +108,9 @@ func (s *emailService) ConfirmMailboxCertificates(ctx context.Context, orgID *uu } // The worker refuses any certificate other than the confirmed pin. if xerr := s.checkCredentials(ctx, *orgID, account.WorkerID, creds); xerr != nil { + if xerr.Identifier == ErrIDMailboxTLSFailed && strings.Contains(xerr.Message, models.MailProbeDetailPinMismatch) { + return nil, nil, errx.ErrEmailCertificatePinMismatch + } return nil, nil, xerr } if err := s.emailRepository.ReplaceSMTPIMAPCredentials(ctx, accountID, creds); err != nil { diff --git a/internal/app/email/validation_verdict.go b/internal/app/email/validation_verdict.go index f2600c74e..1029e12bc 100644 --- a/internal/app/email/validation_verdict.go +++ b/internal/app/email/validation_verdict.go @@ -2,8 +2,10 @@ package email import ( "fmt" + "slices" "strings" + "github.com/warmbly/warmbly/internal/config" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/pkg/mailcause" @@ -71,6 +73,9 @@ func validationError(v models.EmailValidationVerdict, creds *models.SmtpImap) *e if hint := providerHint(lead); hint != "" { msg += " " + hint } + for _, hint := range connectionHints(legs) { + msg += " " + hint + } id := ErrIDMailboxServerDeclined switch lead.res.Reason { @@ -81,7 +86,11 @@ func validationError(v models.EmailValidationVerdict, creds *models.SmtpImap) *e case models.MailProbeTLS: id = ErrIDMailboxTLSFailed } - return withCause(errx.NewWithIdentifier(errx.BadRequest, id, msg), v, creds, lead) + e := errx.NewWithIdentifier(errx.BadRequest, id, msg) + if id == ErrIDMailboxTLSFailed && slices.ContainsFunc(legs, untrustedCertificate) { + e.Reason = ReasonCertificateUntrusted + } + return withCause(e, v, creds, lead) } // withCause attaches the grouping key an import reads; a single connect ignores it. @@ -183,6 +192,43 @@ func presented(cert *models.MailCertificate) string { return s + " with SHA-256 key fingerprint " + cert.Fingerprint + "." } +// ReasonCertificateUntrusted is the response reason on a mailbox_tls_failed the owner can resolve by trusting the server's own certificate. +const ReasonCertificateUntrusted = "certificate_untrusted" + +// TrustCertificateHint follows a failure on a certificate no authority vouches for. +const TrustCertificateHint = "If this is your own server with a self-signed or company certificate, compare that fingerprint with the one its owner gives you, then test again trusting the server's own certificate (tls_unverified in the API)." + +// untrustedCertificate reports a leg that failed on a certificate no authority vouches for and that is not already trusted on its own. +func untrustedCertificate(l failedLeg) bool { + return l.res.Reason == models.MailProbeTLS && l.res.Certificate != nil && !l.res.Certificate.Trusted && + l.res.Certificate.Fingerprint != "" && (l.svc == nil || !l.svc.TLSUnverified) +} + +// connectionHints says what to do about failures the server's own words do not explain. +func connectionHints(legs []failedLeg) []string { + var private, untrusted bool + for _, l := range legs { + switch { + case l.res.Reason == models.MailProbeUnreachable && l.res.Detail == models.MailProbeDetailPrivateHost: + private = true + case untrustedCertificate(l): + untrusted = true + } + } + var hints []string + if private { + hint := "Use the host name your provider gives for connecting from outside your network." + if config.SelfHosted() { + hint += " To connect mailboxes on a private network, the instance's operator can set MAIL_ALLOW_PRIVATE_HOSTS=true." + } + hints = append(hints, hint) + } + if untrusted { + hints = append(hints, TrustCertificateHint) + } + return hints +} + // failedLeg is one side of a verdict that did not pass, with the service it // was about. type failedLeg struct { diff --git a/internal/app/email/validation_verdict_test.go b/internal/app/email/validation_verdict_test.go index ad11c0738..48b71e423 100644 --- a/internal/app/email/validation_verdict_test.go +++ b/internal/app/email/validation_verdict_test.go @@ -144,3 +144,40 @@ func TestValidationError_NamesTheFallbackPort(t *testing.T) { t.Fatalf("got %q", xerr.Message) } } + +// TestValidationError_PrivateHost: the operator setting is named only where an operator can change it. +func TestValidationError_PrivateHost(t *testing.T) { + leg := models.EmailValidationLeg{Reason: models.MailProbeUnreachable, Detail: models.MailProbeDetailPrivateHost} + verdict := models.EmailValidationVerdict{SMTP: leg, IMAP: leg} + creds := &models.SmtpImap{SMTP: &models.Service{Host: "mail.corp.example", Port: 587}, IMAP: &models.Service{Host: "mail.corp.example", Port: 993}} + + t.Setenv("DEPLOYMENT_MODE", "cloud") + xerr := validationError(verdict, creds) + if xerr.Identifier != ErrIDMailboxUnreachable || !strings.Contains(xerr.Message, "outside your network") || strings.Contains(xerr.Message, "MAIL_ALLOW_PRIVATE_HOSTS") { + t.Fatalf("hosted: %q", xerr.Message) + } + if strings.Count(xerr.Message, "outside your network") != 1 { + t.Fatalf("hint repeated per leg: %q", xerr.Message) + } + + t.Setenv("DEPLOYMENT_MODE", "self_hosted") + if xerr := validationError(verdict, creds); !strings.Contains(xerr.Message, "MAIL_ALLOW_PRIVATE_HOSTS=true") { + t.Fatalf("self-hosted: %q", xerr.Message) + } +} + +// TestValidationError_UntrustedCertificate: a certificate no authority vouches for offers the trust option, a missing STARTTLS does not. +func TestValidationError_UntrustedCertificate(t *testing.T) { + ok := models.EmailValidationLeg{OK: true} + cert := &models.MailCertificate{Fingerprint: strings.Repeat("ab", 32), Subject: "mail.corp.example", SelfSigned: true} + untrusted := validationError(models.EmailValidationVerdict{SMTP: ok, IMAP: models.EmailValidationLeg{ + Reason: models.MailProbeTLS, Detail: "the server's certificate is not one a recognised certificate authority vouches for", Certificate: cert}}, gmailCreds()) + if untrusted.Identifier != ErrIDMailboxTLSFailed || untrusted.Reason != ReasonCertificateUntrusted || !strings.Contains(untrusted.Message, TrustCertificateHint) { + t.Fatalf("untrusted: %q", untrusted.Message) + } + noTLS := validationError(models.EmailValidationVerdict{IMAP: ok, SMTP: models.EmailValidationLeg{ + Reason: models.MailProbeTLS, Detail: "the server offers no STARTTLS on this port"}}, gmailCreds()) + if strings.Contains(noTLS.Message, TrustCertificateHint) || noTLS.Reason != "" { + t.Fatalf("no STARTTLS offered the trust option: %q", noTLS.Message) + } +} diff --git a/internal/app/worker/event_send_email.go b/internal/app/worker/event_send_email.go index 9a89c90dc..b36841a9f 100644 --- a/internal/app/worker/event_send_email.go +++ b/internal/app/worker/event_send_email.go @@ -360,8 +360,8 @@ func (w *WorkerService) failSend(ctx context.Context, sendEmail models.SendEmail // Fixed reasons for a send the worker could not open; they never carry the underlying error. const ( - sendReasonKeyUnavailable = "The workspace encryption key could not be loaded, so nothing was sent." - sendReasonUndecryptable = "The message could not be decrypted, so nothing was sent." + sendReasonKeyUnavailable = "Nothing was sent because Warmbly could not load this workspace's encryption key just now. This is a problem on Warmbly's side, not with the mailbox or the recipient. A campaign tries the step again on its own; send a single email again in a few minutes. If it keeps happening, contact Warmbly support, or your instance's operator if Warmbly is self-hosted." + sendReasonUndecryptable = "Nothing was sent because Warmbly could not read the stored message. This is a problem on Warmbly's side, not with the mailbox or the recipient. Contact Warmbly support, or your instance's operator if Warmbly is self-hosted, and mention when it failed." ) var ( diff --git a/internal/client/mailtls/mailtls.go b/internal/client/mailtls/mailtls.go index 271092f9b..c10916c6f 100644 --- a/internal/client/mailtls/mailtls.go +++ b/internal/client/mailtls/mailtls.go @@ -227,3 +227,12 @@ func AsPinError(err error, capture *Capture) *PinError { } return nil } + +// CertificateRefused reports whether err is a certificate that failed CA, host name or validity checks. +func CertificateRefused(err error) bool { + var verr *tls.CertificateVerificationError + var unknown x509.UnknownAuthorityError + var invalid x509.CertificateInvalidError + var host x509.HostnameError + return errors.As(err, &verr) || errors.As(err, &unknown) || errors.As(err, &invalid) || errors.As(err, &host) +} diff --git a/internal/client/netbind/netbind.go b/internal/client/netbind/netbind.go index 3ca2fc50d..2e7aaf7b8 100644 --- a/internal/client/netbind/netbind.go +++ b/internal/client/netbind/netbind.go @@ -108,9 +108,9 @@ func InsecureTLS() bool { insecureTLS = config.MailTLSInsecureAllowed() switch { case insecureTLS: - log.Warn().Msg("MAIL_TLS_INSECURE=true: mailbox certificates are not verified and SMTP may skip STARTTLS") + log.Warn().Msg("MAIL_TLS_INSECURE=true: mailbox certificates are not verified and SMTP may skip STARTTLS. Unset it to verify them; a mailbox on a server with its own certificate can trust that server in its own settings instead") case set: - log.Error().Msg("MAIL_TLS_INSECURE=true is ignored on hosted production: mailbox certificates are verified unless the mailbox records its own opt-out") + log.Error().Msg("MAIL_TLS_INSECURE=true is ignored on hosted production: mailbox certificates are verified, and a mailbox on a server with its own certificate trusts it in its own settings. Remove the variable from this node's environment") } }) return insecureTLS diff --git a/internal/client/smtpimap/imap/err.go b/internal/client/smtpimap/imap/err.go index de54eba83..c08ea98bf 100644 --- a/internal/client/smtpimap/imap/err.go +++ b/internal/client/smtpimap/imap/err.go @@ -12,11 +12,15 @@ import ( "github.com/emersion/go-imap/v2" "github.com/rs/zerolog/log" + "github.com/warmbly/warmbly/internal/client/netbind" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" ) func connectionFailureReason(err error) string { + if errors.Is(err, netbind.ErrPrivateMailHost) { + return errx.MailConnectionReasonPrivateHost + } var dns *net.DNSError if errors.As(err, &dns) { if dns.IsTimeout { diff --git a/internal/client/smtpimap/imap/err_test.go b/internal/client/smtpimap/imap/err_test.go index 7a929e4a0..a797b921d 100644 --- a/internal/client/smtpimap/imap/err_test.go +++ b/internal/client/smtpimap/imap/err_test.go @@ -12,6 +12,7 @@ import ( "testing" goimap "github.com/emersion/go-imap/v2" + "github.com/warmbly/warmbly/internal/client/netbind" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" ) @@ -35,6 +36,7 @@ func TestConnectionFailureReasonDoesNotExposeTransportDetails(t *testing.T) { {"closed_socket", net.ErrClosed, "connection closed"}, {"timeout", context.DeadlineExceeded, "connection timed out"}, {"op_timeout", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ETIMEDOUT}, "connection timed out"}, + {"private_host", &net.OpError{Op: "dial", Net: "tcp", Err: netbind.ErrPrivateMailHost}, errx.MailConnectionReasonPrivateHost}, {"other", errors.New("secret.invalid password=secret"), "connection failed"}, } { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/client/smtpimap/smtp/client.go b/internal/client/smtpimap/smtp/client.go index 97d7eb5e4..9b6714509 100644 --- a/internal/client/smtpimap/smtp/client.go +++ b/internal/client/smtpimap/smtp/client.go @@ -34,7 +34,15 @@ import ( const SendTimeout = 90 * time.Second // errNoSTARTTLS distinguishes a missing upgrade from a network failure. -var errNoSTARTTLS = errors.New("the server does not offer STARTTLS") +var errNoSTARTTLS = errors.New(errx.MailConnectionReasonNoSTARTTLS) + +// tlsFailure reports a handshake that failed for a reason other than a pin. +func tlsFailure(base *errx.MailError, stage string, err error) *errx.MailError { + if mailtls.CertificateRefused(err) { + base = errx.ErrMailServerUnreachableAt(stage, errors.New(errx.MailConnectionReasonCertificate)) + } + return sendFailure(base, err, stage, "mailbox") +} // pinFailure reports a pinned certificate refusal as a retryable mailbox // connection error, never as a credentials failure. @@ -361,7 +369,11 @@ func (c *Client) sendRaw(ctx context.Context, from string, to []string, data []b if err == nil { err = errors.New("dial returned no connection") } - return sendFailure(errx.ErrMailServerUnreachableAt("dial "+addr, dialCause(err)), err, "dial", "mailbox") + cause := dialCause(err) + if errors.Is(err, netbind.ErrPrivateMailHost) { + cause = errors.New(errx.MailConnectionReasonPrivateHost) + } + return sendFailure(errx.ErrMailServerUnreachableAt("dial "+addr, cause), err, "dial", "mailbox") } conn := dialed.Conn defer conn.Close() @@ -382,7 +394,7 @@ func (c *Client) sendRaw(ctx context.Context, from string, to []string, data []b if perr := mailtls.AsPinError(err, capture); perr != nil { return pinFailure(perr) } - return sendFailure(errx.ErrMailServerUnreachableAt("dial "+addr, err), err, "dial", "mailbox") + return tlsFailure(errx.ErrMailServerUnreachableAt("dial "+addr, err), "dial", err) } conn = tlsConn } @@ -407,14 +419,14 @@ func (c *Client) sendRaw(ctx context.Context, from string, to []string, data []b // even when the relay advertises it: a self-signed certificate no client // can verify is exactly why the mode was chosen. The MAIL_TLS_INSECURE // dev knob additionally allows a server with no STARTTLS at all (the - // local mailpit sink); it is honoured only under APP_ENV=dev. + // local mailpit sink); it is honoured on self-hosted and development instances. if !implicitTLS && resolved != models.MailSecurityNone { if ok, _ := client.Extension("STARTTLS"); ok { if err := client.StartTLS(tlsConf); err != nil { if perr := mailtls.AsPinError(err, capture); perr != nil { return pinFailure(perr) } - return sendFailure(errx.ErrMailServerUnreachable, err, "starttls", "mailbox") + return tlsFailure(errx.ErrMailServerUnreachable, "starttls", err) } } else if !netbind.InsecureTLS() && !c.plaintext { return sendFailure(errx.ErrMailServerUnreachableAt("starttls", errNoSTARTTLS), errNoSTARTTLS, "starttls", "mailbox") diff --git a/internal/client/smtpimap/smtp/unreachable_detail_test.go b/internal/client/smtpimap/smtp/unreachable_detail_test.go index 4eadf7d84..28ffc35b0 100644 --- a/internal/client/smtpimap/smtp/unreachable_detail_test.go +++ b/internal/client/smtpimap/smtp/unreachable_detail_test.go @@ -87,7 +87,7 @@ func TestUnreachableSaysWhenTheServerOffersNoSTARTTLS(t *testing.T) { if string(err.Code) != "SERVER_UNREACHABLE" { t.Fatalf("code = %q, want SERVER_UNREACHABLE", err.Code) } - if !strings.Contains(err.Message, "starttls") || !strings.Contains(err.Message, "does not offer STARTTLS") { + if !strings.Contains(err.Message, "starttls") || !strings.Contains(err.Message, "offers no STARTTLS on this port") { t.Errorf("message %q does not explain the missing STARTTLS", err.Message) } } @@ -134,3 +134,23 @@ func TestDialCausePassesThroughAnythingElse(t *testing.T) { t.Fatalf("dialCause rewrote a non-OpError: %v", got) } } + +// A private address is named as such, with what to use instead. +func TestUnreachableSaysWhenTheHostIsPrivate(t *testing.T) { + t.Setenv("DEPLOYMENT_MODE", "cloud") + t.Setenv("APP_ENV", "prod") + srv := newFakeServer(t, "LOGIN") + host, port := srv.addr() + c := newTestClient(host, port) + + mailErr := c.sendRaw(t.Context(), "sender@warmbly.test", []string{"to@example.test"}, []byte("Subject: hi\r\n\r\nbody\r\n")) + if mailErr == nil { + t.Fatal("send succeeded against a loopback server on a hosted instance") + } + if string(mailErr.Code) != "SERVER_UNREACHABLE" || !strings.Contains(mailErr.Message, "private network address") { + t.Fatalf("got %s %q", mailErr.Code, mailErr.Message) + } + if info := mailErr.GetUserErrorInfo(); info.Title != "Server address is not public" { + t.Fatalf("title = %q", info.Title) + } +} diff --git a/internal/email/probe.go b/internal/email/probe.go index 69c9c8a82..b851391d4 100644 --- a/internal/email/probe.go +++ b/internal/email/probe.go @@ -2,6 +2,7 @@ package email import ( "context" + "crypto/x509" "errors" "io" "net" @@ -13,6 +14,7 @@ import ( "github.com/emersion/go-imap/v2" "github.com/warmbly/warmbly/internal/client/mailtls" + "github.com/warmbly/warmbly/internal/client/netbind" "github.com/warmbly/warmbly/internal/models" ) @@ -42,15 +44,35 @@ func tlsFail(ctx context.Context, err error, capture *mailtls.Capture) ProbeResu if perr := mailtls.AsPinError(err, capture); perr != nil { res.Reason = models.MailProbeTLS if perr.Kind == mailtls.PinMissing { - res.Detail = "the server's certificate has not been confirmed for this mailbox" + res.Detail = "the server's certificate has not been confirmed for this mailbox. Confirm it under Settings > Server certificates on the mailbox" } else { - res.Detail = "the server's certificate does not match the one confirmed for this mailbox" + res.Detail = models.MailProbeDetailPinMismatch + ". If the server's owner replaced it, re-confirm it under Settings > Server certificates on the mailbox" } + } else if detail := certificateDetail(err); detail != "" { + res.Reason, res.Detail = models.MailProbeTLS, detail } res.Certificate = capture.Certificate(err) return res } +// certificateDetail says in plain words why a certificate did not verify, or "" when err is not about one. +func certificateDetail(err error) string { + var host x509.HostnameError + var invalid x509.CertificateInvalidError + var unknown x509.UnknownAuthorityError + switch { + case errors.As(err, &host): + return "the server's certificate is issued for a different host name. Use the host name the certificate names, usually the one in your provider's documentation" + case errors.As(err, &invalid) && invalid.Reason == x509.Expired: + return "the server's certificate has expired or is not valid yet" + case errors.As(err, &unknown), errors.As(err, &invalid): + return "the server's certificate is not one a recognised certificate authority vouches for, which is normal for a self-signed or company certificate" + case mailtls.CertificateRefused(err): + return "the server's certificate could not be verified" + } + return "" +} + func probeOK() ProbeResult { return ProbeResult{OK: true} } // probeFail classifies err. A deadline wins over the given reason, because a @@ -79,6 +101,9 @@ func isServerReply(err error) bool { // both ends of the socket when the worker binds a local address, and the // worker's address is not the customer's to see. func netDetail(err error) string { + if errors.Is(err, netbind.ErrPrivateMailHost) { + return models.MailProbeDetailPrivateHost + } var dnsErr *net.DNSError if errors.As(err, &dnsErr) { if dnsErr.IsNotFound { diff --git a/internal/email/probe_test.go b/internal/email/probe_test.go index 07322b164..f3c74e6d1 100644 --- a/internal/email/probe_test.go +++ b/internal/email/probe_test.go @@ -2,12 +2,17 @@ package email import ( "context" + "crypto/tls" + "crypto/x509" "errors" + "fmt" "net" "net/textproto" + "strings" "testing" "time" + "github.com/warmbly/warmbly/internal/client/netbind" "github.com/warmbly/warmbly/internal/models" ) @@ -45,3 +50,32 @@ func TestProbeFail_SocketErrorsAreClosedWords(t *testing.T) { t.Fatalf("got %+v", res) } } + +// A refused private address is reported as that, not as a failed connection. +func TestProbeFail_PrivateHostIsNamed(t *testing.T) { + err := &net.OpError{Op: "dial", Net: "tcp", Err: fmt.Errorf("dial 10.0.0.5:587: %w", netbind.ErrPrivateMailHost)} + res := probeFail(context.Background(), models.MailProbeUnreachable, err) + if res.Reason != models.MailProbeUnreachable || res.Detail != models.MailProbeDetailPrivateHost { + t.Fatalf("got %+v", res) + } +} + +// Each certificate refusal says why in plain words. +func TestCertificateDetail(t *testing.T) { + for _, tc := range []struct { + err error + want string + }{ + {x509.HostnameError{Certificate: &x509.Certificate{}, Host: "mail.example.org"}, "different host name"}, + {x509.CertificateInvalidError{Reason: x509.Expired}, "expired"}, + {x509.UnknownAuthorityError{}, "self-signed or company certificate"}, + {&tls.CertificateVerificationError{Err: x509.UnknownAuthorityError{}}, "self-signed or company certificate"}, + } { + if got := certificateDetail(tc.err); !strings.Contains(got, tc.want) { + t.Errorf("certificateDetail(%T) = %q, want %q", tc.err, got, tc.want) + } + } + if got := certificateDetail(errors.New("tls: first record does not look like a TLS handshake")); got != "" { + t.Errorf("non-certificate error described as a certificate: %q", got) + } +} diff --git a/internal/email/smtp.go b/internal/email/smtp.go index 1922a6531..4ae15cd6f 100644 --- a/internal/email/smtp.go +++ b/internal/email/smtp.go @@ -117,7 +117,7 @@ func verifySMTPSession(ctx context.Context, conn net.Conn, resolved, host, user, return tlsFail(ctx, err, capture) } } else if !netbind.InsecureTLS() { - return probeFailText(models.MailProbeTLS, "the server offers no STARTTLS on this port") + return probeFailText(models.MailProbeTLS, "the server offers no STARTTLS on this port, and Warmbly only signs in over an encrypted connection. Use the server's SSL/TLS port (usually 465) or a port that offers STARTTLS (usually 587)") } } @@ -126,7 +126,10 @@ func verifySMTPSession(ctx context.Context, conn net.Conn, resolved, host, user, // PLAIN, and no password mechanism runs outside TLS or loopback. auth, aerr := wsmtp.NegotiateAuth(c, user, pass, host) if errors.Is(aerr, wsmtp.ErrSMTPCleartextAuth) { - return probeFail(ctx, models.MailProbeCleartext, aerr) + return probeFailText(models.MailProbeCleartext, "the server only offers sign-in over an unencrypted connection, and Warmbly never sends a password unencrypted. Use the server's SSL/TLS port (usually 465) or a port that offers STARTTLS (usually 587)") + } + if errors.Is(aerr, wsmtp.ErrSMTPNoSupportedAuth) { + return probeFailText(models.MailProbeProtocol, noSupportedAuthDetail(c)) } if aerr != nil { return probeFail(ctx, models.MailProbeProtocol, aerr) @@ -182,3 +185,12 @@ func ehloName(user string) string { } return "localhost" } + +// noSupportedAuthDetail names the sign-in methods the server offered when none is one Warmbly speaks. +func noSupportedAuthDetail(c *smtp.Client) string { + detail := "the server offers no sign-in method Warmbly supports (Warmbly signs in with LOGIN, PLAIN or CRAM-MD5)" + if _, offered := c.Extension("AUTH"); offered != "" { + detail = "the server offers no sign-in method Warmbly supports: it offers " + probeDetail(errors.New(offered)) + ", and Warmbly signs in with LOGIN, PLAIN or CRAM-MD5" + } + return detail + ". Use the provider's documented SMTP submission host, which usually offers one of them" +} diff --git a/internal/errx/common.go b/internal/errx/common.go index 2ac8f605d..353eb4a1b 100644 --- a/internal/errx/common.go +++ b/internal/errx/common.go @@ -25,6 +25,9 @@ var ( // Authorization ErrToken = New(Unauthorized, "Invalid or expired token.") ErrAuth = New(Unauthorized, "Missing or invalid Authorization header.") + // ErrSessionEnded answers a token whose session was signed out or reached + // its absolute lifetime; only a new sign-in continues. + ErrSessionEnded = NewWithIdentifier(Unauthorized, "session_ended", "Your session has ended. Sign in again to continue.") ErrUser = New(BadRequest, "User doesn't exists.") ErrPassword = New(BadRequest, "Password must be at least 8 characters long.") @@ -170,32 +173,33 @@ var ( // The "none" refusals are two different problems with two different ways // out, so they do not share a message: one is a host that has to change, // the other is a mode this deployment cannot offer at all. - ErrEmailSMTPSecurityNotLocal = New(BadRequest, "SMTP security \"none\" is only allowed for a mail server on this machine (localhost, 127.0.0.1 or ::1).") - ErrEmailIMAPSecurityNotLocal = New(BadRequest, "IMAP security \"none\" is only allowed for a mail server on this machine (localhost, 127.0.0.1 or ::1).") - ErrEmailSMTPSecurityHosted = New(BadRequest, "SMTP security \"none\" needs a self-hosted instance, where the worker runs on the same machine as the mail server. Use tls or starttls.") - ErrEmailIMAPSecurityHosted = New(BadRequest, "IMAP security \"none\" needs a self-hosted instance, where the worker runs on the same machine as the mail server. Use tls or starttls.") - ErrEmailSMTPTLSPin = NewWithIdentifier(BadRequest, "invalid_tls_pin", "SMTP tls_pin must be a SHA-256 fingerprint: 64 hexadecimal characters.") - ErrEmailIMAPTLSPin = NewWithIdentifier(BadRequest, "invalid_tls_pin", "IMAP tls_pin must be a SHA-256 fingerprint: 64 hexadecimal characters.") - ErrEmailCertificateNotPinned = NewWithIdentifier(BadRequest, "mailbox_certificate_not_pinned", "Neither server on this mailbox is trusted on a confirmed certificate, so there is nothing to confirm.") - ErrEmailCertificatePin = NewWithIdentifier(BadRequest, "invalid_tls_pin", "Each server trusted on a confirmed certificate needs its SHA-256 fingerprint (smtp_pin, imap_pin): 64 hexadecimal characters.") - ErrEmailIMAPHost = New(BadRequest, "IMAP host is required.") - ErrEmailIMAPPort = New(BadRequest, "IMAP port must be between 1 and 65535.") - ErrEmailCredentialsRequired = New(BadRequest, "SMTP and IMAP credentials are required.") - ErrEmailTrackingDomain = New(BadRequest, "Invalid tracking domain.") - ErrEmailTrackingDomainLength = New(BadRequest, "Tracking domain is too long (max 253 characters).") - ErrEmailName = New(BadRequest, "Invalid name. Must be 2–100 characters and contain only letters, numbers, spaces, '-', '.', or '’'.") - ErrEmailSignaturePlain = New(BadRequest, "Plain email signature is too long.") - ErrEmailSignatureHTML = New(BadRequest, "HTML email signature is too long.") - ErrEmailMinWaitTime = New(BadRequest, "Minimum time gap between emails must be between 0 and 86400 seconds.") - ErrEmailCampaignLimit = New(BadRequest, fmt.Sprintf("Campaign limit must be between %d and %d.", config.LimitMin, config.LimitMax)) - ErrEmailTimezone = New(BadRequest, "Invalid timezone. Use an IANA name such as Europe/London or America/Denver, or leave it empty to follow the campaign.") - ErrEmailWarmupBase = New(BadRequest, "Warmup base must be between 0 and 100.") - ErrEmailWarmupMax = New(BadRequest, "Warmup max amount must be between 0 and 100.") - ErrEmailWarmupIncrease = New(BadRequest, "Warmup increase amount must be between 0 and 100.") - ErrEmailReplyRate = New(BadRequest, "Warmup reply rate must be between 0 and 100.") - ErrEmailWarmupPlacement = New(BadRequest, "Warmup filing must be one of: folder, inbox, archive.") - ErrEmailWarmupFolder = New(BadRequest, fmt.Sprintf("Warmup folder must be at most %d characters and contain no folder separators or control characters.", config.WarmupFolderMaxLen)) - ErrEmailWarmupRetention = New(BadRequest, fmt.Sprintf("Warmup retention must be between %d and %d days, or 0 to follow the instance setting.", config.WarmupMailRetentionDaysMin, config.RetentionDaysMax)) + ErrEmailSMTPSecurityNotLocal = New(BadRequest, "SMTP security \"none\" is only allowed for a mail server on this machine (localhost, 127.0.0.1 or ::1).") + ErrEmailIMAPSecurityNotLocal = New(BadRequest, "IMAP security \"none\" is only allowed for a mail server on this machine (localhost, 127.0.0.1 or ::1).") + ErrEmailSMTPSecurityHosted = New(BadRequest, "SMTP security \"none\" needs a self-hosted instance, where the worker runs on the same machine as the mail server. Use tls or starttls.") + ErrEmailIMAPSecurityHosted = New(BadRequest, "IMAP security \"none\" needs a self-hosted instance, where the worker runs on the same machine as the mail server. Use tls or starttls.") + ErrEmailSMTPTLSPin = NewWithIdentifier(BadRequest, "invalid_tls_pin", "SMTP tls_pin must be a SHA-256 fingerprint: 64 hexadecimal characters.") + ErrEmailIMAPTLSPin = NewWithIdentifier(BadRequest, "invalid_tls_pin", "IMAP tls_pin must be a SHA-256 fingerprint: 64 hexadecimal characters.") + ErrEmailCertificateNotPinned = NewWithIdentifier(BadRequest, "mailbox_certificate_not_pinned", "Both servers on this mailbox are verified by a certificate authority, so there is no certificate to confirm.") + ErrEmailCertificatePin = NewWithIdentifier(BadRequest, "invalid_tls_pin", "Send the SHA-256 fingerprint of each server trusted on its own certificate (smtp_pin, imap_pin) as 64 hexadecimal characters. Run the certificate check first and confirm the fingerprints it reports.") + ErrEmailCertificatePinMismatch = NewWithIdentifier(BadRequest, "mailbox_tls_failed", "The server did not present the certificate with the fingerprint you confirmed, so nothing was changed. Run the certificate check again (Re-confirm in the dashboard) and confirm the fingerprint it reports.") + ErrEmailIMAPHost = New(BadRequest, "IMAP host is required.") + ErrEmailIMAPPort = New(BadRequest, "IMAP port must be between 1 and 65535.") + ErrEmailCredentialsRequired = New(BadRequest, "SMTP and IMAP credentials are required.") + ErrEmailTrackingDomain = New(BadRequest, "Invalid tracking domain.") + ErrEmailTrackingDomainLength = New(BadRequest, "Tracking domain is too long (max 253 characters).") + ErrEmailName = New(BadRequest, "Invalid name. Must be 2–100 characters and contain only letters, numbers, spaces, '-', '.', or '’'.") + ErrEmailSignaturePlain = New(BadRequest, "Plain email signature is too long.") + ErrEmailSignatureHTML = New(BadRequest, "HTML email signature is too long.") + ErrEmailMinWaitTime = New(BadRequest, "Minimum time gap between emails must be between 0 and 86400 seconds.") + ErrEmailCampaignLimit = New(BadRequest, fmt.Sprintf("Campaign limit must be between %d and %d.", config.LimitMin, config.LimitMax)) + ErrEmailTimezone = New(BadRequest, "Invalid timezone. Use an IANA name such as Europe/London or America/Denver, or leave it empty to follow the campaign.") + ErrEmailWarmupBase = New(BadRequest, "Warmup base must be between 0 and 100.") + ErrEmailWarmupMax = New(BadRequest, "Warmup max amount must be between 0 and 100.") + ErrEmailWarmupIncrease = New(BadRequest, "Warmup increase amount must be between 0 and 100.") + ErrEmailReplyRate = New(BadRequest, "Warmup reply rate must be between 0 and 100.") + ErrEmailWarmupPlacement = New(BadRequest, "Warmup filing must be one of: folder, inbox, archive.") + ErrEmailWarmupFolder = New(BadRequest, fmt.Sprintf("Warmup folder must be at most %d characters and contain no folder separators or control characters.", config.WarmupFolderMaxLen)) + ErrEmailWarmupRetention = New(BadRequest, fmt.Sprintf("Warmup retention must be between %d and %d days, or 0 to follow the instance setting.", config.WarmupMailRetentionDaysMin, config.RetentionDaysMax)) // Disconnecting a mailbox has to reach the machine syncing it before the // row goes: afterwards there is no assignment left to read and nothing that @@ -269,6 +273,25 @@ var ( ErrAdvisorFixForbidden = New(Forbidden, "You can see this recommendation but don't have permission to apply the change it makes.") ErrAdvisorNoAgentFix = New(BadRequest, "This recommendation needs a person: there's no change an agent can safely make for it.") + // ErrAttemptCheckUnavailable is answered when the per-account attempt + // budget cannot be read or charged. The attempt is not evaluated, so the + // caller can retry it safely once the cache answers again. + ErrAttemptCheckUnavailable = &Error{Code: ServiceUnavailable, Identifier: "attempt_check_unavailable", RetryAfter: 5, + Message: "We couldn't check this attempt right now, so it was not counted. Wait a few seconds and try again."} + // ErrCodeLookupUnavailable is the same condition for a pairing-code lookup. + ErrCodeLookupUnavailable = &Error{Code: ServiceUnavailable, Identifier: "attempt_check_unavailable", RetryAfter: 5, + Message: "We couldn't look up this code right now. Wait a few seconds and try again."} + + // Account and workspace standing. Each names what the person can do next. + ErrAccountSuspended = NewWithIdentifier(Forbidden, "account_suspended", + "This account is suspended and cannot sign in. If you think this is a mistake, contact support or the administrator of this server.") + ErrAccountSendingSuspended = NewWithIdentifier(Forbidden, "account_sending_suspended", + "This account is not allowed to send email. If you think this is a mistake, contact support or the administrator of this server.") + ErrAccountWorkspaceCreateSuspended = NewWithIdentifier(Forbidden, "account_workspace_create_suspended", + "This account is not allowed to create new workspaces. You can still join a workspace you are invited to. If you think this is a mistake, contact support or the administrator of this server.") + ErrWorkspaceSendingSuspended = NewWithIdentifier(Forbidden, "workspace_sending_suspended", + "Sending is paused for this workspace while it is under review. Contact support to resolve the review; nothing else in the workspace is affected.") + // Tracking domains ErrTrackingDomainTaken = NewWithIdentifier(Conflict, "tracking_domain_taken", "Another workspace on this instance already uses this tracking domain. Choose a host on a domain your workspace sends from.") ) diff --git a/internal/errx/email.go b/internal/errx/email.go index 178c17870..6d882fccf 100644 --- a/internal/errx/email.go +++ b/internal/errx/email.go @@ -2,6 +2,7 @@ package errx import ( "fmt" + "strings" "time" "github.com/google/uuid" @@ -173,18 +174,48 @@ const ( MailConnectionReasonClosed = "connection closed" MailConnectionReasonTimeout = "connection timed out" MailConnectionReasonFailed = "connection failed" + MailConnectionReasonPrivateHost = "private network address" + MailConnectionReasonNoSTARTTLS = "no STARTTLS offered" MailConnectionStageIMAPDial = "IMAP dial" MailConnectionStageIMAPTLS = "IMAP TLS" MailConnectionStageIMAPSTARTTLS = "IMAP STARTTLS" - mailServerUnreachableFormat = "The connection to the mail server could not be established (%s: %s). The server may be offline or blocking the connection." + mailServerUnreachablePrefix = "The connection to the mail server could not be established (%s: %s). " + mailServerUnreachableHint = "The server may be offline or blocking the connection." + // MailPrivateHostHint is what a mailbox owner reads when its server resolves to a private address. + MailPrivateHostHint = "The server's host name points to a private network address, and Warmbly only connects mailboxes to public addresses. Use the host name your provider gives for connecting from outside your network." + // MailCertificateHint is what a mailbox owner reads when its server's certificate did not verify. + MailCertificateHint = "The server's certificate is not one a recognised certificate authority vouches for, has expired, or is for another host name. Use the host name the certificate is issued for. For a server with its own certificate, choose Update credentials on the mailbox and trust the certificate when the test offers it." + // MailCleartextAuthMessage is the refusal to sign in over a link that is not encrypted. + MailCleartextAuthMessage = "This mail server only offers sign-in over an unencrypted connection, and Warmbly never sends a mailbox password unencrypted. Use the server's SSL/TLS port (usually 465) or a port that offers STARTTLS (usually 587)." + // MailNoSTARTTLSHint is what a mailbox owner reads when the port offers no encryption. + MailNoSTARTTLSHint = "The server offers no STARTTLS on this port, and Warmbly only signs in over an encrypted connection. Use the server's SSL/TLS port (usually 465) or a port that offers STARTTLS (usually 587)." ) +// connectionHints are the reasons with something more specific to say than the default. +var connectionHints = map[string]string{ + MailConnectionReasonPrivateHost: MailPrivateHostHint, + MailConnectionReasonCertificate: MailCertificateHint, + MailConnectionReasonNoSTARTTLS: MailNoSTARTTLSHint, +} + +// mailServerUnreachableMessage is the stored message for a connection failure at stage. +func mailServerUnreachableMessage(stage, reason string) string { + hint, ok := connectionHints[reason] + if !ok { + hint = mailServerUnreachableHint + } + return fmt.Sprintf(mailServerUnreachablePrefix, stage, reason) + hint +} + +// IsKnownIMAPConnectionMessage accepts each fixed IMAP connection message, +// in its current wording and the generic one earlier workers still send. func IsKnownIMAPConnectionMessage(message string) bool { for _, stage := range [...]string{MailConnectionStageIMAPDial, MailConnectionStageIMAPTLS, MailConnectionStageIMAPSTARTTLS} { for _, reason := range [...]string{MailConnectionReasonDNSFailure, MailConnectionReasonDNSTimeout, MailConnectionReasonCertificate, MailConnectionReasonWorkerAddress, MailConnectionReasonRefused, MailConnectionReasonRoute, MailConnectionReasonClosed, - MailConnectionReasonTimeout, MailConnectionReasonFailed} { - if message == fmt.Sprintf(mailServerUnreachableFormat, stage, reason) { + MailConnectionReasonTimeout, MailConnectionReasonFailed, MailConnectionReasonPrivateHost} { + if message == mailServerUnreachableMessage(stage, reason) || + message == fmt.Sprintf(mailServerUnreachablePrefix, stage, reason)+mailServerUnreachableHint { return true } } @@ -211,7 +242,7 @@ var ( return MError( MailErrorWarning, MailErrorCodeServerUnreachable, - fmt.Sprintf(mailServerUnreachableFormat, stage, cause), + mailServerUnreachableMessage(stage, cause.Error()), MailErrorResolveMethodRetry, ) } @@ -313,13 +344,13 @@ var ( ErrMailCleartextAuth = MError( MailErrorCritical, MailErrorCodeAuthUnsupported, - "This mail server offers no encrypted connection, and Warmbly will not send a mailbox password in the clear. Use the server's TLS or STARTTLS port.", + MailCleartextAuthMessage, MailErrorResolveMethodReload, ) ErrMailAuthUnsupported = MError( MailErrorCritical, MailErrorCodeAuthUnsupported, - "This mail server asks for a sign-in method Warmbly does not support. Check the server's documentation for an app password or an alternative SMTP host.", + "This mail server offers no sign-in method Warmbly supports (Warmbly signs in with LOGIN, PLAIN or CRAM-MD5). Use the provider's documented SMTP submission host, which usually offers one of them, or check the server's documentation for an app password.", MailErrorResolveMethodReload, ) ErrMailDomainAuthRejected = MError( @@ -382,6 +413,17 @@ func (e *MailError) GetUserErrorInfo() UserErrorInfo { case MailErrorCodeServerUnreachable: info.Title = "Connection Error" info.ActionRequired = "The email server is temporarily unavailable. We'll retry automatically." + switch { + case strings.Contains(e.Message, MailPrivateHostHint): + info.Title = "Server address is not public" + info.ActionRequired = "Choose Update credentials on the mailbox and enter the server's public host name" + case strings.Contains(e.Message, MailCertificateHint): + info.Title = "Server certificate not verified" + info.ActionRequired = "Choose Update credentials on the mailbox to correct the host name or trust the server's own certificate" + case strings.Contains(e.Message, MailNoSTARTTLSHint): + info.Title = "No encrypted connection offered" + info.ActionRequired = "Choose Update credentials on the mailbox and use the server's SSL/TLS port or a port that offers STARTTLS" + } case MailErrorCodeNotFound: info.Title = "Mailbox Item Missing" info.ActionRequired = "The folder or message is no longer on the mail server. Nothing to do; we'll skip it." @@ -409,6 +451,10 @@ func (e *MailError) GetUserErrorInfo() UserErrorInfo { case MailErrorCodeAuthUnsupported: info.Title = "Sign-in method not supported" info.ActionRequired = "This server asks for an authentication method Warmbly does not support. An app password, or the provider's documented SMTP host, usually works." + if strings.HasPrefix(e.Message, MailCleartextAuthMessage) { + info.Title = "Encrypted sign-in not offered" + info.ActionRequired = "Choose Update credentials on the mailbox and use the server's SSL/TLS port or a port that offers STARTTLS" + } case MailErrorCodeRecipientRejected: info.Title = "Recipient Rejected" info.ActionRequired = "The recipient address was not accepted" diff --git a/internal/errx/email_test.go b/internal/errx/email_test.go index 8a446859a..ba83ae1a8 100644 --- a/internal/errx/email_test.go +++ b/internal/errx/email_test.go @@ -2,6 +2,8 @@ package errx import ( "errors" + "fmt" + "strings" "testing" ) @@ -9,7 +11,7 @@ func TestKnownIMAPConnectionMessageRequiresExactEnvelope(t *testing.T) { for _, stage := range []string{MailConnectionStageIMAPDial, MailConnectionStageIMAPTLS, MailConnectionStageIMAPSTARTTLS} { for _, reason := range []string{MailConnectionReasonDNSFailure, MailConnectionReasonDNSTimeout, MailConnectionReasonCertificate, MailConnectionReasonWorkerAddress, MailConnectionReasonRefused, MailConnectionReasonRoute, MailConnectionReasonClosed, - MailConnectionReasonTimeout, MailConnectionReasonFailed} { + MailConnectionReasonTimeout, MailConnectionReasonFailed, MailConnectionReasonPrivateHost} { e := ErrMailServerUnreachableAt(stage, errors.New(reason)) if !IsKnownIMAPConnectionMessage(e.Message) || e.Code != MailErrorCodeServerUnreachable || e.ResolveMethod != MailErrorResolveMethodRetry { t.Fatal("known category rejected or retry semantics changed") @@ -32,3 +34,36 @@ func TestKnownIMAPConnectionMessageRequiresExactEnvelope(t *testing.T) { t.Fatal("generic factory compatibility changed") } } + +func TestUnreachableMessagesSayWhatToDo(t *testing.T) { + for _, tc := range []struct { + reason, hint, title string + }{ + {MailConnectionReasonPrivateHost, MailPrivateHostHint, "Server address is not public"}, + {MailConnectionReasonCertificate, MailCertificateHint, "Server certificate not verified"}, + {MailConnectionReasonNoSTARTTLS, MailNoSTARTTLSHint, "No encrypted connection offered"}, + } { + e := ErrMailServerUnreachableAt(MailConnectionStageIMAPDial, errors.New(tc.reason)) + if !strings.HasSuffix(e.Message, tc.hint) { + t.Fatalf("%s: message %q lacks its hint", tc.reason, e.Message) + } + // SMTP failures carry a status suffix after the hint. + e.Message += " (SMTP dial status 0)" + if info := e.GetUserErrorInfo(); info.Title != tc.title || !strings.Contains(info.ActionRequired, "Update credentials") { + t.Fatalf("%s: got %+v", tc.reason, info) + } + } + if info := ErrMailCleartextAuth.GetUserErrorInfo(); info.Title != "Encrypted sign-in not offered" { + t.Fatalf("cleartext auth: got %+v", info) + } +} + +func TestKnownIMAPConnectionMessageAcceptsEarlierWording(t *testing.T) { + legacy := fmt.Sprintf(mailServerUnreachablePrefix, MailConnectionStageIMAPTLS, MailConnectionReasonCertificate) + mailServerUnreachableHint + if !IsKnownIMAPConnectionMessage(legacy) { + t.Fatal("message from an earlier worker rejected") + } + if !IsKnownIMAPConnectionMessage(ErrMailServerUnreachableAt(MailConnectionStageIMAPDial, errors.New(MailConnectionReasonPrivateHost)).Message) { + t.Fatal("private host message rejected") + } +} diff --git a/internal/errx/errx.go b/internal/errx/errx.go index dd7412075..a4a79dd5e 100644 --- a/internal/errx/errx.go +++ b/internal/errx/errx.go @@ -3,6 +3,7 @@ package errx import ( "errors" "fmt" + "strconv" "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" @@ -28,6 +29,11 @@ type Error struct { // it. Neither is ever written to a response by Handle. Cause string `json:"-"` Detail string `json:"-"` + // Reason is an optional finer key written to the response, for a client + // that offers a different next step within one Identifier. + Reason string `json:"-"` + // RetryAfter, in seconds, is sent as the Retry-After header when set. + RetryAfter int `json:"-"` } // Error implements error interface. @@ -95,6 +101,7 @@ type response struct { Error string `json:"error"` Message string `json:"message"` Code string `json:"code"` + Reason string `json:"reason,omitempty"` RequestID string `json:"request_id,omitempty"` } @@ -116,10 +123,14 @@ func Handle(c *gin.Context, err error) { // JSON sends a business error as JSON response. func JSON(c *gin.Context, err *Error) { httpCode, httpError := err.resolve() + if err.RetryAfter > 0 { + c.Header("Retry-After", strconv.Itoa(err.RetryAfter)) + } c.JSON(httpCode, response{ Error: httpError, Message: clientMessage(c, err), Code: err.identifier(), + Reason: err.Reason, RequestID: c.GetString("request_id"), }) } @@ -132,8 +143,7 @@ const genericServerMessage = "Something went wrong." // An Internal message describes something that went wrong inside the service, // and handlers routinely built one from the underlying error: driver text with // SQLSTATE codes and column names, provider responses, file paths. None of that -// helps the caller and all of it helps somebody mapping the system, which is -// what CASA 6.2.1 is about. +// helps the caller and all of it helps somebody mapping the system. // // So an Internal error answers with one fixed sentence and the request id. The // real message is logged against that id, which is where an operator should be diff --git a/internal/models/event_schema_test.go b/internal/models/event_schema_test.go index 0f5d2fd46..486fbbb8d 100644 --- a/internal/models/event_schema_test.go +++ b/internal/models/event_schema_test.go @@ -81,6 +81,9 @@ func TestEventAvroReadsPreUpgradeEnvelopes(t *testing.T) { want.TransportGeneration = [16]byte{} want.SealedSecrets = "" want.Sync.State.TransportGeneration = [16]byte{} + for _, svc := range []*Service{want.SmtpImap.Credentials.SMTP, want.SmtpImap.Credentials.IMAP} { + svc.TLSUnverified, svc.TLSPin = false, "" + } assertBody(t, got.Body, want) }) t.Run("job", func(t *testing.T) { @@ -105,6 +108,9 @@ func TestEventAvroReadsPreUpgradeEnvelopes(t *testing.T) { // branchesAfterFixtures are union branches added after the fixtures were written. var branchesAfterFixtures = []any{"warmbly.events.JobEventWarmupRescued"} +// fieldsAfterFixtures are record fields added after the fixtures were written. +var fieldsAfterFixtures = []any{"transport_generation", "sealed_secrets", "tls_unverified", "tls_pin"} + func preTransportSchema(t *testing.T, schema avro.Schema) avro.Schema { t.Helper() doc, err := SchemaDocument(schema) @@ -123,7 +129,7 @@ func preTransportSchema(t *testing.T, schema avro.Schema) avro.Schema { kept := fields[:0] for _, f := range fields { field := f.(map[string]any) - if field["name"] == "transport_generation" || field["name"] == "sealed_secrets" || x["name"] == "EmailErrorEvent" && field["name"] == "operation" { + if slices.Contains(fieldsAfterFixtures, field["name"]) || x["name"] == "EmailErrorEvent" && field["name"] == "operation" { continue } strip(f) diff --git a/internal/models/event_worker_validation.go b/internal/models/event_worker_validation.go index 37e405476..977b79c49 100644 --- a/internal/models/event_worker_validation.go +++ b/internal/models/event_worker_validation.go @@ -42,6 +42,11 @@ const ( MailProbeCleartext = "cleartext" // MailProbeTimeout: the budget ran out before the server answered. MailProbeTimeout = "timeout" + + // MailProbeDetailPrivateHost is the unreachable detail for a server on a non-public address. + MailProbeDetailPrivateHost = "the host name points to a private network address, and Warmbly only connects mailboxes to public addresses" + // MailProbeDetailPinMismatch starts the TLS detail for a certificate other than the confirmed one. + MailProbeDetailPinMismatch = "the server's certificate does not match the one confirmed for this mailbox" ) // EmailValidationLeg is one side (SMTP or IMAP) of a worker's verdict. diff --git a/internal/pkg/mailcause/catalogue.go b/internal/pkg/mailcause/catalogue.go index 5cb506d74..5e56a4347 100644 --- a/internal/pkg/mailcause/catalogue.go +++ b/internal/pkg/mailcause/catalogue.go @@ -36,6 +36,7 @@ const ( AuthRefused = "auth_refused" HostNotFound = "host_not_found" HostUnreachable = "host_unreachable" + PrivateHost = "private_host" PortBlocked = "port_blocked" TLSFailed = "tls_failed" CleartextRefused = "cleartext_refused" @@ -82,19 +83,21 @@ var catalogue = []Cause{ {TooManyLogins, "Too many sign-in attempts", "The provider is limiting sign-ins for this account or from this network. Wait 15 to 60 minutes before retrying, and avoid retrying the same failing password repeatedly.", true}, {AuthUnavailable, "The server does not take a password here", - "The server did not offer password sign-in on this port. Check the host and port: use the provider's submission server (usually port 587 or 465), not the MX server or port 25.", false}, + "The server did not offer a password sign-in method Warmbly supports (LOGIN, PLAIN or CRAM-MD5) on this port. Check the host and port: use the provider's submission server (usually port 587 or 465), not the MX server or port 25.", false}, {AuthRefused, "The server refused the sign-in", "The server refused this username and password. Check both, and whether the provider needs an app password or a username other than the email address.", true}, {HostNotFound, "The server name does not exist", "The server hostname could not be found in DNS. Check the spelling of the SMTP and IMAP host.", false}, {HostUnreachable, "The server could not be reached", "No connection could be made to the server. Check the host and port, and that the server accepts connections from outside your network.", false}, + {PrivateHost, "The server is on a private network", + "The server's host name points to a private network address, and Warmbly only connects mailboxes to public addresses. Use the host name your provider gives for connecting from outside your network.", false}, {PortBlocked, "The mail port looks blocked", "The SMTP port did not answer while IMAP worked, which usually means the port is blocked on the way. Try port 587 with STARTTLS instead of 465 or 25, or check the provider's documented SMTP port.", false}, {TLSFailed, "The secure connection failed", - "The server did not complete a secure connection. Check the security setting for the port: 465 and 993 use SSL/TLS, 587 and 143 use STARTTLS. Also check that the hostname matches the server's certificate.", false}, + "The server did not complete a secure connection. Check the security setting for the port: 465 and 993 use SSL/TLS, 587 and 143 use STARTTLS. Also check that the hostname matches the server's certificate. A server with a self-signed or company certificate can be connected on its own certificate from the connect form.", false}, {CleartextRefused, "Encryption is required", - "The connection was attempted without encryption, which is not allowed here. Use SSL/TLS on 465 or 993, or STARTTLS on 587 or 143.", false}, + "Warmbly only signs in over an encrypted connection, and this one was not encrypted or the server only offers sign-in without encryption. Use SSL/TLS on 465 or 993, or STARTTLS on 587 or 143.", false}, {Timeout, "The server did not answer in time", "The server did not answer before the time ran out. It may be slow or overloaded; retry later, and check the host and port if it keeps happening.", true}, {Temporary, "The server asked to retry later", diff --git a/internal/pkg/mailcause/classify.go b/internal/pkg/mailcause/classify.go index 4b1edc1ef..c4f825f25 100644 --- a/internal/pkg/mailcause/classify.go +++ b/internal/pkg/mailcause/classify.go @@ -118,9 +118,11 @@ func classify(in Input) string { return TooManyLogins case hasAny(text, "must issue a starttls", "must issue starttls", "starttls first", "requires tls", "encryption required", "must use tls"): return CleartextRefused + case has(text, "private network address"): + return PrivateHost case hasAny(text, "no such host", "nxdomain", "name or service not known"): return HostNotFound - case hasAny(text, "no supported authentication", "auth not supported", "authentication not supported", + case hasAny(text, "no supported authentication", "no sign-in method warmbly supports", "auth not supported", "authentication not supported", "authentication not enabled", "unrecognized authentication type", "does not support auth", "authentication mechanism not supported", "auth not available", "authentication not available"): return AuthUnavailable diff --git a/internal/pkg/mailcause/mailcause_test.go b/internal/pkg/mailcause/mailcause_test.go index 2946a262c..778aa4c56 100644 --- a/internal/pkg/mailcause/mailcause_test.go +++ b/internal/pkg/mailcause/mailcause_test.go @@ -94,6 +94,9 @@ func TestClassify(t *testing.T) { SMTP: Leg{Reason: models.MailProbeUnreachable, Host: "mail.example.org", Port: 25, Detail: "dial tcp: i/o timeout"}}, PortBlocked}, {"587 unreachable is not a blocked port", Input{IMAP: ok("IMAP"), SMTP: Leg{Reason: models.MailProbeUnreachable, Host: "mail.example.org", Port: 587, Detail: "connect: connection refused"}}, HostUnreachable}, + {"private host", Input{ + SMTP: Leg{Reason: models.MailProbeUnreachable, Host: "mail.corp.example", Port: 25, Detail: models.MailProbeDetailPrivateHost}, + IMAP: Leg{Reason: models.MailProbeUnreachable, Host: "mail.corp.example", Port: 993, Detail: models.MailProbeDetailPrivateHost}}, PrivateHost}, {"no such host", Input{IMAP: ok("IMAP"), SMTP: Leg{Reason: models.MailProbeUnreachable, Host: "smtp.exmaple.org", Port: 465, Detail: "dial tcp: lookup smtp.exmaple.org: no such host"}}, HostNotFound}, {"tls", Input{IMAP: ok("IMAP"), @@ -103,6 +106,8 @@ func TestClassify(t *testing.T) { SMTP: Leg{Reason: models.MailProbeProtocol, Detail: "530 5.7.0 Must issue a STARTTLS command first"}}, CleartextRefused}, {"auth unavailable", Input{IMAP: ok("IMAP"), SMTP: Leg{Reason: models.MailProbeProtocol, Host: "mx.example.org", Port: 25, Detail: "smtp: server doesn't support AUTH; 503 5.5.1 Error: authentication not enabled"}}, AuthUnavailable}, + {"no supported sign-in method", Input{IMAP: ok("IMAP"), + SMTP: Leg{Reason: models.MailProbeProtocol, Host: "mail.example.org", Port: 587, Detail: "the server offers no sign-in method Warmbly supports: it offers XOAUTH2, and Warmbly signs in with LOGIN, PLAIN or CRAM-MD5"}}, AuthUnavailable}, {"timeout both", Input{IMAP: Leg{Reason: models.MailProbeTimeout, Port: 993}, SMTP: Leg{Reason: models.MailProbeTimeout, Port: 587}}, Timeout}, {"temporary", Input{IMAP: ok("IMAP"), SMTP: Leg{Reason: models.MailProbeTemporary, Detail: "451 4.3.0 Temporary server error"}}, Temporary}, {"auth outranks timeout", Input{MailHost: "other", IMAP: Leg{Reason: models.MailProbeTimeout}, diff --git a/web/src/components/app/emails/UpdateCredentialsDialog.tsx b/web/src/components/app/emails/UpdateCredentialsDialog.tsx index 6449694fc..1cd2ba4d6 100644 --- a/web/src/components/app/emails/UpdateCredentialsDialog.tsx +++ b/web/src/components/app/emails/UpdateCredentialsDialog.tsx @@ -18,7 +18,7 @@ import { allowsNoEncryption, defaultImapSecurity, defaultSmtpSecurity, - MAILBOX_TLS_FAILED, + offersCertificateTrust, validPort, type MailSecurity, } from "@/lib/api/models/app/emails/Service"; @@ -154,7 +154,7 @@ export default function UpdateCredentialsDialog({ onClose(); } catch (e) { // Surfaced by the toast; a certificate failure also offers the opt-out. - if ((e as AppError)?.code === MAILBOX_TLS_FAILED) setCertFailed(true); + if (offersCertificateTrust(e)) setCertFailed(true); } finally { setSubmitting(false); } diff --git a/web/src/components/app/modals/AddEmailModal.tsx b/web/src/components/app/modals/AddEmailModal.tsx index 4299bd1e3..16d5caf37 100644 --- a/web/src/components/app/modals/AddEmailModal.tsx +++ b/web/src/components/app/modals/AddEmailModal.tsx @@ -60,7 +60,7 @@ import { allowsNoEncryption, defaultImapSecurity, defaultSmtpSecurity, - MAILBOX_TLS_FAILED, + offersCertificateTrust, validPort, type MailSecurity, } from "@/lib/api/models/app/emails/Service"; @@ -1088,7 +1088,7 @@ function SmtpImapPanel({ onDone, onError }: { onDone: () => void; onError: (e: u onDone(); } catch (e) { // Surfaced by the toast, except a full allowance, which gets its dialog. - if ((e as AppError)?.code === MAILBOX_TLS_FAILED) setCertFailed(true); + if (offersCertificateTrust(e)) setCertFailed(true); onError(e); } finally { setSubmitting(false); diff --git a/web/src/lib/api/client/normalizeError.ts b/web/src/lib/api/client/normalizeError.ts index c8b4e1127..31d296290 100644 --- a/web/src/lib/api/client/normalizeError.ts +++ b/web/src/lib/api/client/normalizeError.ts @@ -9,6 +9,8 @@ export interface AppError { /** Stable machine-readable code from the API, for branching on a specific * condition rather than matching on human-readable text. */ code?: string; + /** Optional finer key within one code, where the API offers a different next step. */ + reason?: string; /** Correlation id the API already returns, so a user can quote it and an * operator can find the matching server-side log line. */ request_id?: string; @@ -48,6 +50,7 @@ export function normalizeError(error: unknown): AppError { status, redirect: true, code: data.code, + reason: data.reason, request_id: data.request_id, }; } @@ -57,6 +60,7 @@ export function normalizeError(error: unknown): AppError { message: data.message || "Unexpected error occured.", status, code: data.code, + reason: data.reason, request_id: data.request_id, } } diff --git a/web/src/lib/api/models/app/emails/Service.ts b/web/src/lib/api/models/app/emails/Service.ts index ddea6ecb1..5bd13c00e 100644 --- a/web/src/lib/api/models/app/emails/Service.ts +++ b/web/src/lib/api/models/app/emails/Service.ts @@ -46,6 +46,15 @@ export interface MailboxCertificates { /** The error code the backend answers when a server's TLS handshake or certificate failed. */ export const MAILBOX_TLS_FAILED = "mailbox_tls_failed"; +/** The reason on a mailbox_tls_failed the owner can resolve by trusting the server's own certificate. */ +export const CERTIFICATE_UNTRUSTED = "certificate_untrusted"; + +/** True when a test failed on a certificate no authority vouches for, not on a missing STARTTLS or a dead port. */ +export function offersCertificateTrust(e: unknown): boolean { + const err = e as { code?: string; reason?: string } | undefined; + return err?.code === MAILBOX_TLS_FAILED && err.reason === CERTIFICATE_UNTRUSTED; +} + /** Groups a hex fingerprint in pairs for reading aloud and comparing. */ export function formatFingerprint(fp: string): string { return (fp.match(/.{1,2}/g) ?? []).join(":").toUpperCase();