kics icon indicating copy to clipboard operation
kics copied to clipboard

Add an offline flag to disable network requests

Open connorg opened this issue 2 years ago • 3 comments

Is your feature request related to a problem? Please describe. kics uses network calls in various places (CIS descriptions, version check, etc.) which makes sense as a default behavior. However, at GitLab we try to keep scanners self-contained for at least a few reasons:

  • We prefer that scanner output is deterministic based on the software version + inputs provided.
  • Customers sometimes require that external network accesses be disabled completely.
  • We want to make users aware of any potential leakage of information about their environment.
  • Some network-based features don't match up with how our users use our kics-based analyzer; for example, admonitions to upgrade mention version numbers and other details that don't match up to our product versioning scheme, which can be confusing.

Describe the solution you'd like

  • A new optional flag, perhaps named --offline, disables built-in external network requests.
  • New external network access requests check for offline mode before they're made.
    • User-provided paths, such as those where go-getter would be used today, would still be allowed. (This is how I've implemented offline mode for security-conscious users on non-Internet-connected networks before.)

Describe alternatives you've considered So far, we have found various ways to disable network calls, but:

  • new network use sites appear in new versions.
  • the process to disable each use site is often different from other use sites.
  • some use sites don't seem possible to disable at all via documented configurations, like the version check.

Additional context We at GitLab would be happy to collaborate on a solution, should the contribution be welcome.

If you'd like to discuss anything about this in a channel other than issues, please see my team page entry or send me an email at cgilbert at my company domain. Thanks!

connorg avatar Jun 14 '22 01:06 connorg

Thanks @connorg for the feedback.

Notifying our users on new versions is a service we want to keep and it's quite common for software with time based releases like KICS (minor release every ~2 weeks).

If the number of the new version is what confuses GitLab users, we can work around that to just mention a newer version is available. BTW, How do you present the current KICS version?

For other readers - KICS works well even without network access, the relevant functions would just timeout and the scan would be done the same way.

kaplanlior avatar Jun 15 '22 14:06 kaplanlior

Hey @kaplanlior, thanks for the response!

Notifying our users on new versions is a service we want to keep and it's quite common for software with time based releases like KICS (minor release every ~2 weeks).

Understood. We're/I'm not proposing to remove it or other network-dependent functions, only to make it possible to disable them for users who don't want them or can't use them (for security or privacy reasons, among others).

If the number of the new version is what confuses GitLab users, we can work around that to just mention a newer version is available. BTW, How do you present the current KICS version?

I don't necessarily want to overrotate on the version message issue, but we print the GitLab analyzer version in the beginning of our default logging output, for example, [INFO] [kics] [2022-02-07T17:05:13Z] ▶ GitLab kics analyzer v1.1.0. Analyzer versions are documented in CHANGELOGs for each analyzer. We apply this same pattern across all of our analyzers, including the one based on kics.

It also wouldn't necessarily be true that there would be a new GitLab analyzer version available if the message were changed, since we release updates only after going through validation. (Analyzer updates aren't always available immediately after the kics upstream release, though we aim to minimize the lag.)

For other readers - KICS works well even without network access, the relevant functions would just timeout and the scan would be done the same way.

We have a large number of users who operate disconnected from the public Internet; I understand that the timeout isn't that high but penalizing every run for these users by 20 seconds does feel a little unnecessary. (EDIT: I suppose an NXDOMAIN might come back faster than that.) https://github.com/Checkmarx/kics/blob/985aa6803372d44482527151471c3ae116e36b7e/pkg/descriptions/client.go#L47

We also have been surprised in the past by changes like the CIS version descriptions being downloaded and replacing existing text, as another example, and we would've been concerned about privacy impacts of telemetry/crash reporting (for similar reasons as https://github.com/Checkmarx/kics/issues/3041) were the disable-telemetry option not available at the time we began looking at kics. It was surprising that we didn't have a single place where we could say, "we'd like to operate self-contained, please" but rather had to disable each of these in different ways.

Hope that explains the use case and rationale a little bit more.

connorg avatar Jun 16 '22 00:06 connorg

Hi @kaplanlior or other team members! Just wanted to check in on this. We'd be happy to contribute to this if such a contribution would be accepted.

connorg avatar Jul 26 '22 01:07 connorg

Hi @connorg,

Hope this message finds you well! In here you can find the command list available to be used when running KICS. In there you can find a command called "--disable-full-descriptions" which can help you regarding the mentioned topic.

Kindly tell me if this command is what you looking for! If not, feel free to re-open the issue or even create a Pull Request with the changes that you think it fits better for your context! :D

gabriel-cx avatar Sep 21 '23 15:09 gabriel-cx