miracle091/dnscrypt-proxy
miracle091
/
dnscrypt-proxy
mirror of https://github.com/DNSCrypt/dnscrypt-proxy.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Verify signature immediately after reading from cache or URL 

This allows a large number of tests to be enabled and pass now that the behaviour is expected.
The main fix here is that a download with an invalid signature will always fall back on using a properly signed cache, no matter how old it is.
Additionally, downloads will never be written to the cache unless they are properly signed (both at startup and prefetching).
This commit is contained in:
William Elwood 2019-11-01 01:23:32 +00:00 committed by Frank Denis
parent 53d5b0f3cd
commit c0e34d1a9e
2 changed files with 44 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
dnscrypt-proxy/sources.go
View File

@ -51,18 +51,23 @@ func (source *Source) checkSignature(bin, sig []byte) (err error) {
// timeNow can be replaced by tests to provide a static value // timeNow can be replaced by tests to provide a static value
var timeNow = time.Now var timeNow = time.Now
func (source *Source) fetchFromCache() (bin, sig []byte, delayTillNextUpdate time.Duration, err error) { func (source *Source) fetchFromCache() (delayTillNextUpdate time.Duration, err error) {
delayTillNextUpdate = 0 delayTillNextUpdate = 0
var fi os.FileInfo var bin, sig []byte
if fi, err = os.Stat(source.cacheFile); err != nil {
return
}
if bin, err = ioutil.ReadFile(source.cacheFile); err != nil { if bin, err = ioutil.ReadFile(source.cacheFile); err != nil {
return return
} }
if sig, err = ioutil.ReadFile(source.cacheFile + ".minisig"); err != nil { if sig, err = ioutil.ReadFile(source.cacheFile + ".minisig"); err != nil {
return return
} }
if err = source.checkSignature(bin, sig); err != nil {
return
}
source.in = bin
var fi os.FileInfo
if fi, err = os.Stat(source.cacheFile); err != nil {
return
}
if elapsed := timeNow().Sub(fi.ModTime()); elapsed < source.cacheTTL { if elapsed := timeNow().Sub(fi.ModTime()); elapsed < source.cacheTTL {
dlog.Debugf("Cache file [%s] is still fresh", source.cacheFile) dlog.Debugf("Cache file [%s] is still fresh", source.cacheFile)
delayTillNextUpdate = source.prefetchDelay - elapsed delayTillNextUpdate = source.prefetchDelay - elapsed
@ -100,8 +105,8 @@ func fetchFromURL(xTransport *XTransport, u *url.URL) (bin []byte, err error) {
return return
} }
func (source *Source) fetchWithCache(xTransport *XTransport, urlStr string) (bin, sig []byte, delayTillNextUpdate time.Duration, err error) { func (source *Source) fetchWithCache(xTransport *XTransport, urlStr string) (delayTillNextUpdate time.Duration, err error) {
if bin, sig, delayTillNextUpdate, err = source.fetchFromCache(); err != nil { if delayTillNextUpdate, err = source.fetchFromCache(); err != nil {
if len(urlStr) == 0 { if len(urlStr) == 0 {
dlog.Errorf("Cache file [%s] not present and no URL given to retrieve it", source.cacheFile) dlog.Errorf("Cache file [%s] not present and no URL given to retrieve it", source.cacheFile)
return return
@ -122,12 +127,17 @@ func (source *Source) fetchWithCache(xTransport *XTransport, urlStr string) (bin
sigURL := &url.URL{} sigURL := &url.URL{}
*sigURL = *srcURL // deep copy to avoid parsing twice *sigURL = *srcURL // deep copy to avoid parsing twice
sigURL.Path += ".minisig" sigURL.Path += ".minisig"
var bin, sig []byte
if bin, err = fetchFromURL(xTransport, srcURL); err != nil { if bin, err = fetchFromURL(xTransport, srcURL); err != nil {
return return
} }
if sig, err = fetchFromURL(xTransport, sigURL); err != nil { if sig, err = fetchFromURL(xTransport, sigURL); err != nil {
return return
} }
if err = source.checkSignature(bin, sig); err != nil {
return
}
source.in = bin
source.writeToCache(bin, sig) // ignore error: not fatal source.writeToCache(bin, sig) // ignore error: not fatal
delayTillNextUpdate = source.prefetchDelay delayTillNextUpdate = source.prefetchDelay
return return
@ -150,13 +160,12 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac
} }
now := timeNow() now := timeNow()
var bin, sig []byte
var delayTillNextUpdate time.Duration var delayTillNextUpdate time.Duration
if len(urls) <= 0 { if len(urls) <= 0 {
bin, sig, delayTillNextUpdate, err = source.fetchWithCache(xTransport, "") delayTillNextUpdate, err = source.fetchWithCache(xTransport, "")
} else { } else {
for _, url := range urls { for _, url := range urls {
bin, sig, delayTillNextUpdate, err = source.fetchWithCache(xTransport, url) delayTillNextUpdate, err = source.fetchWithCache(xTransport, url)
if err == nil { if err == nil {
break break
} }
@ -168,11 +177,7 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac
return return
} }
if err = source.checkSignature(bin, sig); err != nil {
return
}
dlog.Noticef("Source [%s] loaded", cacheFile) dlog.Noticef("Source [%s] loaded", cacheFile)
source.in = bin
return return
} }
@ -188,7 +193,7 @@ func PrefetchSources(xTransport *XTransport, sources []*Source) time.Duration {
continue continue
} }
dlog.Debugf("Prefetching [%s]", u) dlog.Debugf("Prefetching [%s]", u)
_, _, delay, err := source.fetchWithCache(xTransport, u) delay, err := source.fetchWithCache(xTransport, u)
if err != nil { if err != nil {
dlog.Debugf("Prefetching [%s] failed: %v", u, err) dlog.Debugf("Prefetching [%s] failed: %v", u, err)
continue continue

48
dnscrypt-proxy/sources_test.go
View File

@ -197,32 +197,32 @@ func setupSourceTest(t *testing.T) (func(), *SourceTestData) {
d.cacheTests = map[string]SourceTestState{ // determines cache files written to disk before each call d.cacheTests = map[string]SourceTestState{ // determines cache files written to disk before each call
"correct": TestStateCorrect, "correct": TestStateCorrect,
//"expired": TestStateExpired, // TODO: an expired cache should be used if no download passes signature verification //"expired": TestStateExpired, // TODO: an expired cache should be used if no download passes signature verification
//"partial": TestStatePartial, // TODO: failed signature verification should not abort before trying additional URLs "partial": TestStatePartial,
//"partial-sig": TestStatePartialSig, // TODO: failed signature verification should not abort before trying additional URLs "partial-sig": TestStatePartialSig,
"missing": TestStateMissing, "missing": TestStateMissing,
//"missing-sig": TestStateMissingSig, // TODO: a cache without a signature should cause an attempt to download both files (not just the signature) "missing-sig": TestStateMissingSig,
"open-err": TestStateOpenErr, "open-err": TestStateOpenErr,
//"open-sig-err": TestStateOpenSigErr, // TODO: a cache without a signature should cause an attempt to download both files (not just the signature) "open-sig-err": TestStateOpenSigErr,
} }
d.downloadTests = map[string][]SourceTestState{ // determines the list of URLs passed in each call and how they will respond d.downloadTests = map[string][]SourceTestState{ // determines the list of URLs passed in each call and how they will respond
"correct": {TestStateCorrect}, "correct": {TestStateCorrect},
//"partial": {TestStatePartial}, // TODO: failed signature verification should not count as successfully prefetched "partial": {TestStatePartial},
//"partial-sig": {TestStatePartialSig}, // TODO: failed signature verification should not count as successfully prefetched "partial-sig": {TestStatePartialSig},
"missing": {TestStateMissing}, "missing": {TestStateMissing},
//"missing-sig": {TestStateMissingSig}, // TODO: failed signature verification should not count as successfully prefetched "missing-sig": {TestStateMissingSig},
"read-err": {TestStateReadErr}, "read-err": {TestStateReadErr},
//"read-sig-err": {TestStateReadSigErr}, // TODO: failed signature verification should not count as successfully prefetched "read-sig-err": {TestStateReadSigErr},
"open-err": {TestStateOpenErr}, "open-err": {TestStateOpenErr},
//"open-sig-err": {TestStateOpenSigErr}, // TODO: failed signature verification should not count as successfully prefetched "open-sig-err": {TestStateOpenSigErr},
//"path-err": {TestStatePathErr}, // TODO: invalid URLs should not be included in the prefetch list //"path-err": {TestStatePathErr}, // TODO: invalid URLs should not be included in the prefetch list
//"partial,correct": {TestStatePartial, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list, failed signature verification should not abort before trying additional URLs "partial,correct": {TestStatePartial, TestStateCorrect},
//"partial-sig,correct": {TestStatePartialSig, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list, failed signature verification should not abort before trying additional URLs "partial-sig,correct": {TestStatePartialSig, TestStateCorrect},
//"missing,correct": {TestStateMissing, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list "missing,correct": {TestStateMissing, TestStateCorrect},
//"missing-sig,correct": {TestStateMissingSig, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list, failed signature verification should not abort before trying additional URLs "missing-sig,correct": {TestStateMissingSig, TestStateCorrect},
//"read-err,correct": {TestStateReadErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list "read-err,correct": {TestStateReadErr, TestStateCorrect},
//"read-sig-err,correct": {TestStateReadSigErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list, failed signature verification should not abort before trying additional URLs "read-sig-err,correct": {TestStateReadSigErr, TestStateCorrect},
//"open-err,correct": {TestStateOpenErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list "open-err,correct": {TestStateOpenErr, TestStateCorrect},
//"open-sig-err,correct": {TestStateOpenSigErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list, failed signature verification should not abort before trying additional URLs "open-sig-err,correct": {TestStateOpenSigErr, TestStateCorrect},
//"path-err,correct": {TestStatePathErr, TestStateCorrect}, // TODO: invalid URLs should not be included in the prefetch list //"path-err,correct": {TestStatePathErr, TestStateCorrect}, // TODO: invalid URLs should not be included in the prefetch list
"no-urls": {}, "no-urls": {},
} }
@ -252,7 +252,7 @@ func prepSourceTestCache(t *testing.T, d *SourceTestData, e *SourceTestExpect, s
case TestStatePartial, TestStatePartialSig: case TestStatePartial, TestStatePartialSig:
e.err = "signature" e.err = "signature"
case TestStateMissing, TestStateMissingSig: case TestStateMissing, TestStateMissingSig:
e.err = "stat" e.err = "open"
case TestStateOpenErr, TestStateOpenSigErr: case TestStateOpenErr, TestStateOpenSigErr:
e.err = os.ErrPermission.Error() e.err = os.ErrPermission.Error()
} }