From 33c9bf6d4286ea3642f3294ddefe63bcf281d379 Mon Sep 17 00:00:00 2001 From: Robert Butts Date: Thu, 10 Jan 2019 16:43:45 -0700 Subject: [PATCH 1/3] Fix TO /dnsseckeys/ksk/generate when no ksk exists Fixes #3153 --- .../traffic_ops_golang/cdn/dnssecrefresh.go | 13 ++-- traffic_ops/traffic_ops_golang/cdn/genksk.go | 66 ++++++++++++++----- .../dbhelpers/db_helpers.go | 12 ++++ .../deliveryservice/dnssec.go | 3 +- 4 files changed, 71 insertions(+), 23 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go b/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go index f97fca798c..287f872485 100644 --- a/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go +++ b/traffic_ops/traffic_ops_golang/cdn/dnssecrefresh.go @@ -174,7 +174,8 @@ func doDNSSECKeyRefresh(tx *sql.Tx, cfg *config.Config) { log.Infoln("The ZSK keys for '" + string(cdnInf.CDNName) + "' are expired!") effectiveDate := expiration.Add(ttl * time.Duration(effectiveMultiplier) * -1) // -1 to subtract isKSK := false - newKeys, err := regenExpiredKeys(isKSK, keys[string(cdnInf.CDNName)], effectiveDate, false, false) + cdnDNSDomain := cdnInf.CDNDomain + "." + newKeys, err := regenExpiredKeys(isKSK, cdnDNSDomain, keys[string(cdnInf.CDNName)], effectiveDate, false, false) if err != nil { log.Errorln("refreshing DNSSEC Keys: regenerating expired ZSK keys: " + err.Error()) } else { @@ -222,7 +223,7 @@ func doDNSSECKeyRefresh(tx *sql.Tx, cfg *config.Config) { log.Infoln("The KSK keys for '" + ds.DSName + "' are expired!") effectiveDate := expiration.Add(ttl * time.Duration(effectiveMultiplier) * -1) // -1 to subtract isKSK := true - newKeys, err := regenExpiredKeys(isKSK, dsKeys, effectiveDate, false, false) + newKeys, err := regenExpiredKeys(isKSK, string(ds.DSName), dsKeys, effectiveDate, false, false) if err != nil { log.Errorln("refreshing DNSSEC Keys: regenerating expired KSK keys for ds '" + string(ds.DSName) + "': " + err.Error()) } else { @@ -242,7 +243,7 @@ func doDNSSECKeyRefresh(tx *sql.Tx, cfg *config.Config) { log.Infoln("The ZSK keys for '" + ds.DSName + "' are expired!") effectiveDate := expiration.Add(ttl * time.Duration(effectiveMultiplier) * -1) // -1 to subtract isKSK := false - newKeys, err := regenExpiredKeys(isKSK, dsKeys, effectiveDate, false, false) + newKeys, err := regenExpiredKeys(isKSK, string(ds.DSName), dsKeys, effectiveDate, false, false) if err != nil { log.Errorln("refreshing DNSSEC Keys: regenerating expired ZSK keys for ds '" + string(ds.DSName) + "': " + err.Error()) } else { @@ -262,6 +263,7 @@ func doDNSSECKeyRefresh(tx *sql.Tx, cfg *config.Config) { type DNSSECKeyRefreshCDNInfo struct { CDNName tc.CDNName + CDNDomain string DNSSECEnabled bool TLDTTLsDNSKEY *uint64 DNSKEYEffectiveMultiplier *uint64 @@ -275,6 +277,7 @@ func getDNSSECKeyRefreshParams(tx *sql.Tx) (map[tc.CDNName]DNSSECKeyRefreshCDNIn WITH cdn_profile_ids AS ( SELECT DISTINCT(c.name) as cdn_name, + c.domain_name as cdn_domain, c.dnssec_enabled as cdn_dnssec_enabled, MAX(p.id) as profile_id -- We only want 1 profile, so get the probably-newest if there's more than one. FROM @@ -307,15 +310,17 @@ GROUP BY pi.cdn_name, pi.cdn_dnssec_enabled params := map[tc.CDNName]DNSSECKeyRefreshCDNInfo{} for rows.Next() { cdnName := tc.CDNName("") + cdnDomain := "" dnssecEnabled := false name := util.StrPtr("") valStr := util.StrPtr("") - if err := rows.Scan(&cdnName, &dnssecEnabled, &name, &valStr); err != nil { + if err := rows.Scan(&cdnName, &cdnDomain, &dnssecEnabled, &name, &valStr); err != nil { return nil, errors.New("scanning cdn dnssec key refresh parameters: " + err.Error()) } inf := params[cdnName] inf.CDNName = cdnName + inf.CDNDomain = cdnDomain inf.DNSSECEnabled = dnssecEnabled if name == nil || valStr == nil { diff --git a/traffic_ops/traffic_ops_golang/cdn/genksk.go b/traffic_ops/traffic_ops_golang/cdn/genksk.go index 360e6a1905..bfbacf4615 100644 --- a/traffic_ops/traffic_ops_golang/cdn/genksk.go +++ b/traffic_ops/traffic_ops_golang/cdn/genksk.go @@ -29,12 +29,15 @@ import ( "github.com/apache/trafficcontrol/lib/go-log" "github.com/apache/trafficcontrol/lib/go-tc" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api" + "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/deliveryservice" "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/riaksvc" ) const DefaultKSKTTLSeconds = 60 const DefaultKSKEffectiveMultiplier = 2 +const DefaultKSKExpiration = 365 * 24 * time.Hour +const DefaultZSKExpiration = 30 * 24 * time.Hour func GenerateKSK(w http.ResponseWriter, r *http.Request) { inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"name"}, nil) @@ -51,6 +54,16 @@ func GenerateKSK(w http.ResponseWriter, r *http.Request) { return } + cdnDomain, cdnExists, err := dbhelpers.GetCDNDomainFromName(inf.Tx.Tx, cdnName) + if err != nil { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting CDN domain: "+err.Error())) + return + } + if !cdnExists { + api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound, errors.New("cdn '"+string(cdnName)+"' not found"), nil) + return + } + ttl, multiplier, err := getKSKParams(inf.Tx.Tx, cdnName) if err != nil { api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting CDN KSK parameters: "+err.Error())) @@ -75,7 +88,8 @@ func GenerateKSK(w http.ResponseWriter, r *http.Request) { } isKSK := true - newKey, err := regenExpiredKeys(isKSK, dnssecKeys[string(cdnName)], *req.EffectiveDate, true, true) + cdnDNSDomain := cdnDomain + "." + newKey, err := regenExpiredKeys(isKSK, cdnDNSDomain, dnssecKeys[string(cdnName)], *req.EffectiveDate, true, true) if err != nil { api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("regenerating CDN DNSSEC keys: "+err.Error())) return @@ -142,9 +156,11 @@ WHERE return ttl, mult, nil } -// regenExpiredKeys regenerates expired keys. The key is the map key into the keys object, which may be a CDN name or a delivery service name. -func regenExpiredKeys(typeKSK bool, existingKeys tc.DNSSECKeySetV11, effectiveDate time.Time, tld bool, resetExp bool) (tc.DNSSECKeySetV11, error) { +const DefaultDNSSECKeyTTL = 60 * time.Second +// regenExpiredKeys regenerates expired keys. The key is the map key into the keys object, which may be a CDN name or a delivery service name. +// The name is the name of the key, either the CDN name or the Delivery Service name. If existingKeys contains any keys marked "new", the name argument is not used, but the name of the previously-new key is used instead. These should match, and a warning is logged if they differ. +func regenExpiredKeys(typeKSK bool, name string, existingKeys tc.DNSSECKeySetV11, effectiveDate time.Time, tld bool, resetExp bool) (tc.DNSSECKeySetV11, error) { existingKey := ([]tc.DNSSECKeyV11)(nil) if typeKSK { existingKey = existingKeys.KSK @@ -162,19 +178,28 @@ func regenExpiredKeys(typeKSK bool, existingKeys tc.DNSSECKeySetV11, effectiveDa } } - if !oldKeyFound { - return existingKeys, errors.New("no old key found") // TODO verify this is correct (Perl doesn't check) + newInception := time.Now() + defaultExpiration := DefaultZSKExpiration + if typeKSK { + defaultExpiration = DefaultKSKExpiration } + newExpiration := newInception.Add(defaultExpiration) - name := oldKey.Name - ttl := time.Duration(oldKey.TTLSeconds) * time.Second - expiration := oldKey.ExpirationDateUnix - inception := oldKey.InceptionDateUnix - const secPerDay = 86400 - expirationDays := (expiration - inception) / secPerDay + ttl := DefaultDNSSECKeyTTL + if oldKeyFound { + expiration := oldKey.ExpirationDateUnix + inception := oldKey.InceptionDateUnix + const secPerDay = 86400 + expirationDays := (expiration - inception) / secPerDay - newInception := time.Now() - newExpiration := time.Now().Add(time.Duration(expirationDays) * time.Hour * 24) + if name != oldKey.Name { + log.Warnln("regenExpiredKeys given name '" + name + "' which doesn't match existingKey name '" + oldKey.Name + "' - using existing key name in refresh! Ignoring expected passed name!") + } + + name = oldKey.Name + ttl = time.Duration(oldKey.TTLSeconds) * time.Second + newExpiration = time.Now().Add(time.Duration(expirationDays) * time.Hour * 24) + } keyType := tc.DNSSECKSKType if !typeKSK { @@ -185,16 +210,21 @@ func regenExpiredKeys(typeKSK bool, existingKeys tc.DNSSECKeySetV11, effectiveDa return tc.DNSSECKeySetV11{}, errors.New("getting and generating DNSSEC keys: " + err.Error()) } - oldKey.Status = tc.DNSSECKeyStatusExpired - if resetExp { - oldKey.ExpirationDateUnix = effectiveDate.Unix() + newKeys := []tc.DNSSECKeyV11{newKey} + if oldKeyFound { + oldKey.Status = tc.DNSSECKeyStatusExpired + if resetExp { + oldKey.ExpirationDateUnix = effectiveDate.Unix() + } + + newKeys = append(newKeys, oldKey) } regenKeys := tc.DNSSECKeySetV11{} if typeKSK { - regenKeys = tc.DNSSECKeySetV11{ZSK: existingKeys.ZSK, KSK: []tc.DNSSECKeyV11{newKey, oldKey}} + regenKeys = tc.DNSSECKeySetV11{ZSK: existingKeys.ZSK, KSK: newKeys} } else { - regenKeys = tc.DNSSECKeySetV11{ZSK: []tc.DNSSECKeyV11{newKey, oldKey}, KSK: existingKeys.KSK} + regenKeys = tc.DNSSECKeySetV11{ZSK: newKeys, KSK: existingKeys.KSK} } return regenKeys, nil } diff --git a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go index ae2f3254fc..b988927ada 100644 --- a/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go +++ b/traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go @@ -174,3 +174,15 @@ func GetCDNNameFromID(tx *sql.Tx, id int64) (tc.CDNName, bool, error) { } return tc.CDNName(name), true, nil } + +// GetCDNDomainFromName returns the domain, whether the cdn exists, and any error. +func GetCDNDomainFromName(tx *sql.Tx, cdnName tc.CDNName) (string, bool, error) { + domain := "" + if err := tx.QueryRow(`SELECT domain_name FROM cdn WHERE name = $1`, cdnName).Scan(&domain); err != nil { + if err == sql.ErrNoRows { + return "", false, nil + } + return "", false, errors.New("Error querying CDN name: " + err.Error()) + } + return domain, true, nil +} diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go b/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go index 20642e3d4c..51d21808d9 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go @@ -23,6 +23,7 @@ import ( "database/sql" "encoding/base64" "errors" + "fmt" "strconv" "strings" "time" @@ -151,7 +152,7 @@ func genKeys(dsName string, ksk bool, ttl time.Duration, tld bool) (string, stri if ksk && tld { dsRecord := dnskey.ToDS(digestType) if dsRecord == nil { - return "", "", nil, errors.New("creating DS record from DNSKEY record: " + err.Error()) + return "", "", nil, fmt.Errorf("creating DS record from DNSKEY record: converting dnskey %++v to DS failed", dnskey) } keyDS = &tc.DNSSECKeyDSRecordV11{Algorithm: int64(dsRecord.Algorithm), DigestType: int64(dsRecord.DigestType), Digest: dsRecord.Digest} } From 33f074526764f7bc51d1a89d684d2e54e8f2958f Mon Sep 17 00:00:00 2001 From: Robert Butts Date: Thu, 10 Jan 2019 17:17:57 -0700 Subject: [PATCH 2/3] Fix TO cdns/dnsseckeys to default if no ttl exists Fixes #3204 (cherry picked from commit a49446632e2a4daa611d1bf88be4c6c0d2c2253c) --- traffic_ops/traffic_ops_golang/cdn/dnssec.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/traffic_ops/traffic_ops_golang/cdn/dnssec.go b/traffic_ops/traffic_ops_golang/cdn/dnssec.go index 52f3ade604..606ac49720 100644 --- a/traffic_ops/traffic_ops_golang/cdn/dnssec.go +++ b/traffic_ops/traffic_ops_golang/cdn/dnssec.go @@ -62,6 +62,15 @@ func CreateDNSSECKeys(w http.ResponseWriter, r *http.Request) { api.WriteResp(w, r, "Successfully created dnssec keys for "+cdnName) } +// DefaultDSTTL is the default DS Record TTL to use, if no CDN Snapshot exists, or if no tld.ttls.DS parameter exists. +// This MUST be the same value as Traffic Router's default. Currently: +// traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/dns/SignatureManager.java:476 +// `final Long dsTtl = ZoneUtils.getLong(config.get("ttls"), "DS", 60);`. +// If Traffic Router and Traffic Ops differ, and a user is using the default, errors may occur! +// Users are advised to set the tld.ttls.DS CRConfig.json Parameter, so the default is not used! +// Traffic Ops functions SHOULD warn whenever this default is used. +const DefaultDSTTL = 60 * time.Second + func GetDNSSECKeys(w http.ResponseWriter, r *http.Request) { inf, userErr, sysErr, errCode := api.NewInfo(r, []string{"name"}, nil) if userErr != nil || sysErr != nil { @@ -85,8 +94,9 @@ func GetDNSSECKeys(w http.ResponseWriter, r *http.Request) { dsTTL, err := GetDSRecordTTL(inf.Tx.Tx, cdnName) if err != nil { - api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError, nil, errors.New("getting DS Record TTL: "+err.Error())) - return + log.Errorln("Getting DNSSEC Keys: getting DS Record TTL from CRConfig Snapshot: " + err.Error()) + log.Errorf("Getting DNSSEC Keys: getting DS Record TTL failed, using default %v. It is STRONGLY ADVISED to fix the error, and ensure a CRConfig Snapshot exists for the CDN, and a tld.ttls.DS CRConfig.json Parameter exists on a Router Profile on the CDN. Default DS Records may cause unexpected behavior or errors!\n", DefaultDSTTL) + dsTTL = DefaultDSTTL } keys, err := deliveryservice.MakeDNSSECKeysFromRiakKeys(riakKeys, dsTTL) From a46a2b13400e5640e20241a0df7bd2398d6e22dc Mon Sep 17 00:00:00 2001 From: Robert Butts Date: Fri, 11 Jan 2019 09:55:38 -0700 Subject: [PATCH 3/3] Revert "Change TO DNSSEC to SHA256" This reverts commit cc2541879ed7d38a54ef6cd160a9b4e9dacb39ca. --- traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go b/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go index 51d21808d9..42a9fab2c7 100644 --- a/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go +++ b/traffic_ops/traffic_ops_golang/deliveryservice/dnssec.go @@ -113,7 +113,7 @@ func GetDNSSECKeysV11(keyType string, dsName string, ttl time.Duration, inceptio func genKeys(dsName string, ksk bool, ttl time.Duration, tld bool) (string, string, *tc.DNSSECKeyDSRecordV11, error) { bits := 1024 flags := 256 - algorithm := dns.RSASHA256 // 8 - http://www.iana.org/assignments/dns-sec-alg-numbers/dns-sec-alg-numbers.xhtml + algorithm := dns.RSASHA1 // 5 - http://www.iana.org/assignments/dns-sec-alg-numbers/dns-sec-alg-numbers.xhtml protocol := 3 if ksk {