Change sharedSecred checking method

The rest-api does not need to check the shared secret because bind itself
can check it. This change also allows to have different shared secrets for
different zones.

See dstapp#55

Author: Golit

Dockerfile

@@ -9,7 +9,7 @@ FROM debian:buster-slim
 MAINTAINER David Prandzioch <[email protected]>
 
 RUN DEBIAN_FRONTEND=noninteractive apt-get update && \
-	apt-get install -q -y bind9 dnsutils && \
+	apt-get install -q -y bind9 dnsutils openssl && \
 	apt-get clean
 
 RUN chmod 770 /var/cache/bind

rest-api/config.go

@@ -7,10 +7,8 @@ import (
 )
 
 type Config struct {
-	SharedSecret   string
 	Server         string
 	Zone           string
-	Domain         string
 	NsupdateBinary string
 	RecordTTL      int
 	Port           int
@@ -38,10 +36,8 @@ func (conf *Config) loadConfigFromFile(path string) {
 func (flagsConf *ConfigFlags) setupFlags() {
 	flag.BoolVar(&flagsConf.DoNotLoadConfig, "noConfig", false, "Do not load the config file")
 	flag.StringVar(&flagsConf.ConfigFile, "c", "/etc/dyndns.json", "The configuration file")
-	flag.StringVar(&flagsConf.SharedSecret, "sharedSecret", "", "The shared secret (default a generated random string)")
 	flag.StringVar(&flagsConf.Server, "server", "localhost", "The address of the bind server")
 	flag.StringVar(&flagsConf.Zone, "zone", "localhost", "Zone")
-	flag.StringVar(&flagsConf.Domain, "domain", "localhost", "Domain")
 	flag.StringVar(&flagsConf.NsupdateBinary, "nsupdateBinary", "nsupdate", "Path to nsupdate program")
 	flag.IntVar(&flagsConf.RecordTTL, "recordTTL", 300, "RecordTTL")
 	flag.IntVar(&flagsConf.Port, "p", 8080, "Port")
@@ -58,9 +54,4 @@ func (flagsConf *ConfigFlags) LoadConfig() {
 		flagsConf.loadConfigFromFile(flagsConf.ConfigFile)
 		flag.Parse() // Parse a second time to overwrite settings from the loaded file
 	}
-
-	// Fix unsafe config values
-	if flagsConf.SharedSecret == "" {
-		flagsConf.SharedSecret = randomString()
-	}
 }

rest-api/main.go

@@ -86,10 +86,10 @@ func updateDomains(r *http.Request, response *WebserviceResponse, onError func()
 	for _, address := range response.Addresses {
 		for _, domain := range response.Domains {
 			recordUpdate := RecordUpdateRequest{
-				domain:   domain,
-				ipaddr:   address.Address,
-				addrType: address.AddrType,
-				ddnskey:  extractor.DdnsKey(r),
+				domain:      domain,
+				ipAddr:      address.Address,
+				addrType:    address.AddrType,
+				secret:      extractor.Secret(r),
 			}
 			result := recordUpdate.updateRecord()
 
@@ -122,7 +122,7 @@ func (r RecordUpdateRequest) updateRecord() string {
 		status = "failed, error: " + result
 	}
 
-	log.Println(fmt.Sprintf("%s record update request: %s -> %s %s", r.addrType, r.domain, r.ipaddr, status))
+	log.Println(fmt.Sprintf("%s record update request: %s -> %s %s", r.addrType, r.domain, r.ipAddr, status))
 
 	return result
 }

rest-api/nsupdate.go

@@ -19,10 +19,13 @@ type NSUpdateInterface interface {
 
 // RecordUpdateRequest data representing a update request
 type RecordUpdateRequest struct {
-	domain   string
-	ipaddr   string
-	addrType string
-	ddnskey  string
+	domain      string
+	ipAddr      string
+	addrType    string
+	ddnsKeyName string
+	secret      string
+	zone        string
+	fqdn        string
 }
 
 // NSUpdate holds resources need for an open nsupdate program
@@ -106,16 +109,16 @@ func (nsupdate *NSUpdate) UpdateRecord(r RecordUpdateRequest) {
 		fqdn = escape(r.domain) + "." + appConfig.Zone
 	}
 
-	if r.ddnskey != "" {
-		fqdnN := strings.TrimLeft(fqdn, ".")
-		nsupdate.write("key hmac-sha256:ddns-key.%s %s\n", fqdnN, escape(r.ddnskey))
+	if r.secret != "" {
+		fqdnN := strings.TrimLeft(appConfig.Zone, ".")
+		nsupdate.write("key hmac-sha256:ddns-key.%s %s\n", fqdnN, escape(r.secret))
 	}
 
 	nsupdate.write("server %s\n", appConfig.Server)
     if appConfig.Zone != "" {
         nsupdate.write("zone %s\n", appConfig.Zone)
     }
 	nsupdate.write("update delete %s %s\n", fqdn, r.addrType)
-	nsupdate.write("update add %s %v %s %s\n", fqdn, appConfig.RecordTTL, r.addrType, escape(r.ipaddr))
+	nsupdate.write("update add %s %v %s %s\n", fqdn, appConfig.RecordTTL, r.addrType, escape(r.ipAddr))
 	nsupdate.write("send\n")
 }

rest-api/request_handler.go

@@ -39,7 +39,6 @@ func ParseAddress(address string) (Address, error) {
 func BuildWebserviceResponseFromRequest(r *http.Request, appConfig *Config, extractors requestDataExtractor) WebserviceResponse {
 	response := WebserviceResponse{}
 
-	sharedSecret := extractors.Secret(r)
 	response.Domains = strings.Split(extractors.Domain(r), ",")
 	for _, address := range strings.Split(extractors.Address(r), ",") {
 		var parsedAddress, error = ParseAddress(address)
@@ -48,8 +47,8 @@ func BuildWebserviceResponseFromRequest(r *http.Request, appConfig *Config, extr
 		}
 	}
 
-	if sharedSecret != appConfig.SharedSecret {
-		log.Println(fmt.Sprintf("Invalid shared secret: %s", sharedSecret))
+	if extractors.Secret(r) == "" { // futher checking is done by bind server as configured
+		log.Println(fmt.Sprintf("Invalid shared secret"))
 		response.Success = false
 		response.Message = "Invalid Credentials"
 		return response

rest-api/request_handler_test.go

@@ -11,7 +11,6 @@ var dynExtractor = dynRequestDataExtractor{}
 
 func TestBuildWebserviceResponseFromRequestToReturnValidObject(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=changeme&domain=foo&addr=1.2.3.4", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -35,7 +34,6 @@ func TestBuildWebserviceResponseFromRequestToReturnValidObject(t *testing.T) {
 
 func TestBuildWebserviceResponseFromRequestWithXRealIPHeaderToReturnValidObject(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=changeme&domain=foo", nil)
 	req.Header.Add("X-Real-Ip", "1.2.3.4")
@@ -60,7 +58,6 @@ func TestBuildWebserviceResponseFromRequestWithXRealIPHeaderToReturnValidObject(
 
 func TestBuildWebserviceResponseFromRequestWithXForwardedForHeaderToReturnValidObject(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=changeme&domain=foo", nil)
 	req.Header.Add("X-Forwarded-For", "1.2.3.4")
@@ -85,7 +82,6 @@ func TestBuildWebserviceResponseFromRequestWithXForwardedForHeaderToReturnValidO
 
 func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenNoSecretIsGiven(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -97,7 +93,6 @@ func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenNoSecretIsGi
 
 func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenInvalidSecretIsGiven(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=foo", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -109,7 +104,6 @@ func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenInvalidSecre
 
 func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenNoDomainIsGiven(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=changeme", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -121,7 +115,6 @@ func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenNoDomainIsGi
 
 func TestBuildWebserviceResponseFromRequestWithMultipleDomains(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=changeme&domain=foo,bar&addr=1.2.3.4", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -145,7 +138,6 @@ func TestBuildWebserviceResponseFromRequestWithMultipleDomains(t *testing.T) {
 
 func TestBuildWebserviceResponseFromRequestWithMalformedMultipleDomains(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=changeme&domain=foo,&addr=1.2.3.4", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -157,7 +149,6 @@ func TestBuildWebserviceResponseFromRequestWithMalformedMultipleDomains(t *testi
 
 func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenNoAddressIsGiven(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("POST", "/update?secret=changeme&domain=foo", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -169,7 +160,6 @@ func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenNoAddressIsG
 
 func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenInvalidAddressIsGiven(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/update?secret=changeme&domain=foo&addr=1.41:2", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, defaultExtractor)
@@ -181,7 +171,6 @@ func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenInvalidAddre
 
 func TestBuildWebserviceResponseFromRequestToReturnValidObjectWithDynExtractor(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/nic/update?hostname=foo&myip=1.2.3.4", nil)
 	req.Header.Add("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte("username:changeme")))
@@ -207,7 +196,6 @@ func TestBuildWebserviceResponseFromRequestToReturnValidObjectWithDynExtractor(t
 
 func TestBuildWebserviceResponseFromRequestToReturnInvalidObjectWhenNoSecretIsGivenWithDynExtractor(t *testing.T) {
 	var appConfig = &Config{}
-	appConfig.SharedSecret = "changeme"
 
 	req, _ := http.NewRequest("GET", "/nic/update", nil)
 	result := BuildWebserviceResponseFromRequest(req, appConfig, dynExtractor)

rest-api/utils.go

@@ -1,41 +1,66 @@
 #!/bin/bash
+NAMED_HOST=${NAMED_HOST:-'localhost'}
+ZONES=$(echo $ZONE | tr ',' '\n')
+RECORD_TTL=${RECORD_TTL:-300}
 
-[ -z "$SHARED_SECRET" ] && echo "SHARED_SECRET not set" && exit 1;
-[ -z "$ZONE" ] && echo "ZONE not set" && exit 1;
-[ -z "$RECORD_TTL" ] && echo "RECORD_TTL not set" && exit 1;
+# Backward compatibility for a single zone
+if [ $(echo "$ZONES" | wc -l) -eq 1 ]; then
+    # Allow update without fqdn
+    ZONE=$(echo "$ZONES" | head -1)
+else
+    # Allow multiple zones and disable updates without fqdn
+    ZONE=""
+fi
+
+# replaces a config value
+function bind-conf-set {
+    local KEY=${1:?'No key set'}
+    local SECRED=${2:-$(openssl rand 32 | base64)}
+    sed -E 's@('"${KEY}"')(\W+)"(.*)"@\1 "'${SECRED}'"@g' /dev/stdin
+}
 
+function bind-zone-add {
+local ZONE=${1:?'No zone set'}
 if ! grep 'zone "'$ZONE'"' /etc/bind/named.conf > /dev/null
 then
-	echo "creating zone...";
+	echo "creating zone for $ZONE...";
 	cat >> /etc/bind/named.conf <<EOF
+include "/etc/bind/ddns/$ZONE.key";
 zone "$ZONE" {
 	type master;
 	file "$ZONE.zone";
 	allow-query { any; };
 	allow-transfer { none; };
-	allow-update { localhost; };
+	update-policy { grant ddns-key.$ZONE zonesub ANY; };
 };
 EOF
 fi
 
 if [ ! -f /var/cache/bind/$ZONE.zone ]
 then
-	echo "creating zone file..."
+	echo "creating zone file for $ZONE..."
 	cat > /var/cache/bind/$ZONE.zone <<EOF
-\$ORIGIN .
+\$ORIGIN ${ZONE}.
 \$TTL 86400	; 1 day
-$ZONE		IN SOA	localhost. root.localhost. (
+@		IN SOA	ns postmaster (
 				74         ; serial
 				3600       ; refresh (1 hour)
 				900        ; retry (15 minutes)
 				604800     ; expire (1 week)
 				86400      ; minimum (1 day)
 				)
-			NS	localhost.
-\$ORIGIN ${ZONE}.
-\$TTL ${RECORD_TTL}
+			NS	ns
+ns			A	127.0.0.1
 EOF
 fi
+}
+
+install -d -o bind -g bind /etc/bind/ddns
+
+for Z in $ZONES; do
+    ddns-confgen -q -z $Z | bind-conf-set secret "${SHARED_SECRET}" | tee /etc/bind/ddns/$Z.key
+    bind-zone-add $Z
+done
 
 # If /var/cache/bind is a volume, permissions are probably not ok
 chown root:bind /var/cache/bind
@@ -48,10 +73,8 @@ then
 	echo "creating REST api config..."
 	cat > /etc/dyndns.json <<EOF
 {
-    "SharedSecret": "${SHARED_SECRET}",
-    "Server": "localhost",
+    "Server": "${NAMED_HOST}",
     "Zone": "${ZONE}.",
-    "Domain": "${ZONE}",
     "NsupdateBinary": "/usr/bin/nsupdate",
 	"RecordTTL": ${RECORD_TTL}
 }