Skip to content

Commit

Permalink
Change sharedSecred checking method
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Golit committed Aug 31, 2020
1 parent eb88b8b commit 00fba17
Show file tree
Hide file tree
Showing 8 changed files with 56 additions and 69 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 0 additions & 9 deletions rest-api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ import (
)

type Config struct {
SharedSecret string
Server string
Zone string
Domain string
NsupdateBinary string
RecordTTL int
Port int
Expand Down Expand Up @@ -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")
Expand All @@ -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()
}
}
10 changes: 5 additions & 5 deletions rest-api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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
}
19 changes: 11 additions & 8 deletions rest-api/nsupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
5 changes: 2 additions & 3 deletions rest-api/request_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
12 changes: 0 additions & 12 deletions rest-api/request_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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")))
Expand All @@ -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)
Expand Down
17 changes: 0 additions & 17 deletions rest-api/utils.go

This file was deleted.

51 changes: 37 additions & 14 deletions setup.sh
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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}
}
Expand Down

0 comments on commit 00fba17

Please sign in to comment.