From 00fba1789d703697dcf8b76ad517e454a4fa5ee8 Mon Sep 17 00:00:00 2001 From: Golit Date: Mon, 31 Aug 2020 16:31:09 +0200 Subject: [PATCH] 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 #55 --- Dockerfile | 2 +- rest-api/config.go | 9 ------ rest-api/main.go | 10 +++---- rest-api/nsupdate.go | 19 +++++++----- rest-api/request_handler.go | 5 ++-- rest-api/request_handler_test.go | 12 -------- rest-api/utils.go | 17 ----------- setup.sh | 51 +++++++++++++++++++++++--------- 8 files changed, 56 insertions(+), 69 deletions(-) delete mode 100644 rest-api/utils.go diff --git a/Dockerfile b/Dockerfile index 0244847..a26347a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,7 +9,7 @@ FROM debian:buster-slim MAINTAINER David Prandzioch 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 diff --git a/rest-api/config.go b/rest-api/config.go index e7d7bf4..8cfc7a0 100644 --- a/rest-api/config.go +++ b/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() - } } diff --git a/rest-api/main.go b/rest-api/main.go index f0a7126..9cba633 100644 --- a/rest-api/main.go +++ b/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 } diff --git a/rest-api/nsupdate.go b/rest-api/nsupdate.go index fd4169c..c5f13e8 100644 --- a/rest-api/nsupdate.go +++ b/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,9 +109,9 @@ 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) @@ -116,6 +119,6 @@ func (nsupdate *NSUpdate) UpdateRecord(r RecordUpdateRequest) { 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") } diff --git a/rest-api/request_handler.go b/rest-api/request_handler.go index 0558308..8ee6b52 100644 --- a/rest-api/request_handler.go +++ b/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 diff --git a/rest-api/request_handler_test.go b/rest-api/request_handler_test.go index e248ae3..d01aca9 100644 --- a/rest-api/request_handler_test.go +++ b/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) diff --git a/rest-api/utils.go b/rest-api/utils.go deleted file mode 100644 index 62bd2df..0000000 --- a/rest-api/utils.go +++ /dev/null @@ -1,17 +0,0 @@ -package main - -import ( - "fmt" - "crypto/rand" - "encoding/base64" -) - -func randomString() string { - random := make([]byte, 32) - _, err := rand.Read(random) - if err != nil { - fmt.Println("error:", err) - return "8Passw0RT!" - } - return base64.StdEncoding.EncodeToString(random) -} diff --git a/setup.sh b/setup.sh index 2377c44..5ab1fa7 100755 --- a/setup.sh +++ b/setup.sh @@ -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 < /var/cache/bind/$ZONE.zone < /etc/dyndns.json <