Refactor signature verification to own function

The cache is no longer destroyed whenever any signature verification fails.
The public key is stored on the Source struct for future use.
This commit is contained in:
William Elwood 2019-10-30 02:00:49 +00:00 committed by Frank Denis
parent da0d7fe841
commit d851c9eeb6
2 changed files with 34 additions and 33 deletions

View File

@ -33,6 +33,15 @@ type Source struct {
urls []string urls []string
format SourceFormat format SourceFormat
in string in string
minisignKey *minisign.PublicKey
}
func (source *Source) checkSignature(bin, sig string) (err error) {
var signature minisign.Signature
if signature, err = minisign.DecodeSignature(sig); err == nil {
_, err = source.minisignKey.Verify([]byte(bin), signature)
}
return
} }
// timeNow can be replaced by tests to provide a static value // timeNow can be replaced by tests to provide a static value
@ -130,18 +139,19 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac
} else { } else {
return source, []URLToPrefetch{}, fmt.Errorf("Unsupported source format: [%s]", formatStr) return source, []URLToPrefetch{}, fmt.Errorf("Unsupported source format: [%s]", formatStr)
} }
minisignKey, err := minisign.NewPublicKey(minisignKeyStr) urlsToPrefetch := []URLToPrefetch{}
if err != nil { if minisignKey, err := minisign.NewPublicKey(minisignKeyStr); err == nil {
return source, []URLToPrefetch{}, err source.minisignKey = &minisignKey
} else {
return source, urlsToPrefetch, err
} }
now := timeNow() now := timeNow()
urlsToPrefetch := []URLToPrefetch{}
sigCacheFile := cacheFile + ".minisig" sigCacheFile := cacheFile + ".minisig"
var sigStr, in string var sigStr, in string
var cached, sigCached bool var cached, sigCached bool
var delayTillNextUpdate, sigDelayTillNextUpdate time.Duration var delayTillNextUpdate, sigDelayTillNextUpdate time.Duration
var sigErr error var err, sigErr error
var preloadURL string var preloadURL string
if len(urls) <= 0 { if len(urls) <= 0 {
in, cached, delayTillNextUpdate, err = fetchWithCache(xTransport, "", cacheFile, refreshDelay) in, cached, delayTillNextUpdate, err = fetchWithCache(xTransport, "", cacheFile, refreshDelay)
@ -172,16 +182,7 @@ func NewSource(xTransport *XTransport, urls []string, minisignKeyStr string, cac
return source, urlsToPrefetch, err return source, urlsToPrefetch, err
} }
signature, err := minisign.DecodeSignature(sigStr) if err = source.checkSignature(in, sigStr); err != nil {
if err != nil {
os.Remove(cacheFile)
os.Remove(sigCacheFile)
return source, urlsToPrefetch, err
}
res, err := minisignKey.Verify([]byte(in), signature)
if err != nil || !res {
os.Remove(cacheFile)
os.Remove(sigCacheFile)
return source, urlsToPrefetch, err return source, urlsToPrefetch, err
} }
if !cached { if !cached {

View File

@ -199,32 +199,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 and should not destroy the cache //"partial": TestStatePartial, // TODO: failed signature verification should not abort before trying additional URLs
//"partial-sig": TestStatePartialSig, // TODO: failed signature verification should not abort before trying additional URLs and should not destroy the cache //"partial-sig": TestStatePartialSig, // TODO: failed signature verification should not abort before trying additional URLs
"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), failed signature verification should not destroy the cache //"missing-sig": TestStateMissingSig, // TODO: a cache without a signature should cause an attempt to download both files (not just the signature)
"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), failed signature verification should not destroy the cache //"open-sig-err": TestStateOpenSigErr, // TODO: a cache without a signature should cause an attempt to download both files (not just the signature)
} }
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 and should not destroy the cache //"partial": {TestStatePartial}, // TODO: failed signature verification should not count as successfully prefetched
//"partial-sig": {TestStatePartialSig}, // TODO: failed signature verification should not count as successfully prefetched and should not destroy the cache //"partial-sig": {TestStatePartialSig}, // TODO: failed signature verification should not count as successfully prefetched
//"missing": {TestStateMissing}, // TODO: a list download failure should not cause an attempt to download the signature for that URL //"missing": {TestStateMissing}, // TODO: a list download failure should not cause an attempt to download the signature for that URL
//"missing-sig": {TestStateMissingSig}, // TODO: failed signature verification should not count as successfully prefetched and should not destroy the cache //"missing-sig": {TestStateMissingSig}, // TODO: failed signature verification should not count as successfully prefetched
//"read-err": {TestStateReadErr}, // TODO: a list download failure should not cause an attempt to download the signature for that URL //"read-err": {TestStateReadErr}, // TODO: a list download failure should not cause an attempt to download the signature for that URL
//"read-sig-err": {TestStateReadSigErr}, // TODO: failed signature verification should not count as successfully prefetched and should not destroy the cache //"read-sig-err": {TestStateReadSigErr}, // TODO: failed signature verification should not count as successfully prefetched
"open-err": {TestStateOpenErr}, "open-err": {TestStateOpenErr},
//"open-sig-err": {TestStateOpenSigErr}, // TODO: failed signature verification should not count as successfully prefetched and should not destroy the cache //"open-sig-err": {TestStateOpenSigErr}, // TODO: failed signature verification should not count as successfully prefetched
//"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 and should not destroy the cache //"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-sig,correct": {TestStatePartialSig, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list, failed signature verification should not abort before trying additional URLs and should not destroy the cache //"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
//"missing,correct": {TestStateMissing, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list //"missing,correct": {TestStateMissing, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list
//"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 and should not destroy the cache //"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
//"read-err,correct": {TestStateReadErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list //"read-err,correct": {TestStateReadErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list
//"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 and should not destroy the cache //"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
//"open-err,correct": {TestStateOpenErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list //"open-err,correct": {TestStateOpenErr, TestStateCorrect}, // TODO: all URLs should be included in the prefetch list
//"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 and should not destroy the cache //"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
//"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": {},
} }
@ -320,7 +320,7 @@ func setupSourceTestCase(t *testing.T, d *SourceTestData, i int,
i = (i + 1) % len(d.sources) // make the cached and downloaded fixtures different i = (i + 1) % len(d.sources) // make the cached and downloaded fixtures different
} }
prepSourceTestDownload(t, d, e, d.sources[i], downloadTest) prepSourceTestDownload(t, d, e, d.sources[i], downloadTest)
e.Source = &Source{e.urls, SourceFormatV2, e.in} e.Source = &Source{e.urls, SourceFormatV2, e.in, d.key}
return return
} }