Complete requirement 4 (ROLIE) (#391)

* 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 <Jan Höfelmeyer jhoefelmeyer@intevation.de>
Co-authored-by: JanHoefelmeyer <hoefelmeyer.jan@gmail.com>
Co-authored-by: Sascha L. Teichmann <sascha.teichmann@intevation.de>

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
+		p.labelChecker.check(p, doc, u)
-		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)
-			}
-		}
 
 		// 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

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 {

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
+// identifier consist of document/tracking/id and document/publisher/namespace,
-// are in there right TLP color.
+// which in sum are unique for each csaf document and the name of a csaf document
-type rolieLabelChecker struct {
+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
+// extractTLP extracts the tlp label of the given document
-// to a TLPLabel. If pointer is nil unlabeled is returned.
+// and defaults to UNLABELED if not found.
-func tlpLabel(label *csaf.TLPLabel) csaf.TLPLabel {
+func (p *processor) extractTLP(doc any) csaf.TLPLabel {
-	if label != nil {
+	labelString, err := p.expr.Eval(`$.document.distribution.tlp.label`, doc)
-		return *label
+	if err != nil {
+		return csaf.TLPLabelUnlabeled
 	}
-	return csaf.TLPLabelUnlabeled
+	label, ok := labelString.(string)
-}
+	if !ok {
-
+		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
 	}
-	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)
+	lc.checkRank(p, label, url)
-
-	// Issue warnings or errors if the advisory is not protected properly.
-	rlc.checkProtection(p, label, url)
 }
 
-// checkProtection tests if a given advisory has the right level
+// checkPermissions checks for mistakes in access-protection.
-// of protection.
+func (lc *labelChecker) checkPermissions(
-func (rlc *rolieLabelChecker) checkProtection(
 	p *processor,
 	label csaf.TLPLabel,
+	doc any,
 	url string,
 ) {
-	switch {
+	switch label {
-	// If we are checking WHITE and we have a test client
+	case csaf.TLPLabelAmber, csaf.TLPLabelRed:
-	// and we get a status forbidden then the access is not open.
+		// If the client has no authorization it shouldn't be able
-	case label == csaf.TLPLabelWhite:
+		// to access TLP:AMBER or TLP:RED advisories
-		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):
 		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,38 +123,96 @@ 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
 // integrity and completeness.
 func (p *processor) processROLIEFeeds(feeds [][]csaf.Feed) error {
@@ -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
+}

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.