#27: never publish a DKIM record with an empty key, and say why one is missing

A fresh deployment published "v=DKIM1; k=rsa; p=" for its own domain. An empty
p= is not an absent key, it is a revoked one (RFC 6376 section 3.6.1): every
verifier reading it treats all signatures from that selector as invalid. On a
domain whose DMARC policy is p=quarantine with strict alignment - which this
reconciler generates - that quarantines all of its own outbound mail. Worse
than publishing nothing.

The chain: DkimRepository.Insert is an INSERT ... SELECT conditional on the
domain existing in the domains table, so on an empty database it inserts no
rows and returns no error. FindPublicKey then finds nothing, ensure() returns
an empty string with no error, and the reconciler faithfully published the
empty value. Every field along that path was behaving as written; nothing
checked that the result meant anything.

Two changes:

- The record builder omits a DKIM record when its key is empty, so the
  revocation cannot be published regardless of why the key is missing.
- Provisioning logs when it produced no key, naming the likely cause. The
  insert being conditional on an onboarded domain is exactly the case a new
  operator hits, and "no key, domain probably not onboarded" is the difference
  between a diagnosis and unsigned mail with no explanation. It stays
  non-fatal: erroring here would abort the whole reconcile and leave a new
  install with no A, MX or SPF records either.

Found while migrating to a new host: the fresh database had no domain row, so
the first reconcile revoked the domain's DKIM. The two published records have
been deleted from the zone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

services/protocols/internal/application/usecase/provision_dkim_keys.go

@@ -3,6 +3,7 @@ package usecase
 import (
 	"context"
 	"fmt"
+	"log"
 )
 
 // DkimKeyProvisioner is the storage side of key provisioning. It is declared
@@ -95,6 +96,17 @@ func (uc *ProvisionDkimKeysUseCase) ensure(ctx context.Context, domainName, algo
 		return "", fmt.Errorf("confirm %s key for %s: %w", algorithm, domainName, err)
 	}
 
+	// The insert is conditional on the domain existing, and inserting nothing
+	// is not an error at the SQL level - so a domain that has never been
+	// onboarded silently produces no key here. Saying so is the difference
+	// between an operator seeing "DKIM is not set up yet, onboard the domain"
+	// and seeing unsigned mail with no explanation.
+	if stored == "" {
+		log.Printf("dkim: no %s key for %s after provisioning - the domain is probably not onboarded yet, so mail from it will not be signed",
+			algorithm, domainName)
+		return "", nil
+	}
+
 	out.Created = append(out.Created, algorithm)
 	return stored, nil
 }

services/protocols/internal/domain/entity/dns_record.go

@@ -77,22 +77,6 @@ func BuildDnsRecordSpecs(spec DnsRecordSpec) []DnsRecord {
 			TTL:     1,
 			Comment: "LambdaMail SPF Record",
 		},
-		// 5. RSA DKIM TXT record
-		{
-			Type:    "TXT",
-			Name:    fmt.Sprintf("default._domainkey.%s", domainName),
-			Value:   fmt.Sprintf("v=DKIM1; k=rsa; p=%s", rsaDkimPubKey),
-			TTL:     1,
-			Comment: "LambdaMail RSA DKIM Key",
-		},
-		// 6. Ed25519 DKIM TXT record
-		{
-			Type:    "TXT",
-			Name:    fmt.Sprintf("default-ed._domainkey.%s", domainName),
-			Value:   fmt.Sprintf("v=DKIM1; k=ed25519; p=%s", edDkimPubKey),
-			TTL:     1,
-			Comment: "LambdaMail Ed25519 DKIM Key",
-		},
 		// 7. DMARC TXT record
 		{
 			Type:    "TXT",
@@ -173,6 +157,34 @@ func BuildDnsRecordSpecs(spec DnsRecordSpec) []DnsRecord {
 		},
 	}
 
+	// 5 and 6. DKIM records, published only once there is a key to put in them.
+	//
+	// "p=" with nothing after it is not an absent key, it is a revoked one
+	// (RFC 6376 section 3.6.1): a verifier reading it treats every signature
+	// from that selector as broken. These used to be emitted unconditionally,
+	// so a deployment that had not provisioned keys yet - a fresh database, or
+	// a missing master key - published a revocation for its own domain. That is
+	// strictly worse than publishing nothing, and on a domain with a strict
+	// DMARC policy it quarantines all of its own outbound mail.
+	if rsaDkimPubKey != "" {
+		records = append(records, DnsRecord{
+			Type:    "TXT",
+			Name:    fmt.Sprintf("default._domainkey.%s", domainName),
+			Value:   fmt.Sprintf("v=DKIM1; k=rsa; p=%s", rsaDkimPubKey),
+			TTL:     1,
+			Comment: "LambdaMail RSA DKIM Key",
+		})
+	}
+	if edDkimPubKey != "" {
+		records = append(records, DnsRecord{
+			Type:    "TXT",
+			Name:    fmt.Sprintf("default-ed._domainkey.%s", domainName),
+			Value:   fmt.Sprintf("v=DKIM1; k=ed25519; p=%s", edDkimPubKey),
+			TTL:     1,
+			Comment: "LambdaMail Ed25519 DKIM Key",
+		})
+	}
+
 	// 2. Conditional AAAA record if IPv6 is supplied
 	if serverIPv6 != "" {
 		records = append(records, DnsRecord{

services/protocols/internal/domain/entity/dns_record_test.go

@@ -1,6 +1,7 @@
 package entity
 
 import (
+	"strings"
 	"testing"
 )
 
@@ -68,3 +69,66 @@ func TestBuildDnsRecordSpecs_GeneratesMandatoryRecords(t *testing.T) {
 		t.Error("DMARC record not found in specs")
 	}
 }
+
+// An empty "p=" is a revocation, not an absent key (RFC 6376 section 3.6.1).
+// Publishing one for a domain that simply has not provisioned keys yet tells
+// every verifier that the selector's signatures are invalid - and on a domain
+// whose DMARC policy is strict, that quarantines all of its own outbound mail.
+func TestDkimRecordsAreOmittedWhenThereIsNoKey(t *testing.T) {
+	records := BuildDnsRecordSpecs(DnsRecordSpec{
+		DomainName: "example.test",
+		MailHost:   "mail.example.test",
+		ServerIPv4: "198.51.100.10",
+	})
+
+	for _, r := range records {
+		if strings.Contains(r.Name, "_domainkey") {
+			t.Errorf("published a DKIM record with no key: %s = %q", r.Name, r.Value)
+		}
+		if strings.Contains(r.Value, "p=\"") || strings.HasSuffix(r.Value, "p=") {
+			t.Errorf("published an empty public key: %s = %q", r.Name, r.Value)
+		}
+	}
+}
+
+func TestDkimRecordsArePublishedWhenKeysExist(t *testing.T) {
+	records := BuildDnsRecordSpecs(DnsRecordSpec{
+		DomainName:    "example.test",
+		MailHost:      "mail.example.test",
+		ServerIPv4:    "198.51.100.10",
+		RsaDkimPubKey: "MIIBIjANBgkq",
+		EdDkimPubKey:  "11qYAYKxCrfV",
+	})
+
+	var rsa, ed bool
+	for _, r := range records {
+		if r.Name == "default._domainkey.example.test" && strings.Contains(r.Value, "MIIBIjANBgkq") {
+			rsa = true
+		}
+		if r.Name == "default-ed._domainkey.example.test" && strings.Contains(r.Value, "11qYAYKxCrfV") {
+			ed = true
+		}
+	}
+	if !rsa {
+		t.Error("the RSA DKIM record was not published despite a key being present")
+	}
+	if !ed {
+		t.Error("the Ed25519 DKIM record was not published despite a key being present")
+	}
+}
+
+// One key present and the other absent must publish exactly one record: an
+// Ed25519 rollout should not revoke the RSA selector that is still signing.
+func TestOnlyTheAvailableDkimKeyIsPublished(t *testing.T) {
+	records := BuildDnsRecordSpecs(DnsRecordSpec{
+		DomainName:    "example.test",
+		MailHost:      "mail.example.test",
+		RsaDkimPubKey: "MIIBIjANBgkq",
+	})
+
+	for _, r := range records {
+		if strings.Contains(r.Name, "default-ed._domainkey") {
+			t.Errorf("published an Ed25519 record with no key: %q", r.Value)
+		}
+	}
+}