fix(dns): register separate handlers for each DNS record to serve multiple records
The previous implementation grouped records by domain and only returned the first matching record. This prevented serving multiple NS records for a domain, which caused GoDaddy to reject the nameservers. Changes: - Modified registerDnsRecords to register a separate handler for each record - This works around smartdns limitation where it breaks after first handler match - Now all NS records are properly served in DNS responses - Added readme.smartdns.md documenting the underlying issue in smartdns module The root cause is in smartdns DnsServer which breaks after finding the first matching handler, preventing multiple records of the same type from being served.
This commit is contained in:
126
readme.smartdns.md
Normal file
126
readme.smartdns.md
Normal file
@ -0,0 +1,126 @@
|
||||
# SmartDNS DnsServer Issues and Required Changes
|
||||
|
||||
## Current Limitation: Cannot Serve Multiple Records of Same Type
|
||||
|
||||
### Problem Description
|
||||
The current implementation of `@push.rocks/smartdns` DnsServer has a critical limitation that prevents it from serving multiple DNS records of the same type for a domain. This is particularly problematic for:
|
||||
- **NS records** - Most domains need 2+ nameservers for redundancy
|
||||
- **A/AAAA records** - Round-robin DNS requires multiple IPs
|
||||
- **MX records** - Multiple mail servers with different priorities
|
||||
- **TXT records** - Multiple TXT records for SPF, DKIM, domain verification, etc.
|
||||
|
||||
### Root Cause
|
||||
In the `processDnsRequest` method of DnsServer, when processing DNS queries:
|
||||
|
||||
```javascript
|
||||
for (const handlerEntry of this.handlers) {
|
||||
if (plugins.minimatch.minimatch(question.name, handlerEntry.domainPattern) &&
|
||||
handlerEntry.recordTypes.includes(question.type)) {
|
||||
const answer = handlerEntry.handler(question);
|
||||
if (answer) {
|
||||
response.answers.push(dnsAnswer);
|
||||
answered = true;
|
||||
break; // <-- PROBLEM: Exits after first matching handler!
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
The `break` statement causes the loop to exit after the first matching handler returns an answer, preventing subsequent handlers from being called.
|
||||
|
||||
### Impact
|
||||
- **NS Records**: Only one nameserver is returned instead of multiple
|
||||
- **Domain Registration Failures**: Registrars like GoDaddy reject domains with only one NS record
|
||||
- **No Round-Robin**: Cannot implement round-robin DNS with multiple A records
|
||||
- **Limited TXT Records**: Cannot serve multiple TXT records (SPF + DKIM + others)
|
||||
|
||||
### Required Changes
|
||||
|
||||
#### Option 1: Continue Processing All Handlers (Recommended)
|
||||
Remove the `break` statement to allow all matching handlers to contribute answers:
|
||||
|
||||
```javascript
|
||||
for (const handlerEntry of this.handlers) {
|
||||
if (plugins.minimatch.minimatch(question.name, handlerEntry.domainPattern) &&
|
||||
handlerEntry.recordTypes.includes(question.type)) {
|
||||
const answer = handlerEntry.handler(question);
|
||||
if (answer) {
|
||||
response.answers.push(dnsAnswer);
|
||||
if (dnssecRequested) {
|
||||
const rrsig = this.generateRRSIG(question.type, [dnsAnswer], question.name);
|
||||
response.answers.push(rrsig);
|
||||
}
|
||||
answered = true;
|
||||
// Don't break - continue checking other handlers
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
#### Option 2: Handler Returns Array
|
||||
Modify the handler interface to allow returning multiple answers:
|
||||
|
||||
```typescript
|
||||
export interface IDnsHandler {
|
||||
domainPattern: string;
|
||||
recordTypes: string[];
|
||||
handler: (question: dnsPacket.Question) => DnsAnswer | DnsAnswer[] | null;
|
||||
}
|
||||
```
|
||||
|
||||
Then update processDnsRequest to handle arrays:
|
||||
|
||||
```javascript
|
||||
const answer = handlerEntry.handler(question);
|
||||
if (answer) {
|
||||
const answers = Array.isArray(answer) ? answer : [answer];
|
||||
for (const ans of answers) {
|
||||
response.answers.push({...ans, ttl: ans.ttl || 300, class: ans.class || 'IN'});
|
||||
}
|
||||
answered = true;
|
||||
break; // Can break here since handler returns all records
|
||||
}
|
||||
```
|
||||
|
||||
### Workaround (Current Implementation)
|
||||
Currently working around this by registering separate handlers for each record. This is inefficient but functional:
|
||||
|
||||
```typescript
|
||||
// Instead of one handler for multiple NS records
|
||||
// We register separate handlers for each NS record
|
||||
for (const record of records) {
|
||||
this.dnsServer.registerHandler(record.name, [record.type], (question) => {
|
||||
if (question.name === record.name && question.type === record.type) {
|
||||
return { /* single record */ };
|
||||
}
|
||||
return null;
|
||||
});
|
||||
}
|
||||
```
|
||||
|
||||
### Other Issues Found
|
||||
|
||||
#### 1. SOA Record Timeout
|
||||
SOA queries sometimes timeout or return incorrect data. This may be related to the DNSSEC implementation or SOA record generation.
|
||||
|
||||
#### 2. DNSSEC Zone Prefix
|
||||
The DnsServer automatically prefixes 'ns1.' to the dnssecZone:
|
||||
```javascript
|
||||
mname: `ns1.${this.options.dnssecZone}`
|
||||
```
|
||||
This can cause mismatches when the actual nameserver doesn't have the 'ns1.' prefix.
|
||||
|
||||
### Testing the Fix
|
||||
After implementing Option 1 or 2, test with:
|
||||
|
||||
```bash
|
||||
# Should return BOTH nameservers
|
||||
dig @nameserver.example.com domain.com NS
|
||||
|
||||
# Should show multiple NS records in answer section
|
||||
domain.com. 3600 IN NS ns1.example.com.
|
||||
domain.com. 3600 IN NS ns2.example.com.
|
||||
```
|
||||
|
||||
### Priority
|
||||
**HIGH** - This issue prevents proper DNS server operation and blocks domain registration at many registrars.
|
||||
@ -693,40 +693,27 @@ export class DcRouter {
|
||||
private registerDnsRecords(records: Array<{name: string; type: string; value: string; ttl?: number}>): void {
|
||||
if (!this.dnsServer) return;
|
||||
|
||||
// Group records by domain pattern
|
||||
const recordsByDomain = new Map<string, typeof records>();
|
||||
|
||||
// Register a separate handler for each record
|
||||
// This ensures multiple records of the same type (like NS records) are all served
|
||||
for (const record of records) {
|
||||
// Use exact domain name for registration - no automatic wildcard prefix
|
||||
const pattern = record.name;
|
||||
if (!recordsByDomain.has(pattern)) {
|
||||
recordsByDomain.set(pattern, []);
|
||||
}
|
||||
recordsByDomain.get(pattern)!.push(record);
|
||||
}
|
||||
|
||||
// Register handlers for each domain pattern
|
||||
for (const [domainPattern, domainRecords] of recordsByDomain) {
|
||||
const recordTypes = [...new Set(domainRecords.map(r => r.type))];
|
||||
|
||||
this.dnsServer.registerHandler(domainPattern, recordTypes, (question) => {
|
||||
const matchingRecord = domainRecords.find(
|
||||
r => r.name === question.name && r.type === question.type
|
||||
);
|
||||
|
||||
if (matchingRecord) {
|
||||
// Register handler for this specific record
|
||||
this.dnsServer.registerHandler(record.name, [record.type], (question) => {
|
||||
// Check if this handler matches the question
|
||||
if (question.name === record.name && question.type === record.type) {
|
||||
return {
|
||||
name: matchingRecord.name,
|
||||
type: matchingRecord.type,
|
||||
name: record.name,
|
||||
type: record.type,
|
||||
class: 'IN',
|
||||
ttl: matchingRecord.ttl || 300,
|
||||
data: this.parseDnsRecordData(matchingRecord.type, matchingRecord.value)
|
||||
ttl: record.ttl || 300,
|
||||
data: this.parseDnsRecordData(record.type, record.value)
|
||||
};
|
||||
}
|
||||
|
||||
return null;
|
||||
});
|
||||
}
|
||||
|
||||
logger.log('info', `Registered ${records.length} DNS handlers (one per record)`);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user