Skip to content

Commit e929f6d

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 8197c0a commit e929f6d

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
@@ -14,10 +14,28 @@ import (
1414

1515
"github.com/google/uuid"
1616
"github.com/julienschmidt/httprouter"
17+
"github.com/miekg/dns"
1718
log "github.com/sirupsen/logrus"
1819
"io"
1920
)
2021

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

152170
func adminListRecords(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
153-
filterType := r.URL.Query().Get("type")
171+
filterType := strings.ToUpper(r.URL.Query().Get("type"))
154172
filterName := r.URL.Query().Get("name")
173+
if filterName != "" {
174+
filterName = toFQDN(filterName)
175+
}
155176
records, err := DB.ListRecords(filterType, filterName)
156177
if err != nil {
157178
log.WithFields(log.Fields{"error": err.Error()}).Error("Error in admin handler")
@@ -191,6 +212,12 @@ func adminCreateRecord(w http.ResponseWriter, r *http.Request, _ httprouter.Para
191212
_, _ = w.Write(jsonError("invalid_record_value"))
192213
return
193214
}
215+
if !probeRR(req.Type, req.Value) {
216+
w.Header().Set("Content-Type", "application/json")
217+
w.WriteHeader(http.StatusBadRequest)
218+
_, _ = w.Write(jsonError("invalid_record_value"))
219+
return
220+
}
194221
ttl := req.TTL
195222
if ttl == 0 {
196223
ttl = 300
@@ -203,7 +230,7 @@ func adminCreateRecord(w http.ResponseWriter, r *http.Request, _ httprouter.Para
203230
}
204231
rec := DNSRecord{
205232
ID: uuid.New().String(),
206-
Name: strings.ToLower(req.Name),
233+
Name: toFQDN(req.Name),
207234
Type: req.Type,
208235
Value: req.Value,
209236
TTL: ttl,
@@ -248,6 +275,12 @@ func adminUpdateRecord(w http.ResponseWriter, r *http.Request, ps httprouter.Par
248275
_, _ = w.Write(jsonError("invalid_record_value"))
249276
return
250277
}
278+
if !probeRR(req.Type, req.Value) {
279+
w.Header().Set("Content-Type", "application/json")
280+
w.WriteHeader(http.StatusBadRequest)
281+
_, _ = w.Write(jsonError("invalid_record_value"))
282+
return
283+
}
251284
if req.TTL != 0 && !validTTL(req.TTL) {
252285
w.Header().Set("Content-Type", "application/json")
253286
w.WriteHeader(http.StatusBadRequest)
@@ -258,7 +291,7 @@ func adminUpdateRecord(w http.ResponseWriter, r *http.Request, ps httprouter.Par
258291
if ttl == 0 {
259292
ttl = 300
260293
}
261-
rec := DNSRecord{ID: id, Name: strings.ToLower(req.Name), Type: req.Type, Value: req.Value, TTL: ttl}
294+
rec := DNSRecord{ID: id, Name: toFQDN(req.Name), Type: req.Type, Value: req.Value, TTL: ttl}
262295
if err := DB.UpdateRecord(rec); err != nil {
263296
w.Header().Set("Content-Type", "application/json")
264297
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)