gocsaf / csaf

Tools to download or provide CSAF (Common Security Advisory Framework) documents.
https://csaf.io
40 stars 23 forks source link

Fix missing error messages from loading provider-metadata.json #531

Closed mgoetzegb closed 4 months ago

mgoetzegb commented 7 months ago

What

Fix: don't drop error messages from loading provider-metadata.json

Additionally removed the duplicate check of provider metadata candidates retrieved from security.txt.

Why

Previously in case of trying last resort dns, all other error messages were dropped.

tschmidtb51 commented 7 months ago

@mgoetzegb Thank you for your contribution. Unfortunately, I'm currently failing to understand the issue. Could you please open an issue describing the steps to reproduce as well as current results and what the changed code would generate as outcome? Please also link the issue to your PR. Many thanks in advance!

mgoetzegb commented 6 months ago

Sorry for the delay, did not find time last week. I now added an issue with steps to reproduce and the change with this PR.

mgoetzegb commented 4 months ago

I prefer a distinction between error and debug messages. For example, TLS errors should be logged as errors, not debug messages. The user should be able to see the relevant errors without setting the log level to debug.

An example output should look like:

{"time":"2024-04-26T15:26:29Z","level":"DEBUG","msg":"http","who":"downloader","method":"GET","url":"https://sick.com/.well-known/csaf/provider-metadata.json"}
{"time":"2024-04-26T15:26:30Z","level":"DEBUG","msg":"http","who":"downloader","method":"GET","url":"https://sick.com/.well-known/security.txt"}
{"time":"2024-04-26T15:26:30Z","level":"DEBUG","msg":"http","who":"downloader","method":"GET","url":"https://sick.com/security.txt"}
{"time":"2024-04-26T15:26:30Z","level":"DEBUG","msg":"http","who":"downloader","method":"GET","url":"https://csaf.data.security.sick.com"}
{"time":"2024-04-26T15:31:00Z","level":"ERROR","msg":"Loading provider-metadata.json","domain":"sick.com","message":"fetching \"https://sick.com/.well-known/csaf/provider-metadata.json\" failed: Get \"https://sick.com/.well-known/csaf/provider-metadata.json\": tls: failed to verify certificate: x509: certificate signed by unknown authority"}
{"time":"2024-04-26T15:31:00Z","level":"ERROR","msg":"Loading provider-metadata.json","domain":"sick.com","message":"Fetching \"https://sick.com/.well-known/security.txt\" failed: Get \"https://sick.com/.well-known/security.txt\": tls: failed to verify certificate: x509: certificate signed by unknown authority"}
{"time":"2024-04-26T15:31:00Z","level":"ERROR","msg":"Loading provider-metadata.json","domain":"sick.com","message":"Fetching \"https://sick.com/security.txt\" failed: Get \"https://sick.com/security.txt\": tls: failed to verify certificate: x509: certificate signed by unknown authority"}
{"time":"2024-04-26T15:31:00Z","level":"ERROR","msg":"Loading provider-metadata.json","domain":"sick.com","message":"fetching \"https://csaf.data.security.sick.com\" failed: Get \"https://csaf.data.security.sick.com\": dial tcp 203.0.113.1:443: connect: connection timed out"}

If a valid provider-metadata.json was found, the fetching errors should be displayed as debug messages.

This requires an adjustment in the calling code, i.e. in the different components.

I rebased the branch and added this in commit f004c47e3c46c74aea94b8e8099d793b28c54981.

Adjustments for csaf_aggregator and csaf_downloader:

If the provider metadata json is invalid, then error messages from loading the provider metadata json are printed on log level error. Otherwise they are printed on level debug if the verbose option is set.

I'm not sure why those separate verbose options exist in the first place, given a leveled logger is used, but that is another topic I think.

The ProviderMetadataLoader is also used in the csaf_cecker, but there the error messages are already unconditionally printed on log level warn, not sure if that is desired. But given I'm not familiar with this component at all, I left it untouched for now.