diff --git a/README.md b/README.md index 756b96a..1bec364 100644 --- a/README.md +++ b/README.md @@ -226,9 +226,23 @@ everyone who sends from it. Publishing one needs three things, and the console file, DMARC, the BIMI record in DNS, the public URL of the logo, the certificate and its public URL, and names the one that is wrong. -The logo is served from `https:///.well-known/bimi/default.svg` and -the record is published with the rest of the zone. Replacing the logo under a -held certificate breaks the certificate, and the console says so. +The logo is served from `https:///.well-known/bimi/default.svg`. + +**The DNS record follows the picture.** With a Cloudflare token configured, the +`default._bimi.` TXT record is part of what the DNS reconciler +publishes, and saving or removing the logo or the certificate in the console +triggers that reconciliation by itself ("Publicar no DNS agora" does the same +on demand). The record is created when a logo exists, updated when the +certificate is added, and removed when the logo is removed, but only a record +this reconciler published: one made by hand is left alone. + +A held certificate is advertised (`a=`) only while it is valid for the logo +being served. The certificate vouches for one exact SVG, so replacing the logo +ends it, and a record that kept naming it would make every receiver reject BIMI, +including those that would have shown the logo without a certificate. The record +then goes back to `l=` alone, and the console says why the certificate is held +back. The panel, the setup check and the reconciler all take that decision from +the same function (`bimi.Publish`), so they cannot disagree about it. A changed profile photo shows up at once everywhere in the webmail: every photo carries a version that is bumped on save, in this tab and the others. The diff --git a/services/protocols/cmd/lambdamail-protocols/bimi_publication.go b/services/protocols/cmd/lambdamail-protocols/bimi_publication.go new file mode 100644 index 0000000..81b5c79 --- /dev/null +++ b/services/protocols/cmd/lambdamail-protocols/bimi_publication.go @@ -0,0 +1,36 @@ +package main + +import ( + "context" + "log" + "time" + + "lambdamail/protocols/internal/domain/bimi" + "lambdamail/protocols/internal/infrastructure/postgres" + httppresentation "lambdamail/protocols/internal/presentation/http" +) + +// bimiPublication is what a domain's BIMI record should say, for the sync, the +// background sweep and the console alike. Read each time, so a logo saved a +// moment ago is what gets published and a removed one stops being published. +func bimiPublication(ctx context.Context, repo *postgres.BimiRepository, cfg config, domain string) bimi.Publication { + if repo == nil { + return bimi.Publication{} + } + logo, err := repo.Load(ctx, domain) + if err != nil { + // Unknown is not "no logo": returning an empty publication here would + // have the sync delete the record over a database hiccup. + log.Printf("BIMI: could not read the logo for %s, leaving its record alone: %v", domain, err) + return bimi.Publication{LogoURL: httppresentation.BimiLogoURL(cfg.PrimaryMailHost)} + } + if logo == nil { + return bimi.Publication{} + } + return bimi.Publish( + logo.SVG, logo.VmcPEM, logo.VmcURL, domain, + httppresentation.BimiLogoURL(cfg.PrimaryMailHost), + httppresentation.BimiVmcURL(cfg.PrimaryMailHost), + time.Now(), + ) +} diff --git a/services/protocols/cmd/lambdamail-protocols/dns_reconciler.go b/services/protocols/cmd/lambdamail-protocols/dns_reconciler.go index 3faed39..c72c9ae 100644 --- a/services/protocols/cmd/lambdamail-protocols/dns_reconciler.go +++ b/services/protocols/cmd/lambdamail-protocols/dns_reconciler.go @@ -42,6 +42,7 @@ func (d domainReconciler) ReconcileDomain( Created: out.CreatedCount, Updated: out.UpdatedCount, Unchanged: out.UnchangedCount, + Deleted: out.DeletedCount, Conflicts: conflicts, Errors: out.Errors, }, nil @@ -54,7 +55,7 @@ func (d domainReconciler) ReconcileDomain( // halfway through each other's zone updates. func reconcileDomainOnDemand( ctx context.Context, cfg config, aliasRepo port.SystemAliasRepository, - dkimRepo *postgres.DkimRepository, domain string, + dkimRepo *postgres.DkimRepository, bimiRepo *postgres.BimiRepository, domain string, ) (*usecase.SyncDnsRecordsOutput, error) { syncUC := usecase.NewSyncDnsRecordsUseCase(cloudflare.NewCloudflareAdapter(cfg.CloudflareToken), aliasRepo) syncUC.SetVerifier(netdns.NewPublicVerifier()) @@ -67,6 +68,8 @@ func reconcileDomainOnDemand( DaneEnabled: cfg.OutboundDane, RelaySpfInclude: relayConfig(cfg).SpfInclude(), } + publication := bimiPublication(ctx, bimiRepo, cfg, domain) + input.BimiLogoURL, input.BimiVmcURL = publication.LogoURL, publication.CertificateURL // A domain with no DKIM key of its own would publish a selector pointing // at nothing, so the keys are provisioned first, exactly as the timer does. diff --git a/services/protocols/cmd/lambdamail-protocols/main.go b/services/protocols/cmd/lambdamail-protocols/main.go index 3d40377..52d759f 100644 --- a/services/protocols/cmd/lambdamail-protocols/main.go +++ b/services/protocols/cmd/lambdamail-protocols/main.go @@ -187,7 +187,7 @@ func run(cfg config) { // ------------------------------------------------------- background jobs go runDeliveryWorker(ctx, outboundWorker) if cfg.CloudflareToken != "" { - go runDnsReconciler(ctx, cfg, aliasRepo, dkimRepo, postgres.NewDomainRepository(pool)) + go runDnsReconciler(ctx, cfg, aliasRepo, dkimRepo, postgres.NewDomainRepository(pool), postgres.NewBimiRepository(pool)) } else { log.Printf("CLOUDFLARE_API_TOKEN is not set: DNS reconciliation is disabled") } @@ -215,9 +215,19 @@ func run(cfg config) { // provider to talk to, so the route can say it is unavailable instead of // reaching Cloudflare with an empty token and reporting a refusal. if cfg.CloudflareToken != "" { + bimiRepo := postgres.NewBimiRepository(pool) router.SetDnsReconciler(domainReconciler{run: func(c context.Context, domain string) (*usecase.SyncDnsRecordsOutput, error) { - return reconcileDomainOnDemand(c, cfg, aliasRepo, dkimRepo, domain) + return reconcileDomainOnDemand(c, cfg, aliasRepo, dkimRepo, bimiRepo, domain) }}) + // Saving or removing a logo or a certificate publishes the change: the + // record in the zone follows the picture, with no button to remember. + router.SetBimiPublisher(func(domain string) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + if _, err := reconcileDomainOnDemand(ctx, cfg, aliasRepo, dkimRepo, bimiRepo, domain); err != nil { + log.Printf("BIMI: publishing the record for %s after a change failed: %v", domain, err) + } + }) } router.SetFolderAdmin(usecase.NewManageFoldersUseCase( postgres.NewWebmailRepository(pool), postgres.NewWebmailRepository(pool))) @@ -578,6 +588,7 @@ func runDeliveryWorker(ctx context.Context, worker *usecase.OutboundWorkerUseCas func runDnsReconciler( ctx context.Context, cfg config, aliasRepo port.SystemAliasRepository, dkimRepo *postgres.DkimRepository, domainRepo *postgres.DomainRepository, + bimiRepo *postgres.BimiRepository, ) { syncUC := usecase.NewSyncDnsRecordsUseCase(cloudflare.NewCloudflareAdapter(cfg.CloudflareToken), aliasRepo) syncUC.SetVerifier(netdns.NewPublicVerifier()) @@ -600,6 +611,9 @@ func runDnsReconciler( // (PLAN.md section 10.4). RelaySpfInclude: relayConfig(cfg).SpfInclude(), } + // The logo and certificate a domain publishes through BIMI + publication := bimiPublication(ctx, bimiRepo, cfg, domain) + input.BimiLogoURL, input.BimiVmcURL = publication.LogoURL, publication.CertificateURL if provisioner != nil { keys, err := provisioner.Execute(ctx, domain) @@ -717,19 +731,12 @@ func (s *dnsSpecSource) ExpectedRecords(ctx context.Context, domain string) ([]e RelaySpfInclude: relayConfig(s.cfg).SpfInclude(), } - // The BIMI record is published only when a mark is actually stored, so - // the panel never advertises a logo that would 404. + // The same decision the sync makes (bimi.Publish), so the panel expects + // exactly what the reconciler writes. if s.bimi != nil { - if logo, err := s.bimi.Load(ctx, domain); err == nil && logo != nil { - spec.BimiLogoURL = httppresentation.BimiLogoURL(s.cfg.PrimaryMailHost) - // The certificate held here wins: it is the one verified against the - // logo being served. An external URL is the fallback. - if len(logo.VmcPEM) > 0 { - spec.BimiVmcURL = httppresentation.BimiVmcURL(s.cfg.PrimaryMailHost) - } else { - spec.BimiVmcURL = logo.VmcURL - } - } + p := bimiPublication(ctx, s.bimi, s.cfg, domain) + spec.BimiLogoURL = p.LogoURL + spec.BimiVmcURL = p.CertificateURL } // Read rather than provisioned: verifying must not create keys as a side diff --git a/services/protocols/internal/application/usecase/sync_dns_bimi_test.go b/services/protocols/internal/application/usecase/sync_dns_bimi_test.go new file mode 100644 index 0000000..8229210 --- /dev/null +++ b/services/protocols/internal/application/usecase/sync_dns_bimi_test.go @@ -0,0 +1,122 @@ +package usecase + +import ( + "context" + "strings" + "testing" + + "lambdamail/protocols/internal/domain/entity" +) + +func bimiInput(logo, cert string) SyncDnsRecordsInput { + return SyncDnsRecordsInput{ + DomainName: "example.test", + MailHost: "mail.example.test", + ServerIPv4: "192.0.2.1", + BimiLogoURL: logo, + BimiVmcURL: cert, + } +} + +const ( + bimiName = "default._bimi.example.test" + bimiLogo = "https://mail.example.test/.well-known/bimi/default.svg" + bimiCert = "https://mail.example.test/.well-known/bimi/vmc.pem" +) + +func bimiRecords(dns *fakeDnsProvider) []entity.DnsRecord { return dns.find("TXT", bimiName) } + +// The gap this closes: the console knew the BIMI record, the reconciler did +// not, so storing a logo never put anything in the zone. +func TestSyncPublishesTheBimiRecord(t *testing.T) { + dns := newFakeDnsProvider() + uc := NewSyncDnsRecordsUseCase(dns, &fakeSystemAliasRepository{}) + + out, err := uc.Execute(context.Background(), bimiInput(bimiLogo, "")) + if err != nil { + t.Fatal(err) + } + got := bimiRecords(dns) + if len(got) != 1 || got[0].Value != "v=BIMI1; l="+bimiLogo+";" { + t.Fatalf("records = %+v", got) + } + if !strings.HasPrefix(got[0].Comment, "LambdaMail") { + t.Errorf("the record must carry the ownership comment, got %q", got[0].Comment) + } + if out.CreatedCount == 0 { + t.Error("nothing reported as created") + } + + // Idempotent + out2, _ := uc.Execute(context.Background(), bimiInput(bimiLogo, "")) + if out2.CreatedCount != 0 || out2.UpdatedCount != 0 || len(bimiRecords(dns)) != 1 { + t.Errorf("second run changed something: created=%d updated=%d", out2.CreatedCount, out2.UpdatedCount) + } +} + +func TestSyncUpdatesTheBimiRecordWhenACertificateIsAdded(t *testing.T) { + dns := newFakeDnsProvider() + uc := NewSyncDnsRecordsUseCase(dns, &fakeSystemAliasRepository{}) + _, _ = uc.Execute(context.Background(), bimiInput(bimiLogo, "")) + + out, err := uc.Execute(context.Background(), bimiInput(bimiLogo, bimiCert)) + if err != nil { + t.Fatal(err) + } + got := bimiRecords(dns) + if len(got) != 1 || got[0].Value != "v=BIMI1; l="+bimiLogo+"; a="+bimiCert+";" { + t.Fatalf("records = %+v", got) + } + if out.UpdatedCount != 1 { + t.Errorf("updated = %d, want 1 (an update, not a second record)", out.UpdatedCount) + } + + // And back, when the certificate is withheld + _, _ = uc.Execute(context.Background(), bimiInput(bimiLogo, "")) + if got := bimiRecords(dns); len(got) != 1 || strings.Contains(got[0].Value, "a=") { + t.Fatalf("the certificate stayed advertised: %+v", got) + } +} + +func TestSyncRemovesTheBimiRecordItPublishedWhenTheLogoGoes(t *testing.T) { + dns := newFakeDnsProvider() + uc := NewSyncDnsRecordsUseCase(dns, &fakeSystemAliasRepository{}) + _, _ = uc.Execute(context.Background(), bimiInput(bimiLogo, "")) + + out, err := uc.Execute(context.Background(), bimiInput("", "")) + if err != nil { + t.Fatal(err) + } + if got := bimiRecords(dns); len(got) != 0 { + t.Fatalf("the record survived the logo: %+v", got) + } + if out.DeletedCount != 1 { + t.Errorf("deleted = %d", out.DeletedCount) + } +} + +// A record the operator made by hand is theirs, even if it looks like ours. +func TestSyncLeavesAHandMadeBimiRecordAlone(t *testing.T) { + dns := newFakeDnsProvider() + dns.seed(entity.DnsRecord{Type: "TXT", Name: bimiName, Value: "v=BIMI1; l=https://cdn.example.test/logo.svg;", TTL: 1, Comment: "made by hand"}) + uc := NewSyncDnsRecordsUseCase(dns, &fakeSystemAliasRepository{}) + + out, err := uc.Execute(context.Background(), bimiInput("", "")) + if err != nil { + t.Fatal(err) + } + if got := bimiRecords(dns); len(got) != 1 || out.DeletedCount != 0 { + t.Fatalf("a record this reconciler did not publish was touched: %+v deleted=%d", got, out.DeletedCount) + } +} + +func TestSyncPublishesNothingForADomainWithoutALogo(t *testing.T) { + dns := newFakeDnsProvider() + uc := NewSyncDnsRecordsUseCase(dns, &fakeSystemAliasRepository{}) + if _, err := uc.Execute(context.Background(), bimiInput("", "")); err != nil { + t.Fatal(err) + } + if got := bimiRecords(dns); len(got) != 0 { + t.Fatalf("records = %+v", got) + } +} diff --git a/services/protocols/internal/application/usecase/sync_dns_records.go b/services/protocols/internal/application/usecase/sync_dns_records.go index 62a0c99..480dba2 100644 --- a/services/protocols/internal/application/usecase/sync_dns_records.go +++ b/services/protocols/internal/application/usecase/sync_dns_records.go @@ -38,6 +38,11 @@ type SyncDnsRecordsInput struct { DaneEnabled bool RelaySpfInclude string AdminEmailAddress string + // BimiLogoURL and BimiVmcURL are what the BIMI record should say (see + // bimi.Publish). With no logo URL the domain has no BIMI record, and one + // this reconciler published earlier is removed. + BimiLogoURL string + BimiVmcURL string } // SyncConflict describes a record that could not be reconciled because a @@ -59,9 +64,12 @@ type SyncDnsRecordsOutput struct { CreatedCount int UpdatedCount int UnchangedCount int - ConflictCount int - Conflicts []SyncConflict - Errors []string + // DeletedCount is the records retired because they are no longer wanted + // (a BIMI record whose logo was removed). + DeletedCount int + ConflictCount int + Conflicts []SyncConflict + Errors []string // Unverified lists records that were written but could not be read back // through the public resolvers. Unverified []string @@ -108,6 +116,8 @@ func (uc *SyncDnsRecordsUseCase) Execute(ctx context.Context, input SyncDnsRecor TlsaHashes: tlsaHashes, DaneEnabled: input.DaneEnabled, RelaySpfInclude: input.RelaySpfInclude, + BimiLogoURL: input.BimiLogoURL, + BimiVmcURL: input.BimiVmcURL, }) zoneID, err := uc.dnsProvider.GetZoneID(ctx, input.DomainName) @@ -165,6 +175,8 @@ func (uc *SyncDnsRecordsUseCase) Execute(ctx context.Context, input SyncDnsRecor } } + uc.retireBimi(ctx, zoneID, input, existingByKey, output) + if input.AdminEmailAddress != "" && uc.aliasRepo != nil { if err := uc.aliasRepo.EnsureSystemAliases(ctx, input.DomainName, input.AdminEmailAddress); err != nil { return nil, fmt.Errorf("ensure system aliases: %w", err) @@ -187,6 +199,31 @@ func (uc *SyncDnsRecordsUseCase) Execute(ctx context.Context, input SyncDnsRecor return output, nil } +// retireBimi removes the BIMI record this reconciler published once the domain +// no longer has a logo: left in place it advertises a logo URL that answers 404. +// +// Only a record carrying this reconciler's comment is touched. One the +// operator created by hand, even with the same v=BIMI1 tag, is theirs. +func (uc *SyncDnsRecordsUseCase) retireBimi( + ctx context.Context, zoneID string, input SyncDnsRecordsInput, + existingByKey map[string][]entity.DnsRecord, output *SyncDnsRecordsOutput, +) { + if input.BimiLogoURL != "" { + return + } + name := "default._bimi." + input.DomainName + for _, rec := range existingByKey[recordKey("TXT", name)] { + if !strings.HasPrefix(rec.Comment, ownedRecordCommentPrefix) { + continue + } + if err := uc.dnsProvider.DeleteRecord(ctx, zoneID, rec.ID); err != nil { + output.Errors = append(output.Errors, fmt.Sprintf("delete TXT %s: %v", name, err)) + continue + } + output.DeletedCount++ + } +} + // verifyPublished re-reads every desired record through public resolvers and // returns the ones that did not check out. func (uc *SyncDnsRecordsUseCase) verifyPublished(ctx context.Context, specs []entity.DnsRecord) []string { diff --git a/services/protocols/internal/domain/bimi/check.go b/services/protocols/internal/domain/bimi/check.go index 76c0d0b..531bb47 100644 --- a/services/protocols/internal/domain/bimi/check.go +++ b/services/protocols/internal/domain/bimi/check.go @@ -58,7 +58,9 @@ type Inputs struct { // the one the operator keeps elsewhere, used only when none is held. CertificatePEM []byte ExternalCertificateURL string - Now time.Time + // Withheld is why a held certificate is not in the record (see Publish). + Withheld string + Now time.Time } // RunChecks runs every check and returns them in the order a receiver would @@ -214,7 +216,12 @@ func checkCertificate(ctx context.Context, in Inputs, fetch Fetcher) []Check { for _, v := range violations { reasons = append(reasons, v.String()) } - return append([]Check{{ID: "certificate", Status: StatusFail, Detail: strings.Join(reasons, "; ")}}, out...) + detail := strings.Join(reasons, "; ") + if in.Withheld != "" { + // The record stays valid for receivers that do not need a certificate + detail = "held here but NOT published in the DNS record, because it does not apply: " + detail + } + return append([]Check{{ID: "certificate", Status: StatusFail, Detail: detail}}, out...) } if len(cert.Warnings) > 0 { return append([]Check{{ID: "certificate", Status: StatusWarn, Detail: strings.Join(cert.Warnings, "; ")}}, out...) diff --git a/services/protocols/internal/domain/bimi/publish.go b/services/protocols/internal/domain/bimi/publish.go new file mode 100644 index 0000000..a0c72d8 --- /dev/null +++ b/services/protocols/internal/domain/bimi/publish.go @@ -0,0 +1,52 @@ +package bimi + +import ( + "strings" + "time" +) + +// Publication is what the domain's BIMI DNS record should say right now. +// +// One decision, taken in one place, because three things have to agree on it: +// the record the reconciler writes to the zone, the one the console expects to +// find there, and the setup check. They used to compute it separately, and the +// reconciler did not compute it at all, so a logo that was stored was never +// published. +type Publication struct { + // LogoURL is empty when the domain has no logo: then there is no record. + LogoURL string + // CertificateURL is the a= tag, empty when none should be published. + CertificateURL string + // Withheld says why a certificate that is held here is not being published. + Withheld string +} + +// Publish decides the record for a domain. +// +// A certificate held here is published only while it is valid for the logo +// being served. The certificate vouches for one exact SVG, so the moment the +// logo is replaced it stops applying, and a record that still names it makes +// receivers reject BIMI outright, including the ones that would have shown the +// logo without a certificate. Withholding it keeps those working and leaves the +// reason for the console to show. +func Publish(svg, pem []byte, externalURL, domain, logoURL, hostedCertificateURL string, now time.Time) Publication { + if len(svg) == 0 { + return Publication{} + } + p := Publication{LogoURL: logoURL, CertificateURL: strings.TrimSpace(externalURL)} + + if len(pem) == 0 { + return p + } + + if _, violations := InspectCertificate(pem, domain, svg, now); len(violations) > 0 { + rules := make([]string, 0, len(violations)) + for _, v := range violations { + rules = append(rules, v.Rule) + } + p.Withheld = strings.Join(rules, ", ") + return p + } + p.CertificateURL = hostedCertificateURL + return p +} diff --git a/services/protocols/internal/domain/bimi/publish_test.go b/services/protocols/internal/domain/bimi/publish_test.go new file mode 100644 index 0000000..d5f3dcb --- /dev/null +++ b/services/protocols/internal/domain/bimi/publish_test.go @@ -0,0 +1,65 @@ +package bimi + +import ( + "testing" + "time" +) + +const ( + pubLogo = "https://mail.ebay.com/.well-known/bimi/default.svg" + pubCert = "https://mail.ebay.com/.well-known/bimi/vmc.pem" +) + +func TestPublishNothingWithoutALogo(t *testing.T) { + if got := Publish(nil, nil, "https://x.test/v.pem", "ebay.com", pubLogo, pubCert, fixtureNow); got != (Publication{}) { + t.Fatalf("a domain with no logo must publish nothing, got %+v", got) + } +} + +func TestPublishLogoOnly(t *testing.T) { + got := Publish(read(t, "ebay.svg"), nil, "", "ebay.com", pubLogo, pubCert, fixtureNow) + if got.LogoURL != pubLogo || got.CertificateURL != "" || got.Withheld != "" { + t.Fatalf("%+v", got) + } +} + +func TestPublishAnExternalCertificateURL(t *testing.T) { + got := Publish(read(t, "ebay.svg"), nil, " https://vmc.example.test/x.pem ", "ebay.com", pubLogo, pubCert, fixtureNow) + if got.CertificateURL != "https://vmc.example.test/x.pem" { + t.Fatalf("%+v", got) + } +} + +func TestPublishTheHeldCertificateWhileItIsValidForTheLogo(t *testing.T) { + got := Publish(read(t, "ebay.svg"), read(t, "ebay.pem"), "https://old.test/x.pem", "ebay.com", pubLogo, pubCert, fixtureNow) + // The held one wins over an external URL: it is the one checked against this logo + if got.CertificateURL != pubCert || got.Withheld != "" { + t.Fatalf("%+v", got) + } +} + +// The reason this exists: replacing the logo under a held certificate must not +// leave a record that points receivers at a certificate for another picture. +func TestPublishWithholdsACertificateThatNoLongerMatchesTheLogo(t *testing.T) { + got := Publish(read(t, "cnn.svg"), read(t, "ebay.pem"), "", "ebay.com", pubLogo, pubCert, fixtureNow) + if got.LogoURL != pubLogo { + t.Errorf("the logo must still be published: %+v", got) + } + if got.CertificateURL != "" || got.Withheld != "logoMismatch" { + t.Fatalf("%+v", got) + } +} + +func TestPublishWithholdsAnExpiredCertificate(t *testing.T) { + got := Publish(read(t, "ebay.svg"), read(t, "ebay.pem"), "", "ebay.com", pubLogo, pubCert, time.Date(2028, 1, 1, 0, 0, 0, 0, time.UTC)) + if got.CertificateURL != "" || got.Withheld != "validity" { + t.Fatalf("%+v", got) + } +} + +func TestPublishFallsBackToTheExternalURLWhenTheHeldCertificateIsWithheld(t *testing.T) { + got := Publish(read(t, "cnn.svg"), read(t, "ebay.pem"), "https://vmc.example.test/x.pem", "ebay.com", pubLogo, pubCert, fixtureNow) + if got.CertificateURL != "https://vmc.example.test/x.pem" || got.Withheld == "" { + t.Fatalf("%+v", got) + } +} diff --git a/services/protocols/internal/domain/entity/dns_record.go b/services/protocols/internal/domain/entity/dns_record.go index ae69287..991f20c 100644 --- a/services/protocols/internal/domain/entity/dns_record.go +++ b/services/protocols/internal/domain/entity/dns_record.go @@ -207,14 +207,10 @@ func BuildDnsRecordSpecs(spec DnsRecordSpec) []DnsRecord { // BIMI, published only when there is a logo to point at. if spec.BimiLogoURL != "" { - value := fmt.Sprintf("v=BIMI1; l=%s;", spec.BimiLogoURL) - if spec.BimiVmcURL != "" { - value = fmt.Sprintf("v=BIMI1; l=%s; a=%s;", spec.BimiLogoURL, spec.BimiVmcURL) - } records = append(records, DnsRecord{ Type: "TXT", Name: fmt.Sprintf("default._bimi.%s", domainName), - Value: value, + Value: BimiRecordValue(spec.BimiLogoURL, spec.BimiVmcURL), TTL: 1, Comment: "LambdaMail BIMI Record", }) @@ -316,3 +312,15 @@ func hostInsideDomain(host, domain string) bool { // inside "example.test". return host == domain || strings.HasSuffix(host, "."+domain) } + +// BimiRecordValue is the text of a domain's BIMI TXT record. The reconciler +// writes it and the console shows it, so both come from here. +func BimiRecordValue(logoURL, certificateURL string) string { + if logoURL == "" { + return "" + } + if certificateURL != "" { + return fmt.Sprintf("v=BIMI1; l=%s; a=%s;", logoURL, certificateURL) + } + return fmt.Sprintf("v=BIMI1; l=%s;", logoURL) +} diff --git a/services/protocols/internal/presentation/http/admin_dns_api.go b/services/protocols/internal/presentation/http/admin_dns_api.go index 4cc5832..21da329 100644 --- a/services/protocols/internal/presentation/http/admin_dns_api.go +++ b/services/protocols/internal/presentation/http/admin_dns_api.go @@ -29,10 +29,12 @@ type DnsSpecSource interface { // name already holding somebody else's value is not this server's to // overwrite - it may be the zone's mail, or a service nobody remembered. type ReconcileResult struct { - Domain string `json:"domain"` - Created int `json:"created"` - Updated int `json:"updated"` - Unchanged int `json:"unchanged"` + Domain string `json:"domain"` + Created int `json:"created"` + Updated int `json:"updated"` + Unchanged int `json:"unchanged"` + // Deleted counts records retired because they are no longer wanted. + Deleted int `json:"deleted"` Conflicts []string `json:"conflicts"` Errors []string `json:"errors"` } diff --git a/services/protocols/internal/presentation/http/bimi_api.go b/services/protocols/internal/presentation/http/bimi_api.go index cfdd39e..ca60afb 100644 --- a/services/protocols/internal/presentation/http/bimi_api.go +++ b/services/protocols/internal/presentation/http/bimi_api.go @@ -13,6 +13,7 @@ import ( "time" "lambdamail/protocols/internal/domain/bimi" + "lambdamail/protocols/internal/domain/entity" ) // BimiStore holds the mark a domain publishes, and the certificate that @@ -47,6 +48,25 @@ type bimiAPI struct { now func() time.Time dns bimi.Resolver fetch bimi.Fetcher + // publish is told when a domain's logo or certificate changed, so the DNS + // record can follow. Nil when there is no DNS provider to publish to. + publish func(domain string) +} + +// SetBimiPublisher wires what happens after a logo or certificate changes. +func (r *Router) SetBimiPublisher(publish func(domain string)) { + if r.bimi != nil { + r.bimi.publish = publish + } +} + +// changed tells the publisher, in the background: a change is not slowed +// down by a round trip to the DNS provider, and its outcome is the +// reconciliation result the console shows, not part of the save. +func (a *bimiAPI) changed(domain string) { + if a.publish != nil { + go a.publish(domain) + } } // SetBimiAPI wires the BIMI logo store. @@ -168,6 +188,7 @@ func (r *Router) handleBimiAdmin(w http.ResponseWriter, req *http.Request) { writeError(w, http.StatusInternalServerError, "DELETE_FAILED", "Could not remove the logo") return } + r.bimi.changed(domain) writeJSON(w, http.StatusOK, map[string]any{"ok": true}) default: writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "GET, PUT or DELETE") @@ -228,6 +249,7 @@ func (r *Router) handleBimiCertificateAdmin(w http.ResponseWriter, req *http.Req writeError(w, http.StatusInternalServerError, "DELETE_FAILED", "Could not remove the certificate") return } + r.bimi.changed(domain) writeJSON(w, http.StatusOK, map[string]any{"ok": true}) default: writeError(w, http.StatusMethodNotAllowed, "METHOD_NOT_ALLOWED", "PUT or DELETE") @@ -259,21 +281,21 @@ func (r *Router) handleBimiCheck(w http.ResponseWriter, req *http.Request) { ctx, cancel := context.WithTimeout(req.Context(), 25*time.Second) defer cancel() + // What the record should say is decided once (bimi.Publish), exactly as the + // sync writes it, so "the record is wrong" means it differs from what the + // reconciler would publish. + publication := bimi.Publish(logo.SVG, logo.VmcPEM, logo.VmcURL, domain, + BimiLogoURL(r.bimi.mailHost), BimiVmcURL(r.bimi.mailHost), r.bimi.now()) in := bimi.Inputs{ Domain: domain, - LogoURL: BimiLogoURL(r.bimi.mailHost), + LogoURL: publication.LogoURL, + CertificateURL: publication.CertificateURL, SVG: logo.SVG, CertificatePEM: logo.VmcPEM, ExternalCertificateURL: logo.VmcURL, + Withheld: publication.Withheld, Now: r.bimi.now(), } - // The record must name the certificate that is actually served - switch { - case len(logo.VmcPEM) > 0: - in.CertificateURL = BimiVmcURL(r.bimi.mailHost) - default: - in.CertificateURL = logo.VmcURL - } results := bimi.RunChecks(ctx, in, r.bimi.dns, r.bimi.fetch) checks := make([]map[string]string, 0, len(results)) @@ -298,15 +320,22 @@ func (a *bimiAPI) get(w http.ResponseWriter, req *http.Request, domain string) { writeJSON(w, http.StatusOK, map[string]any{"configured": false}) return } + publication := bimi.Publish(logo.SVG, logo.VmcPEM, logo.VmcURL, domain, + BimiLogoURL(a.mailHost), BimiVmcURL(a.mailHost), a.now()) writeJSON(w, http.StatusOK, map[string]any{ - "configured": true, - "etag": logo.ETag, - "vmc_url": logo.VmcURL, - "bytes": len(logo.SVG), - "svg": string(logo.SVG), - "logo_url": BimiLogoURL(a.mailHost), - "certificate_url": BimiVmcURL(a.mailHost), - "certificate": a.describeCertificate(domain, logo), + "configured": true, + "etag": logo.ETag, + // The record exactly as the reconciler writes it, and the reason a held + // certificate is left out of it, if it is + "dns_record": entity.BimiRecordValue(publication.LogoURL, publication.CertificateURL), + "certificate_withheld": publication.Withheld, + "dns_publishing": a.publish != nil, + "vmc_url": logo.VmcURL, + "bytes": len(logo.SVG), + "svg": string(logo.SVG), + "logo_url": BimiLogoURL(a.mailHost), + "certificate_url": BimiVmcURL(a.mailHost), + "certificate": a.describeCertificate(domain, logo), }) } @@ -391,9 +420,11 @@ func (a *bimiAPI) putCertificate(w http.ResponseWriter, req *http.Request, domai writeError(w, http.StatusConflict, "NO_LOGO", "Save the logo first") return } + a.changed(domain) writeJSON(w, http.StatusOK, map[string]any{ - "ok": true, - "certificate": a.describeCertificate(domain, &BimiLogo{SVG: logo.SVG, VmcPEM: pem}), + "ok": true, + "dns_publishing": a.publish != nil, + "certificate": a.describeCertificate(domain, &BimiLogo{SVG: logo.SVG, VmcPEM: pem}), }) } @@ -446,7 +477,8 @@ func (a *bimiAPI) put(w http.ResponseWriter, req *http.Request, domain string) { } // A certificate vouches for one exact SVG. Replacing the logo under a held // certificate breaks it, which receivers do not report; say so now. - response := map[string]any{"ok": true, "etag": etag} + a.changed(domain) + response := map[string]any{"ok": true, "etag": etag, "dns_publishing": a.publish != nil} if held, err := a.store.Load(req.Context(), domain); err == nil && held != nil && len(held.VmcPEM) > 0 { if cert, _ := bimi.InspectCertificate(held.VmcPEM, domain, held.SVG, a.now()); cert != nil && !cert.MatchesLogo { response["certificate_mismatch"] = true diff --git a/services/protocols/internal/presentation/http/bimi_api_test.go b/services/protocols/internal/presentation/http/bimi_api_test.go index fbbcd9e..61b1e39 100644 --- a/services/protocols/internal/presentation/http/bimi_api_test.go +++ b/services/protocols/internal/presentation/http/bimi_api_test.go @@ -335,3 +335,123 @@ func TestFetcherRefusesPrivateAndPlainHTTP(t *testing.T) { } } } + +// publishRecorder collects the domains the console asked to have published. +type publishRecorder struct{ domains chan string } + +func newPublishRecorder() *publishRecorder { return &publishRecorder{domains: make(chan string, 8)} } + +func (p *publishRecorder) record(domain string) { p.domains <- domain } + +func (p *publishRecorder) expect(t *testing.T, what string) { + t.Helper() + select { + case got := <-p.domains: + if got != "ebay.com" { + t.Errorf("%s published %q", what, got) + } + case <-time.After(2 * time.Second): + t.Fatalf("%s did not ask for the DNS record to be published", what) + } +} + +func (p *publishRecorder) expectNothing(t *testing.T, what string) { + t.Helper() + select { + case got := <-p.domains: + t.Fatalf("%s published %q though it changed nothing", what, got) + case <-time.After(150 * time.Millisecond): + } +} + +// The record in the zone follows the picture: every change to the logo or the +// certificate asks for the DNS to be reconciled, and nothing else does. +func TestBimiChangesAreHandedToThePublisher(t *testing.T) { + store := &memoryBimiStore{logos: map[string]*BimiLogo{}} + r := newBimiRouter(store) + rec := newPublishRecorder() + r.bimi.publish = rec.record + + putLogo(t, r, fixture(t, "ebay.svg")) + rec.expect(t, "saving the logo") + + putCertificate(r, fixture(t, "ebay.pem")) + rec.expect(t, "saving the certificate") + + // Reads and refused uploads change nothing and publish nothing + bimiCall(r, http.MethodGet, "/api/v1/admin/bimi?domain=ebay.com", "", true) + putCertificate(r, []byte("-----BEGIN PRIVATE KEY-----\nQUJD\n-----END PRIVATE KEY-----\n")) + putLogo(t, r, []byte("")) + rec.expectNothing(t, "a read or a refused upload") + + bimiCall(r, http.MethodDelete, "/api/v1/admin/bimi/certificate?domain=ebay.com", "", true) + rec.expect(t, "removing the certificate") + + bimiCall(r, http.MethodDelete, "/api/v1/admin/bimi?domain=ebay.com", "", true) + rec.expect(t, "removing the logo") +} + +func TestBimiWithoutAPublisherStillWorks(t *testing.T) { + r := newBimiRouter(&memoryBimiStore{logos: map[string]*BimiLogo{}}) + resp := jsonBody(t, putLogo(t, r, fixture(t, "ebay.svg"))) + if resp["ok"] != true || resp["dns_publishing"] != false { + t.Fatalf("%v", resp) + } +} + +// The check expects what the reconciler writes: with a certificate that no +// longer matches the logo that is a record without a=, and that is correct. +func TestBimiCheckExpectsWhatTheReconcilerPublishes(t *testing.T) { + store := &memoryBimiStore{logos: map[string]*BimiLogo{}} + r := newBimiRouter(store) + putLogo(t, r, fixture(t, "cnn.svg")) + store.logos["ebay.com"].VmcPEM = fixture(t, "ebay.pem") // a certificate for another picture + + r.bimi.dns = fakeBimiDNS{ + "_dmarc.ebay.com": {"v=DMARC1; p=reject"}, + "default._bimi.ebay.com": {"v=BIMI1; l=https://mail.ebay.com/.well-known/bimi/default.svg;"}, + } + body := jsonBody(t, bimiCall(r, http.MethodGet, "/api/v1/admin/bimi/check?domain=ebay.com", "", true)) + + status := map[string]string{} + detail := map[string]string{} + for _, c := range body["checks"].([]any) { + m := c.(map[string]any) + status[m["id"].(string)] = m["status"].(string) + detail[m["id"].(string)] = m["detail"].(string) + } + if status["record"] != "ok" { + t.Errorf("the record without a= is what the reconciler writes here, but the check said %s: %s", status["record"], detail["record"]) + } + if status["certificate"] != "fail" || !strings.Contains(detail["certificate"], "NOT published") { + t.Errorf("the certificate check must say it is held back: %s %s", status["certificate"], detail["certificate"]) + } +} + +// The console shows the record the reconciler writes, taken from the server +// rather than put together again in the browser. +func TestBimiGetReportsTheRecordItWouldPublish(t *testing.T) { + store := &memoryBimiStore{logos: map[string]*BimiLogo{}} + r := newBimiRouter(store) + putLogo(t, r, fixture(t, "ebay.svg")) + read := func() map[string]any { + return jsonBody(t, bimiCall(r, http.MethodGet, "/api/v1/admin/bimi?domain=ebay.com", "", true)) + } + + got := read() + if got["dns_record"] != "v=BIMI1; l=https://mail.ebay.com/.well-known/bimi/default.svg;" || got["certificate_withheld"] != "" { + t.Fatalf("logo only: %v", got) + } + + putCertificate(r, fixture(t, "ebay.pem")) + if got := read(); got["dns_record"] != "v=BIMI1; l=https://mail.ebay.com/.well-known/bimi/default.svg; a=https://mail.ebay.com/.well-known/bimi/vmc.pem;" { + t.Fatalf("with its certificate: %v", got["dns_record"]) + } + + // The logo is replaced: the certificate is held but no longer advertised + putLogo(t, r, fixture(t, "cnn.svg")) + got = read() + if got["certificate_withheld"] != "logoMismatch" || strings.Contains(got["dns_record"].(string), "a=") { + t.Fatalf("after replacing the logo: %v / %v", got["dns_record"], got["certificate_withheld"]) + } +} diff --git a/services/webmail/messages/en.json b/services/webmail/messages/en.json index 56fe4d3..39083f4 100644 --- a/services/webmail/messages/en.json +++ b/services/webmail/messages/en.json @@ -187,7 +187,13 @@ "bimiError_noSize": "The SVG has no size (viewBox, or width and height), so it cannot be positioned.", "bimiError_empty": "No drawing could be found in that file.", "bimiError_notImage": "Choose a PNG or SVG file.", - "bimiError_tooComplex": "The drawing is too complex to fit in 32 KB." + "bimiError_tooComplex": "The drawing is too complex to fit in 32 KB.", + "bimiPublishNow": "Publish to DNS now", + "bimiPublishing": "Publishing...", + "bimiPublished": "Published to DNS: {created} created, {updated} updated, {deleted} removed.", + "bimiPublishUnavailable": "Publishing to DNS is not set up in this environment (the Cloudflare token is missing).", + "bimiAutoPublish": "When you save or remove the logo or the certificate, the record is published to DNS automatically.", + "bimiWithheld": "The stored certificate is not being advertised in DNS because it no longer applies to this logo ({reason}). Without it the record stays valid for providers that do not require a certificate." }, "auth": { "adminLoginTitle": "Admin console", diff --git a/services/webmail/messages/es.json b/services/webmail/messages/es.json index e72d6eb..a68caa3 100644 --- a/services/webmail/messages/es.json +++ b/services/webmail/messages/es.json @@ -187,7 +187,13 @@ "bimiError_noSize": "El SVG no tiene tamaño (viewBox, o width y height), así que no se puede colocar.", "bimiError_empty": "No se encontró ningún dibujo en ese archivo.", "bimiError_notImage": "Elija un archivo PNG o SVG.", - "bimiError_tooComplex": "El dibujo es demasiado complejo para caber en 32 KB." + "bimiError_tooComplex": "El dibujo es demasiado complejo para caber en 32 KB.", + "bimiPublishNow": "Publicar en el DNS ahora", + "bimiPublishing": "Publicando...", + "bimiPublished": "Publicado en el DNS: {created} creado(s), {updated} actualizado(s), {deleted} eliminado(s).", + "bimiPublishUnavailable": "La publicación en el DNS no está configurada en este entorno (falta el token de Cloudflare).", + "bimiAutoPublish": "Al guardar o quitar el logotipo o el certificado, el registro se publica en el DNS automáticamente.", + "bimiWithheld": "El certificado guardado no se está anunciando en el DNS porque ya no corresponde a este logotipo ({reason}). Sin él, el registro sigue siendo válido para los proveedores que no exigen certificado." }, "auth": { "adminLoginTitle": "Consola de administración", diff --git a/services/webmail/messages/pt-BR.json b/services/webmail/messages/pt-BR.json index 2799a1e..3930a8c 100644 --- a/services/webmail/messages/pt-BR.json +++ b/services/webmail/messages/pt-BR.json @@ -187,7 +187,13 @@ "bimiError_noSize": "O SVG não tem tamanho (viewBox, ou width e height), por isso não dá para posicioná-lo.", "bimiError_empty": "Não foi possível encontrar um desenho nesse arquivo.", "bimiError_notImage": "Escolha um arquivo PNG ou SVG.", - "bimiError_tooComplex": "O desenho é complexo demais para caber em 32 KB." + "bimiError_tooComplex": "O desenho é complexo demais para caber em 32 KB.", + "bimiPublishNow": "Publicar no DNS agora", + "bimiPublishing": "Publicando...", + "bimiPublished": "Publicado no DNS: {created} criado(s), {updated} atualizado(s), {deleted} removido(s).", + "bimiPublishUnavailable": "A publicação no DNS não está configurada neste ambiente (falta o token do Cloudflare).", + "bimiAutoPublish": "Ao salvar ou remover o logotipo ou o certificado, o registro é publicado no DNS automaticamente.", + "bimiWithheld": "O certificado guardado não está sendo anunciado no DNS porque não vale mais para este logotipo ({reason}). Sem ele, o registro continua válido para os provedores que não exigem certificado." }, "auth": { "adminLoginTitle": "Console de administração", diff --git a/services/webmail/src/components/BimiPanel.tsx b/services/webmail/src/components/BimiPanel.tsx index 4ac67e4..e878545 100644 --- a/services/webmail/src/components/BimiPanel.tsx +++ b/services/webmail/src/components/BimiPanel.tsx @@ -1,7 +1,7 @@ "use client"; import React, { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { Upload, ZoomIn, ZoomOut, Download, Trash2, CheckCircle2, AlertTriangle, Copy, Play } from "lucide-react"; +import { Upload, ZoomIn, ZoomOut, Download, Trash2, CheckCircle2, AlertTriangle, Copy, Play, CloudUpload } from "lucide-react"; import { Badge } from "./ui/Badge"; import { Button } from "./ui/Button"; import { useTranslations } from "../i18n/provider"; @@ -50,6 +50,17 @@ interface BimiState { logo_url?: string; certificate_url?: string; certificate?: CertificateInfo; + /** The TXT record as the reconciler writes it. */ + dns_record?: string; + /** Why a certificate that is held is left out of that record, if it is. */ + certificate_withheld?: string; + /** Whether saving a change publishes the record by itself. */ + dns_publishing?: boolean; +} + +interface PublishOutcome { + ok: boolean; + message: string; } interface CheckResult { @@ -131,6 +142,9 @@ export function BimiPanel({ domain, onNotice }: BimiPanelProps) { const [pem, setPem] = useState(""); const [certProblems, setCertProblems] = useState([]); + const [publishing, setPublishing] = useState(false); + const [published, setPublished] = useState(null); + const [checks, setChecks] = useState<{ ready: boolean; checks: CheckResult[] } | null>(null); const [checking, setChecking] = useState(false); const [copied, setCopied] = useState(false); @@ -160,6 +174,7 @@ export function BimiPanel({ domain, onNotice }: BimiPanelProps) { setProblems([]); setError(null); setChecks(null); + setPublished(null); setPem(""); setCertProblems([]); setTitle(domain); @@ -359,6 +374,36 @@ export function BimiPanel({ domain, onNotice }: BimiPanelProps) { } }; + // -------------------------------------------------------------- publish + + /** Writes the domain's records to the DNS provider now, the same sync that runs on a timer. */ + const publishNow = async () => { + setPublishing(true); + setPublished(null); + try { + const res = await fetch(`/api/v1/admin/dns/reconcile?${q}`, { method: "POST" }); + const data = await res.json().catch(() => ({})); + if (res.status === 503) { + setPublished({ ok: false, message: t("admin.bimiPublishUnavailable") }); + } else if (!res.ok) { + setPublished({ ok: false, message: data.message ?? t("errors.serverError") }); + } else { + const problems = [...(data.errors ?? []), ...(data.conflicts ?? [])]; + setPublished({ + ok: problems.length === 0, + message: + t("admin.bimiPublished", { created: data.created ?? 0, updated: data.updated ?? 0, deleted: data.deleted ?? 0 }) + + (problems.length ? ` ${problems.join("; ")}` : ""), + }); + setChecks(null); + } + } catch { + setPublished({ ok: false, message: t("errors.serverError") }); + } finally { + setPublishing(false); + } + }; + // --------------------------------------------------------------- check const runCheck = async () => { @@ -374,9 +419,7 @@ export function BimiPanel({ domain, onNotice }: BimiPanelProps) { } }; - const record = bimi?.configured - ? `v=BIMI1; l=${bimi.logo_url};${bimi.certificate?.present ? ` a=${bimi.certificate_url};` : vmcUrl.trim() ? ` a=${vmcUrl.trim()};` : ""}` - : ""; + const record = bimi?.dns_record ?? ""; const copyRecord = async () => { try { @@ -699,14 +742,30 @@ export function BimiPanel({ domain, onNotice }: BimiPanelProps) {
default._bimi.{domain} TXT {record} -
+
+
+ {bimi?.dns_publishing &&

{t("admin.bimiAutoPublish")}

} + {bimi?.certificate_withheld && ( +

+ {t("admin.bimiWithheld", { reason: bimi.certificate_withheld })} +

+ )} + {published && ( +

+ {published.message} +

+ )} +

{t("admin.bimiCheckTitle")}