From de27a668d1327a776f87ef6cd3d74abb3cfc5aed Mon Sep 17 00:00:00 2001 From: JanHoefelmeyer <107021473+JanHoefelmeyer@users.noreply.github.com> Date: Thu, 13 Jul 2023 22:22:11 +0200 Subject: [PATCH] Complete requirement 4 (ROLIE) (#391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Create dummy structure to uniquely identify each advisory * Remove dummy values, remove unused variable for now * Formatting * Add Evaluation of whether a white Advisory is access protected and add it to the respective slice, implement functionality * Initialize p.whiteAdvisories before using it, stop sorting if no Client was used * Ammend rules to include requirement 4, warning instead of error if white advisory is found protected, use badWhitePermissions.use() * Formatting * Fix typo: avaible -> available * Improve check on whether building identifier failed * Move extracting of tlp labels and related functions from processor to roliecheck * Create Labelchecker and check access of white advisories regardless of whether ROLIE feeds exist. Only check Ranks if ROLIE feeds are used * Formatting * Do not use label checker as a pointer. * Rename label checker * Add XXX to questionable code. * Simplify checking white advisories. * Improve error message if no checks for accessibility of white advisories were done * Extract TLP label directly without extractTLP function, consistent plural in error message * Add comments and check type assertion in tlp label extraction. * Move check for white advisories to label checker. * Improve methods naming an comments. * Address a few review questions. * Move functionality of checkProtection fully into evaluateTLP * Add comments and warn only if we are in a white feed or in a dirlisting. --------- Co-authored-by: JanHoefelmeyer Co-authored-by: JanHoefelmeyer Co-authored-by: Sascha L. Teichmann --- cmd/csaf_checker/processor.go | 79 ++++++------ cmd/csaf_checker/reporters.go | 2 +- cmd/csaf_checker/roliecheck.go | 215 ++++++++++++++++++++++----------- cmd/csaf_checker/rules.go | 5 +- 4 files changed, 183 insertions(+), 118 deletions(-) diff --git a/cmd/csaf_checker/processor.go b/cmd/csaf_checker/processor.go index e03261e3..510ad7c7 100644 --- a/cmd/csaf_checker/processor.go +++ b/cmd/csaf_checker/processor.go @@ -53,7 +53,7 @@ type processor struct { pmd256 []byte pmd any keys *crypto.KeyRing - labelChecker *rolieLabelChecker + labelChecker labelChecker invalidAdvisories topicMessages badFilenames topicMessages @@ -190,6 +190,10 @@ func newProcessor(opts *options) (*processor, error) { expr: util.NewPathEval(), ageAccept: ageAccept(opts), validator: validator, + labelChecker: labelChecker{ + advisories: map[csaf.TLPLabel]util.Set[string]{}, + whiteAdvisories: map[identifier]bool{}, + }, }, nil } @@ -241,7 +245,7 @@ func (p *processor) clean() { p.badROLIECategory.reset() p.badWhitePermissions.reset() p.badAmberRedPermissions.reset() - p.labelChecker = nil + p.labelChecker.reset() } // run calls checkDomain function for each domain in the given "domains" parameter. @@ -361,6 +365,7 @@ func (p *processor) domainChecks(domain string) []func(*processor, string) error (*processor).checkMissing, (*processor).checkInvalid, (*processor).checkListing, + (*processor).checkWhitePermissions, ) return checks @@ -735,28 +740,7 @@ func (p *processor) integrity( } } - // Extract the tlp level of the entry - if tlpa, err := p.expr.Eval( - `$.document`, doc); err != nil { - p.badROLIEFeed.error( - "Extracting 'tlp level' from %s failed: %v", u, err) - } else { - tlpe := extractTLP(tlpa) - // If the client has no authorization it shouldn't be able - // to access TLP:AMBER or TLP:RED advisories - if !p.opts.protectedAccess() && - (tlpe == csaf.TLPLabelAmber || tlpe == csaf.TLPLabelRed) { - - p.badAmberRedPermissions.use() - p.badAmberRedPermissions.error( - "Advisory %s of TLP level %v is not access protected.", - u, tlpe) - } - // check if current feed has correct or all of their tlp levels entries. - if p.labelChecker != nil { - p.labelChecker.check(p, tlpe, u) - } - } + p.labelChecker.check(p, doc, u) // Check if file is in the right folder. p.badFolders.use() @@ -870,25 +854,6 @@ func (p *processor) integrity( return nil } -// extractTLP tries to extract a valid TLP label from an advisory -// Returns "UNLABELED" if it does not exist, the label otherwise -func extractTLP(tlpa any) csaf.TLPLabel { - if document, ok := tlpa.(map[string]any); ok { - if distri, ok := document["distribution"]; ok { - if distribution, ok := distri.(map[string]any); ok { - if tlp, ok := distribution["tlp"]; ok { - if label, ok := tlp.(map[string]any); ok { - if labelstring, ok := label["label"].(string); ok { - return csaf.TLPLabel(labelstring) - } - } - } - } - } - } - return csaf.TLPLabelUnlabeled -} - // checkIndex fetches the "index.txt" and calls "checkTLS" method for HTTPS checks. // It extracts the file names from the file and passes them to "integrity" function. // It returns error if fetching/reading the file(s) fails, otherwise nil. @@ -946,7 +911,7 @@ func (p *processor) checkIndex(base string, mask whereType) error { } // Block rolie checks. - p.labelChecker = nil + p.labelChecker.feedLabel = "" return p.integrity(files, base, mask, p.badIndices.add) } @@ -1041,7 +1006,7 @@ func (p *processor) checkChanges(base string, mask whereType) error { } // Block rolie checks. - p.labelChecker = nil + p.labelChecker.feedLabel = "" return p.integrity(files, base, mask, p.badChanges.add) } @@ -1215,6 +1180,30 @@ func (p *processor) checkListing(string) error { return nil } +// checkWhitePermissions checks if the TLP:WHITE advisories are +// available with unprotected access. +func (p *processor) checkWhitePermissions(string) error { + + var ids []string + for id, open := range p.labelChecker.whiteAdvisories { + if !open { + ids = append(ids, id.String()) + } + } + + if len(ids) == 0 { + return nil + } + + sort.Strings(ids) + + p.badWhitePermissions.error( + "TLP:WHITE advisories with ids %s are only available access-protected.", + strings.Join(ids, ", ")) + + return nil +} + // checkProviderMetadata checks provider-metadata.json. If it exists, // decodes, and validates against the JSON schema. // According to the result, the respective error messages added to diff --git a/cmd/csaf_checker/reporters.go b/cmd/csaf_checker/reporters.go index d28db3bf..9a8357b4 100644 --- a/cmd/csaf_checker/reporters.go +++ b/cmd/csaf_checker/reporters.go @@ -157,7 +157,7 @@ func (r *tlsReporter) report(p *processor, domain *Domain) { func (r *tlpWhiteReporter) report(p *processor, domain *Domain) { req := r.requirement(domain) if !p.badWhitePermissions.used() { - req.message(InfoType, "No advisories labeled TLP:WHITE tested for accessibility.") + req.message(InfoType, "No access-protected advisories labeled TLP:WHITE found.") return } if len(p.badWhitePermissions) == 0 { diff --git a/cmd/csaf_checker/roliecheck.go b/cmd/csaf_checker/roliecheck.go index fd52323d..6f6a961a 100644 --- a/cmd/csaf_checker/roliecheck.go +++ b/cmd/csaf_checker/roliecheck.go @@ -9,6 +9,7 @@ package main import ( + "errors" "net/http" "net/url" "sort" @@ -18,13 +19,33 @@ import ( "github.com/csaf-poc/csaf_distribution/v2/util" ) -// rolieLabelChecker helps to check id advisories in ROLIE feeds -// are in there right TLP color. -type rolieLabelChecker struct { +// identifier consist of document/tracking/id and document/publisher/namespace, +// which in sum are unique for each csaf document and the name of a csaf document +type identifier struct { + id string + namespace string +} + +// String implements fmt.Stringer +func (id identifier) String() string { + return "(" + id.namespace + ", " + id.id + ")" +} + +// labelChecker helps to check if advisories are of the right TLP color. +type labelChecker struct { feedURL string feedLabel csaf.TLPLabel - advisories map[csaf.TLPLabel]util.Set[string] + advisories map[csaf.TLPLabel]util.Set[string] + whiteAdvisories map[identifier]bool +} + +// reset brings the checker back to an initial state. +func (lc *labelChecker) reset() { + lc.feedLabel = "" + lc.feedURL = "" + lc.advisories = map[csaf.TLPLabel]util.Set[string]{} + lc.whiteAdvisories = map[identifier]bool{} } // tlpLevel returns an inclusion order of TLP colors. @@ -43,81 +64,55 @@ func tlpLevel(label csaf.TLPLabel) int { } } -// tlpLabel returns the value of a none-nil pointer -// to a TLPLabel. If pointer is nil unlabeled is returned. -func tlpLabel(label *csaf.TLPLabel) csaf.TLPLabel { - if label != nil { - return *label +// extractTLP extracts the tlp label of the given document +// and defaults to UNLABELED if not found. +func (p *processor) extractTLP(doc any) csaf.TLPLabel { + labelString, err := p.expr.Eval(`$.document.distribution.tlp.label`, doc) + if err != nil { + return csaf.TLPLabelUnlabeled } - return csaf.TLPLabelUnlabeled -} - -// add registers a given url to a label. -func (rlc *rolieLabelChecker) add(label csaf.TLPLabel, url string) { - advs := rlc.advisories[label] - if advs == nil { - advs = util.Set[string]{} - rlc.advisories[label] = advs + label, ok := labelString.(string) + if !ok { + return csaf.TLPLabelUnlabeled } - advs.Add(url) + return csaf.TLPLabel(label) } // check tests if the TLP label of an advisory is used correctly. -func (rlc *rolieLabelChecker) check( +func (lc *labelChecker) check( p *processor, - label csaf.TLPLabel, + doc any, url string, ) { + label := p.extractTLP(doc) + + // Check the permissions. + lc.checkPermissions(p, label, doc, url) + // Associate advisory label to urls. - rlc.add(label, url) + lc.add(label, url) // If entry shows up in feed of higher tlp level, give out info or warning. - rlc.checkRank(p, label, url) - - // Issue warnings or errors if the advisory is not protected properly. - rlc.checkProtection(p, label, url) + lc.checkRank(p, label, url) } -// checkProtection tests if a given advisory has the right level -// of protection. -func (rlc *rolieLabelChecker) checkProtection( +// checkPermissions checks for mistakes in access-protection. +func (lc *labelChecker) checkPermissions( p *processor, label csaf.TLPLabel, + doc any, url string, ) { - switch { - // If we are checking WHITE and we have a test client - // and we get a status forbidden then the access is not open. - case label == csaf.TLPLabelWhite: - p.badWhitePermissions.use() - // We only need to download it with an unauthorized client - // if have not done it yet. - if p.usedAuthorizedClient() { - res, err := p.unauthorizedClient().Get(url) - if err != nil { - p.badWhitePermissions.error( - "Unexpected Error %v when trying to fetch: %s", err, url) - } else if res.StatusCode == http.StatusForbidden { - p.badWhitePermissions.error( - "Advisory %s of TLP level WHITE is access protected.", url) - } - } - - // If we are checking AMBER or above we need to download - // the data again with the open client. - // If this does not result in status forbidden the - // server may be wrongly configured. - case tlpLevel(label) >= tlpLevel(csaf.TLPLabelAmber): + switch label { + case csaf.TLPLabelAmber, csaf.TLPLabelRed: + // If the client has no authorization it shouldn't be able + // to access TLP:AMBER or TLP:RED advisories p.badAmberRedPermissions.use() - // It is an error if we downloaded the advisory with - // an unauthorized client. if !p.usedAuthorizedClient() { p.badAmberRedPermissions.error( - "Advisory %s of TLP level %v is not properly access protected.", + "Advisory %s of TLP level %v is not access protected.", url, label) } else { - // We came here by an authorized download which is okay. - // So its bad if we can download it with an unauthorized client, too. res, err := p.unauthorizedClient().Get(url) if err != nil { p.badAmberRedPermissions.error( @@ -128,36 +123,94 @@ func (rlc *rolieLabelChecker) checkProtection( url, label) } } + + case csaf.TLPLabelWhite: + // If we found a white labeled document we need to track it + // to find out later if there was an unprotected way to access it. + + p.badWhitePermissions.use() + // Being not able to extract the identifier from the document + // indicates that the document is not valid. Should not happen + // as the schema validation passed before. + p.invalidAdvisories.use() + if id, err := p.extractAdvisoryIdentifier(doc); err != nil { + p.invalidAdvisories.error("Bad document %s: %v", url, err) + } else if !lc.whiteAdvisories[id] { + // Only do check if we haven't seen it as accessible before. + + if !p.usedAuthorizedClient() { + // We already downloaded it without protection + lc.whiteAdvisories[id] = true + } else { + // Need to try to re-download it unauthorized. + if resp, err := p.unauthorizedClient().Get(url); err == nil { + accessible := resp.StatusCode == http.StatusOK + lc.whiteAdvisories[id] = accessible + // If we are in a white rolie feed or in a dirlisting + // directly warn if we cannot access it. + // The cases of being in an amber or red feed are resolved. + if !accessible && + (lc.feedLabel == "" || lc.feedLabel == csaf.TLPLabelWhite) { + p.badWhitePermissions.warn( + "Advisory %s of TLP level WHITE is access-protected.", url) + } + } + } + } } } +// add registers a given url to a label. +func (lc *labelChecker) add(label csaf.TLPLabel, url string) { + advs := lc.advisories[label] + if advs == nil { + advs = util.Set[string]{} + lc.advisories[label] = advs + } + advs.Add(url) +} + // checkRank tests if a given advisory is contained by the // the right feed color. -func (rlc *rolieLabelChecker) checkRank( +func (lc *labelChecker) checkRank( p *processor, label csaf.TLPLabel, url string, ) { - switch advisoryRank, feedRank := tlpLevel(label), tlpLevel(rlc.feedLabel); { + // Only do this check when we are inside a ROLIE feed. + if lc.feedLabel == "" { + return + } + + switch advisoryRank, feedRank := tlpLevel(label), tlpLevel(lc.feedLabel); { case advisoryRank < feedRank: if advisoryRank == 0 { // All kinds of 'UNLABELED' p.badROLIEFeed.info( "Found unlabeled advisory %q in feed %q.", - url, rlc.feedURL) + url, lc.feedURL) } else { p.badROLIEFeed.warn( "Found advisory %q labled TLP:%s in feed %q (TLP:%s).", url, label, - rlc.feedURL, rlc.feedLabel) + lc.feedURL, lc.feedLabel) } case advisoryRank > feedRank: // Must not happen, give error p.badROLIEFeed.error( "%s of TLP level %s must not be listed in feed %s of TLP level %s", - url, label, rlc.feedURL, rlc.feedLabel) + url, label, lc.feedURL, lc.feedLabel) + } +} + +// defaults returns the value of the referencend pointer p +// if it is not nil, def otherwise. +func defaults[T any](p *T, def T) T { + if p != nil { + return *p } + return def } // processROLIEFeeds goes through all ROLIE feeds and checks their @@ -199,10 +252,6 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { } } - p.labelChecker = &rolieLabelChecker{ - advisories: map[csaf.TLPLabel]util.Set[string]{}, - } - // Phase 2: check for integrity. for _, fs := range feeds { for i := range fs { @@ -228,7 +277,7 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { continue } - label := tlpLabel(feed.TLPLabel) + label := defaults(feed.TLPLabel, csaf.TLPLabelUnlabeled) if err := p.categoryCheck(feedBase, label); err != nil { if err != errContinue { return err @@ -241,8 +290,6 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { // TODO: Issue a warning if we want check AMBER+ without an // authorizing client. - // TODO: Complete criteria for requirement 4. - if err := p.integrity(files, feedBase, rolieMask, p.badProviderMetadata.add); err != nil { if err != errContinue { return err @@ -280,7 +327,7 @@ func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error { feedBase := base.ResolveReference(up) makeAbs := makeAbsolute(feedBase) - label := tlpLabel(feed.TLPLabel) + label := defaults(feed.TLPLabel, csaf.TLPLabelUnlabeled) switch label { case csaf.TLPLabelUnlabeled: @@ -441,3 +488,31 @@ func (p *processor) serviceCheck(feeds [][]csaf.Feed) error { // TODO: Check conformity with RFC8322 return nil } + +// extractAdvisoryIdentifier extracts document/publisher/namespace and +// document/tracking/id from advisory and stores it in an identifier. +func (p *processor) extractAdvisoryIdentifier(doc any) (identifier, error) { + namespace, err := p.expr.Eval(`$.document.publisher.namespace`, doc) + if err != nil { + return identifier{}, err + } + + idString, err := p.expr.Eval(`$.document.tracking.id`, doc) + if err != nil { + return identifier{}, err + } + + ns, ok := namespace.(string) + if !ok { + return identifier{}, errors.New("cannot extract 'namespace'") + } + id, ok := idString.(string) + if !ok { + return identifier{}, errors.New("cannot extract 'id'") + } + + return identifier{ + namespace: ns, + id: id, + }, nil +} diff --git a/cmd/csaf_checker/rules.go b/cmd/csaf_checker/rules.go index a1bc3fbf..02d4c991 100644 --- a/cmd/csaf_checker/rules.go +++ b/cmd/csaf_checker/rules.go @@ -31,7 +31,7 @@ type requirementRules struct { var ( publisherRules = &requirementRules{ cond: condAll, - subs: ruleAtoms(1, 2, 3 /* 4 */), + subs: ruleAtoms(1, 2, 3, 4), } providerRules = &requirementRules{ @@ -163,7 +163,8 @@ func (p *processor) eval(requirement int) bool { return !p.badFilenames.hasErrors() case 3: return len(p.noneTLS) == 0 - + case 4: + return !p.badWhitePermissions.hasErrors() case 5: return !p.badAmberRedPermissions.hasErrors() // Currently, only domains using HTTP-Header redirects are checked.