Commit 01856d444d

01856d444da2d5d102113c67e35d1a477bae0541

parent: 90622795ab

Verified · cmc ci/build: success ci/test: success

cmc <hello@cleberg.net> · 2026-09-28 08:33 UTC

mail: require TLS to a non-local relay; implicit TLS option

Closes #280

Layout: unified · split

.gitbay/wiki/Admin.org +9 −2
@@ -125,10 +125,17 @@ build page's live log arrives only when the build ends;
125125 registration. Requires [mail].
126126
127127** [mail]
128- =smtp_host= (host:port, 587 assumed), =from=, optional =smtp_user= /
129 =smtp_pass=. STARTTLS when offered. Required for invite/open
128- =smtp_host= (host:port; 587 assumed, 465 with =tls = "implicit"=),
129 =from=, optional =smtp_user= / =smtp_pass=. Required for invite/open
130130 registration and self-service =email add=; in closed mode you may omit
131131 it entirely and assert addresses by hand (below).
132- =tls= — =starttls= (default) or =implicit= (TLS from the first byte,
133 for relays on 465).
134- =require_tls= — with =starttls=, a relay that does not offer STARTTLS
135 gets no mail: delivery fails and retries, and the admin page's Mail
136 table shows the error. Unset, it is on for every relay except
137 =localhost= and loopback addresses; set =false= to allow plaintext
138 to a remote relay.
132139
133140** [push]
134141Push notifications to Apple devices, delivered by gitbayd talking to
.gitbay/wiki/Architecture/03-Deployment.org +1 −1
@@ -57,7 +57,7 @@ a database check.
5757| Destination | Trigger | TLS | Guard |
5858|------------------------+-------------------------------+----------------------------------------------+----------------------------------------------------------------|
5959| ACME directory | certificate issue and renewal | yes | host policy limits names to the site and claimed pages domains (=main.go=) |
60| SMTP relay | queued mail | STARTTLS when offered | Go's =PlainAuth= will not send credentials over plaintext to a non-local host (=internal/mail/mail.go=) |
60| SMTP relay | queued mail | STARTTLS required for a non-local relay, or implicit TLS | =mail.require_tls=; Go's =PlainAuth= will not send credentials over plaintext to a non-local host (=internal/mail/mail.go=) |
6161| APNs | queued push | yes, HTTP/2 | provider token signed with the operator's .p8 key |
6262| Webhook URLs | recorded events | yes when https; certificate verified | private, loopback and link-local targets refused at save and again at connect time; no redirects (=internal/webhook/webhook.go=) |
6363| Mirror URLs | mirror schedule | per URL | the same address check at save time (=internal/control/mirrorcmd.go=); git makes the connection, so there is no connect-time re-check |
.gitbay/wiki/Architecture/06-Data-and-Cryptography.org +1 −1
@@ -60,7 +60,7 @@ secret, webhook secret and mirror token.
6060| HTTP port 80 | ACME challenges and redirect only |
6161| git:// | none (public data only; off by default) |
6262| Runner ↔ server | SSH |
63| SMTP | STARTTLS when the relay offers it |
63| SMTP | STARTTLS required unless the relay is local (=mail.require_tls=), or implicit TLS (=mail.tls=) |
6464| APNs | TLS, HTTP/2 |
6565| Webhooks | TLS when the URL is https; HMAC-SHA256 body signature in =X-Gitbay-Signature-256= (=internal/webhook/webhook.go=) |
6666| Mirrors | per URL; token passed through =GIT_ASKPASS=, never argv (=internal/mirror/mirror.go=) |
.gitbay/wiki/Architecture/09-Controls.org +1 −1
@@ -77,7 +77,7 @@ chapter names of OWASP ASVS 4.0 where one fits.
7777|---------------------------------------------+----------+------------------------------------------------------------------|
7878| SSRF protection on user-supplied URLs | partial | webhooks at save and connect; mirrors at save only (#279) |
7979| Webhook payload integrity | in place | HMAC-SHA256 header |
80| SMTP credentials protected in transit | partial | STARTTLS opportunistic (#280); Go refuses PLAIN auth without TLS to a remote host |
80| SMTP credentials protected in transit | in place | STARTTLS required for non-local relays, implicit TLS optional (=internal/mail/mail.go=) |
8181| Upload size limits | in place | per-owner storage quota at push (=internal/sshd/sshd.go=); API body 1 MiB |
8282
8383** CI and build isolation
.gitbay/wiki/Architecture/10-Known-Gaps.org −1
@@ -20,7 +20,6 @@ what the 2026-09-27 review found; remove a row when its issue closes.
2020| #274 | Backups | The local backup archive is not encrypted | medium |
2121| #275 | Audit | Refused writes are not audited; the audit table is writable by the daemon user | medium |
2222| #279 | SSRF | Mirror URLs are checked when saved, not when git connects | medium |
23| #280 | Mail | STARTTLS only when the relay offers it | medium |
2423| #282 | Hook socket | Anything that can open =hook.sock= can act as any user | medium |
2524| #297 | Credentials | A browser session can mint tokens and keys that outlive it | low |
2625
CHANGELOG.org +4
@@ -35,6 +35,10 @@ must add =--scope full=. Existing tokens keep their scope.
3535 the login page's mailed links have (#278).
3636- The HTTPS listener refuses TLS below 1.2, in both certificate modes
3737 (#281).
38- Mail to a non-local relay requires TLS by default; a relay that does
39 not offer STARTTLS gets no mail unless =mail.require_tls = false=
40 restores the old behaviour. =mail.tls = "implicit"= speaks TLS from
41 the first byte, for relays on port 465 (#280).
3842
3943* v1.36.0 — 2026-09-23
4044
internal/mail/mail.go +41 −7
@@ -1,10 +1,13 @@
11// Package mail sends transactional email over SMTP: verification codes and
2// invites. STARTTLS is used when the server offers it; PLAIN auth when
3// credentials are configured.
2// invites. The connection is encrypted with STARTTLS, or with TLS from the
3// first byte when mail.tls = "implicit"; a relay that offers neither gets
4// nothing unless mail.require_tls is off. PLAIN auth when credentials are
5// configured.
46package mail
57
68import (
79 "crypto/tls"
10 "crypto/x509"
811 "fmt"
912 "net"
1013 "net/smtp"
@@ -14,30 +17,43 @@ import (
1417 "gitbay.org/gitbay/internal/config"
1518)
1619
20// rootCAs verifies the relay's certificate; nil is the system pool.
21var rootCAs *x509.CertPool
22
1723// Send delivers one plain-text message. cfg.Mail.SMTPHost is host:port.
1824func Send(cfg config.Config, to, subject, body string) error {
1925 m := cfg.Mail
2026 if m.SMTPHost == "" || m.From == "" {
2127 return fmt.Errorf("[mail] smtp_host and from must be configured")
2228 }
29 implicit := m.TLS == "implicit"
2330 host := m.SMTPHost
2431 if !strings.Contains(host, ":") {
25 host += ":587"
32 if implicit {
33 host += ":465"
34 } else {
35 host += ":587"
36 }
2637 }
2738 hostname, _, _ := net.SplitHostPort(host)
39 tlsCfg := &tls.Config{ServerName: hostname, RootCAs: rootCAs}
2840
2941 msg := strings.NewReplacer("\n", "\r\n").Replace(fmt.Sprintf(
3042 "From: %s\nTo: %s\nSubject: %s\nDate: %s\nMIME-Version: 1.0\nContent-Type: text/plain; charset=utf-8\n\n%s\n",
3143 m.From, to, subject, time.Now().Format(time.RFC1123Z), body))
3244
33 c, err := smtp.Dial(host)
45 c, err := dial(host, hostname, implicit, tlsCfg)
3446 if err != nil {
3547 return fmt.Errorf("smtp dial %s: %w", host, err)
3648 }
3749 defer c.Close()
38 if ok, _ := c.Extension("STARTTLS"); ok {
39 if err := c.StartTLS(&tls.Config{ServerName: hostname}); err != nil {
40 return fmt.Errorf("starttls: %w", err)
50 if !implicit {
51 if ok, _ := c.Extension("STARTTLS"); ok {
52 if err := c.StartTLS(tlsCfg); err != nil {
53 return fmt.Errorf("starttls: %w", err)
54 }
55 } else if m.TLSRequired() {
56 return fmt.Errorf("%s does not offer STARTTLS and mail.require_tls is on; not sending in clear", host)
4157 }
4258 }
4359 if m.SMTPUser != "" {
@@ -63,3 +79,21 @@ func Send(cfg config.Config, to, subject, body string) error {
6379 }
6480 return c.Quit()
6581}
82
83// dial opens the SMTP session: plain TCP for STARTTLS, or TLS from the
84// first byte.
85func dial(addr, hostname string, implicit bool, tlsCfg *tls.Config) (*smtp.Client, error) {
86 if !implicit {
87 return smtp.Dial(addr)
88 }
89 conn, err := tls.Dial("tcp", addr, tlsCfg)
90 if err != nil {
91 return nil, err
92 }
93 c, err := smtp.NewClient(conn, hostname)
94 if err != nil {
95 conn.Close()
96 return nil, err
97 }
98 return c, nil
99}
internal/mail/mail_test.go added +142
@@ -0,0 +1,142 @@
1package mail
2
3import (
4 "bufio"
5 "crypto/tls"
6 "crypto/x509"
7 "fmt"
8 "net"
9 "net/http"
10 "net/http/httptest"
11 "strings"
12 "sync"
13 "testing"
14
15 "gitbay.org/gitbay/internal/config"
16)
17
18// fakeRelay is an SMTP server that never offers STARTTLS. Given a TLS
19// config it speaks TLS from the first byte, as a port-465 relay does.
20type fakeRelay struct {
21 addr string
22 mu sync.Mutex
23 data []string
24}
25
26func startRelay(t *testing.T, tlsCfg *tls.Config) *fakeRelay {
27 t.Helper()
28 ln, err := net.Listen("tcp", "127.0.0.1:0")
29 if err != nil {
30 t.Fatal(err)
31 }
32 if tlsCfg != nil {
33 ln = tls.NewListener(ln, tlsCfg)
34 }
35 t.Cleanup(func() { ln.Close() })
36 f := &fakeRelay{addr: ln.Addr().String()}
37 go func() {
38 for {
39 conn, err := ln.Accept()
40 if err != nil {
41 return
42 }
43 go f.serve(conn)
44 }
45 }()
46 return f
47}
48
49func (f *fakeRelay) serve(conn net.Conn) {
50 defer conn.Close()
51 r := bufio.NewReader(conn)
52 fmt.Fprint(conn, "220 fake\r\n")
53 var body strings.Builder
54 inData := false
55 for {
56 line, err := r.ReadString('\n')
57 if err != nil {
58 return
59 }
60 line = strings.TrimRight(line, "\r\n")
61 switch {
62 case inData && line == ".":
63 f.mu.Lock()
64 f.data = append(f.data, body.String())
65 f.mu.Unlock()
66 inData = false
67 fmt.Fprint(conn, "250 ok\r\n")
68 case inData:
69 body.WriteString(line + "\n")
70 case strings.HasPrefix(line, "EHLO"), strings.HasPrefix(line, "HELO"):
71 fmt.Fprint(conn, "250-fake\r\n250 SIZE 1000000\r\n")
72 case line == "DATA":
73 inData = true
74 fmt.Fprint(conn, "354 go\r\n")
75 case line == "QUIT":
76 fmt.Fprint(conn, "221 bye\r\n")
77 return
78 default:
79 fmt.Fprint(conn, "250 ok\r\n")
80 }
81 }
82}
83
84func (f *fakeRelay) delivered() int {
85 f.mu.Lock()
86 defer f.mu.Unlock()
87 return len(f.data)
88}
89
90func mailCfg(host string) config.Config {
91 var cfg config.Config
92 cfg.Mail.SMTPHost, cfg.Mail.From = host, "gitbay@example.test"
93 return cfg
94}
95
96func TestRequireTLSRefusesPlaintextRelay(t *testing.T) {
97 relay := startRelay(t, nil)
98 cfg := mailCfg(relay.addr)
99 on := true
100 cfg.Mail.RequireTLS = &on
101 err := Send(cfg, "a@example.test", "subject", "body")
102 if err == nil || !strings.Contains(err.Error(), "STARTTLS") {
103 t.Fatalf("Send = %v, want a refusal naming STARTTLS", err)
104 }
105 if n := relay.delivered(); n != 0 {
106 t.Fatalf("%d message(s) sent in clear", n)
107 }
108}
109
110// A loopback relay has no network to cross; the default leaves it in
111// clear, which is what the e2e suite's fake relay relies on.
112func TestLoopbackRelayDefaultsToPlaintext(t *testing.T) {
113 relay := startRelay(t, nil)
114 if err := Send(mailCfg(relay.addr), "a@example.test", "subject", "body"); err != nil {
115 t.Fatal(err)
116 }
117 if n := relay.delivered(); n != 1 {
118 t.Fatalf("delivered %d, want 1", n)
119 }
120}
121
122func TestImplicitTLS(t *testing.T) {
123 ts := httptest.NewTLSServer(http.NotFoundHandler())
124 defer ts.Close()
125 pool := x509.NewCertPool()
126 pool.AddCert(ts.Certificate())
127 prev := rootCAs
128 rootCAs = pool
129 defer func() { rootCAs = prev }()
130
131 relay := startRelay(t, &tls.Config{Certificates: ts.TLS.Certificates})
132 cfg := mailCfg(relay.addr)
133 cfg.Mail.TLS = "implicit"
134 on := true
135 cfg.Mail.RequireTLS = &on
136 if err := Send(cfg, "a@example.test", "subject", "body"); err != nil {
137 t.Fatal(err)
138 }
139 if n := relay.delivered(); n != 1 {
140 t.Fatalf("delivered %d, want 1", n)
141 }
142}