Skip to content

Commit d8fbc00

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 c23fe5f commit d8fbc00

4 files changed

Lines changed: 52 additions & 6 deletions

File tree

api.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,32 @@ import (
1515

1616
"github.com/google/uuid"
1717
"github.com/julienschmidt/httprouter"
18+
"github.com/miekg/dns"
1819
log "github.com/sirupsen/logrus"
1920
"io"
2021
)
2122

2223
//go:embed openapi.json
2324
var openapiSpec []byte
2425

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

162181
func adminListRecords(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
163-
filterType := r.URL.Query().Get("type")
182+
filterType := strings.ToUpper(r.URL.Query().Get("type"))
164183
filterName := r.URL.Query().Get("name")
184+
if filterName != "" {
185+
filterName = toFQDN(filterName)
186+
}
165187
records, err := DB.ListRecords(filterType, filterName)
166188
if err != nil {
167189
log.WithFields(log.Fields{"error": err.Error()}).Error("Error in admin handler")
@@ -201,6 +223,12 @@ func adminCreateRecord(w http.ResponseWriter, r *http.Request, _ httprouter.Para
201223
_, _ = w.Write(jsonError("invalid_record_value"))
202224
return
203225
}
226+
if !probeRR(req.Type, req.Value) {
227+
w.Header().Set("Content-Type", "application/json")
228+
w.WriteHeader(http.StatusBadRequest)
229+
_, _ = w.Write(jsonError("invalid_record_value"))
230+
return
231+
}
204232
ttl := req.TTL
205233
if ttl == 0 {
206234
ttl = 300
@@ -213,7 +241,7 @@ func adminCreateRecord(w http.ResponseWriter, r *http.Request, _ httprouter.Para
213241
}
214242
rec := DNSRecord{
215243
ID: uuid.New().String(),
216-
Name: strings.ToLower(req.Name),
244+
Name: toFQDN(req.Name),
217245
Type: req.Type,
218246
Value: req.Value,
219247
TTL: ttl,
@@ -258,6 +286,12 @@ func adminUpdateRecord(w http.ResponseWriter, r *http.Request, ps httprouter.Par
258286
_, _ = w.Write(jsonError("invalid_record_value"))
259287
return
260288
}
289+
if !probeRR(req.Type, req.Value) {
290+
w.Header().Set("Content-Type", "application/json")
291+
w.WriteHeader(http.StatusBadRequest)
292+
_, _ = w.Write(jsonError("invalid_record_value"))
293+
return
294+
}
261295
if req.TTL != 0 && !validTTL(req.TTL) {
262296
w.Header().Set("Content-Type", "application/json")
263297
w.WriteHeader(http.StatusBadRequest)
@@ -268,7 +302,7 @@ func adminUpdateRecord(w http.ResponseWriter, r *http.Request, ps httprouter.Par
268302
if ttl == 0 {
269303
ttl = 300
270304
}
271-
rec := DNSRecord{ID: id, Name: strings.ToLower(req.Name), Type: req.Type, Value: req.Value, TTL: ttl}
305+
rec := DNSRecord{ID: id, Name: toFQDN(req.Name), Type: req.Type, Value: req.Value, TTL: ttl}
272306
if err := DB.UpdateRecord(rec); err != nil {
273307
w.Header().Set("Content-Type", "application/json")
274308
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)