Skip to content

Commit c941f89

Browse files
committed
fix: address PR review — CORS headers, FQDN normalization, TXT quoting, RR validation, DB index
- CORS: restore X-Api-User and X-Api-Key to AllowedHeaders (required by /update endpoint) - api: normalize record names to FQDN (trailing dot) on create/update/list-filter so DNS lookups against q.Name (which always ends with dot) match stored records - api: normalize type filter to uppercase for case-insensitive GET /admin/records?type= - api: add probeRR helper — validates record value parses as a real DNS RR before storing, preventing values that are silently dropped at serve time - dns: guard answerManaged against unknown QTYPEs (empty TypeToString result) - dns: quote TXT and CAA values in RR string so values with spaces parse correctly - db: add index on dns_records(name, type) to avoid full table scans on every DNS request
1 parent 686e796 commit c941f89

4 files changed

Lines changed: 51 additions & 6 deletions

File tree

api.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,31 @@ import (
88
"fmt"
99
"github.com/google/uuid"
1010
"github.com/julienschmidt/httprouter"
11+
"github.com/miekg/dns"
1112
log "github.com/sirupsen/logrus"
1213
"io"
1314
"net/http"
1415
"strings"
1516
"time"
1617
)
1718

19+
// toFQDN ensures a DNS name ends with a trailing dot for consistent storage and lookup.
20+
func toFQDN(name string) string {
21+
name = strings.ToLower(name)
22+
if !strings.HasSuffix(name, ".") {
23+
name += "."
24+
}
25+
return name
26+
}
27+
28+
// probeRR validates that rtype+value form a parseable DNS RR using a placeholder name and TTL.
29+
func probeRR(rtype, value string) bool {
30+
if rtype == "TXT" || rtype == "CAA" {
31+
value = `"` + strings.ReplaceAll(value, `"`, `\"`) + `"`
32+
}
33+
_, err := dns.NewRR(fmt.Sprintf("probe.invalid. 300 IN %s %s", rtype, value))
34+
return err == nil
35+
}
1836
// RegResponse is a struct for registration response JSON
1937
type RegResponse struct {
2038
Username string `json:"username"`
@@ -147,8 +165,11 @@ func adminBearerMiddleware(next httprouter.Handle) httprouter.Handle {
147165
}
148166

149167
func adminListRecords(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
150-
filterType := r.URL.Query().Get("type")
168+
filterType := strings.ToUpper(r.URL.Query().Get("type"))
151169
filterName := r.URL.Query().Get("name")
170+
if filterName != "" {
171+
filterName = toFQDN(filterName)
172+
}
152173
records, err := DB.ListRecords(filterType, filterName)
153174
if err != nil {
154175
log.WithFields(log.Fields{"error": err.Error()}).Error("Error in admin handler")
@@ -188,6 +209,12 @@ func adminCreateRecord(w http.ResponseWriter, r *http.Request, _ httprouter.Para
188209
_, _ = w.Write(jsonError("invalid_record_value"))
189210
return
190211
}
212+
if !probeRR(req.Type, req.Value) {
213+
w.Header().Set("Content-Type", "application/json")
214+
w.WriteHeader(http.StatusBadRequest)
215+
_, _ = w.Write(jsonError("invalid_record_value"))
216+
return
217+
}
191218
ttl := req.TTL
192219
if ttl == 0 {
193220
ttl = 300
@@ -200,7 +227,7 @@ func adminCreateRecord(w http.ResponseWriter, r *http.Request, _ httprouter.Para
200227
}
201228
rec := DNSRecord{
202229
ID: uuid.New().String(),
203-
Name: strings.ToLower(req.Name),
230+
Name: toFQDN(req.Name),
204231
Type: req.Type,
205232
Value: req.Value,
206233
TTL: ttl,
@@ -245,6 +272,12 @@ func adminUpdateRecord(w http.ResponseWriter, r *http.Request, ps httprouter.Par
245272
_, _ = w.Write(jsonError("invalid_record_value"))
246273
return
247274
}
275+
if !probeRR(req.Type, req.Value) {
276+
w.Header().Set("Content-Type", "application/json")
277+
w.WriteHeader(http.StatusBadRequest)
278+
_, _ = w.Write(jsonError("invalid_record_value"))
279+
return
280+
}
248281
if req.TTL != 0 && !validTTL(req.TTL) {
249282
w.Header().Set("Content-Type", "application/json")
250283
w.WriteHeader(http.StatusBadRequest)
@@ -255,7 +288,7 @@ func adminUpdateRecord(w http.ResponseWriter, r *http.Request, ps httprouter.Par
255288
if ttl == 0 {
256289
ttl = 300
257290
}
258-
rec := DNSRecord{ID: id, Name: strings.ToLower(req.Name), Type: req.Type, Value: req.Value, TTL: ttl}
291+
rec := DNSRecord{ID: id, Name: toFQDN(req.Name), Type: req.Type, Value: req.Value, TTL: ttl}
259292
if err := DB.UpdateRecord(rec); err != nil {
260293
w.Header().Set("Content-Type", "application/json")
261294
if errors.Is(err, sql.ErrNoRows) {

db.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ var dnsRecordsTable = `
5858
created INTEGER NOT NULL
5959
);`
6060

61+
var dnsRecordsIndex = `
62+
CREATE INDEX IF NOT EXISTS idx_dns_records_name_type ON dns_records (name, type);`
63+
6164
// getSQLiteStmt replaces all PostgreSQL prepared statement placeholders (eg. $1, $2) with SQLite variant "?"
6265
func getSQLiteStmt(s string) string {
6366
re, _ := regexp.Compile(`\$[0-9]`)
@@ -91,6 +94,7 @@ func (d *acmedb) Init(engine string, connection string) error {
9194
_, _ = d.DB.Exec(txtTablePG)
9295
}
9396
_, _ = d.DB.Exec(dnsRecordsTable)
97+
_, _ = d.DB.Exec(dnsRecordsIndex)
9498
// If everything is fine, handle db upgrade tasks
9599
if err == nil {
96100
err = d.checkDBUpgrades(versionString)

dns.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,15 +249,23 @@ func (d *DNSServer) answerOwnChallenge(q dns.Question) ([]dns.RR, error) {
249249
}
250250

251251
func (d *DNSServer) answerManaged(q dns.Question) []dns.RR {
252+
qtype := dns.TypeToString[q.Qtype]
253+
if qtype == "" {
254+
return nil
255+
}
252256
name := strings.ToLower(q.Name)
253-
records, err := d.DB.ListRecords(dns.TypeToString[q.Qtype], name)
257+
records, err := d.DB.ListRecords(qtype, name)
254258
if err != nil {
255259
log.WithFields(log.Fields{"error": err.Error()}).Debug("Error querying managed records")
256260
return nil
257261
}
258262
var rrs []dns.RR
259263
for _, rec := range records {
260-
rrStr := fmt.Sprintf("%s %d IN %s %s", rec.Name, rec.TTL, rec.Type, rec.Value)
264+
value := rec.Value
265+
if rec.Type == "TXT" || rec.Type == "CAA" {
266+
value = `"` + strings.ReplaceAll(value, `"`, `\"`) + `"`
267+
}
268+
rrStr := fmt.Sprintf("%s %d IN %s %s", rec.Name, rec.TTL, rec.Type, value)
261269
rr, err := dns.NewRR(rrStr)
262270
if err != nil {
263271
log.WithFields(log.Fields{"error": err.Error(), "record": rrStr}).Warning("Could not parse managed RR")

main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func startHTTPAPI(errChan chan error, config DNSConfig, dnsServers []*DNSServer)
118118
c := cors.New(cors.Options{
119119
AllowedOrigins: config.API.CorsOrigins,
120120
AllowedMethods: []string{"GET", "POST", "PUT", "DELETE"},
121-
AllowedHeaders: []string{"Authorization", "Content-Type"},
121+
AllowedHeaders: []string{"Authorization", "Content-Type", "X-Api-User", "X-Api-Key"},
122122
OptionsPassthrough: false,
123123
Debug: config.General.Debug,
124124
})

0 commit comments

Comments
 (0)