v3 Policy Spec Review

[GeekMasher]

Here is the current plan for the Policy as Code spec v3. This is a working Discussion and will be updated directly

Please add comments to anything that might interest you in adding support for.

https://github.com/GeekMasherOrg/security-v3

Top Level

version: "3"

name: "Policy Name"
description: "This is the Policy"

threatmodels:
  # ... Threat Models Block

codescanning:
  # ... Code Scanning Block

supplychain:
  # ... Supply Chain Block

secretscanning:
  # ... Secret Scanning Block

Threat Models Block

# This block allows you to have many different threat models
# which are applied on different condictions.
threatmodels:
  # default will be the default to apply to any thing
  # if not present, if will use the other `codescanning`, 
  # `supplychain`, `secretscanning` blocks.
  default:
    uses: ./policies/default.yml

  # this key can be anything
  geekmasher-repos:
    # uses which policy file for these repos
    uses: ./policies/geekmasher.yml
    # list of repos the uses block will be applies too
    repositories:
      - geekmasher/ghastoolkit

  advanced-security:
    uses: ./policies/high.yml
    repositories:
      - advanced-security/policy-as-code
      - advanced-security/codeql-queries

The uses key in the thread models block will load in the correct policy files stored in the repository. An example:

security: (repo)
  |- policy.yml (with threadmodel)
  |- polcies/
    |- high.yml (standard policy files, no threat model block 
    |- geekmasher.yml

Code Scanning Block

Code Scanning blocks can be a single instance (directly below) or a list.

codescanning:
  # Code Scanning is enabled
  enabled: true

  # anything above what is set
  severity: error

  # filter: identifier conditions
  ids:
    - js/sql-injection    # or condition
    - java/sql-injection
  ids-warning:
    - js/xss     # or condition
  ids-ignore:
    - js/log-injection     # or condition

  names:
    - "The Title of the Issue"   # or condition
  
  # filter: on CWEs
  cwes:
    - CWE-79  # or condition
    - CWE-89

  # filter: on OWASP (web apps or mobile)
  owasp-top10:
    - A03:2021  # or condition
    - M01:2016

  # filter: only apply the poc to these tools
  tools:
    - CodeQL    # or condition
    - Semgrep

  # mandate: make sure that the analysis / tools are present
  tools-required:
    - CodeQL    # and condition

  # filter: condition of time of alerts creation
  remediate:
    # ... Remediation Block

List of blocks:

codescanning:
  - name: codeql
    enabled: true
    tools:
      - CodeQL
    ids:
      - rule #...
    # ...
  - name: semgrep
    enabled: true
    tools:
      - Semgrep
    ids:
      - rule #...

Supply Chain Block

supply-chain:
  # Both Dependency Graph and Dependabot are enabled
  enabled: true

  # anything in dependabot that is this or higher
  severity: error

  # filter: Identifiers / PURL in Dependency Graph
  ids:
    - npm:jest     # or condition
  ids-warnings:
    - npm:express     # or condition
  ids-ignore:
    - npm:react     # or condition

  advisories:
    - GHSA-446m-mv8f-q348
  advisories-ignore:
    - GHSA-446m-mv8f-q347

  # filter: licenses present in Dependency Graph
  licenses:
    - GPL-*
 
  # filter: on unknown licenses, similar to `license` but with predefined list
  licenses-unknown: true

  # warn: on licenses present but warn only
  licenses-warning:
    - NOASSERTION

  # filter: apply an ignore on the license used
  licenses-ignore:
    - MIT

  # filter: condition of time of alerts creation
  remediate:
    # ... Remediation Block

Secret Scanning Block

Work in progress

Remediation Block

Nothing changes in this block

remediate:
  errors: 7      # after 7 days, error
  warnings: 30   # after 30 days, error
  all: 90        # after 90 days, error

[GeekMasher]

Code Scanning - Tools

Should this allow for semantic versions? A user could say codeql >= 2.12.0

[Killklli]

What should really get modified into here is allowing for multiple code scanning types under a single block based off of the tools

codescanning:
  - default:
      # Code Scanning is enabled
      enabled: true

      # anything above what is set
      severity: error

  - CodeQL/Veracode:
      # Code Scanning is enabled
      enabled: true

      # If the tool results are mandatory to fail a set, by default any other tools are not mandatory
      mandatory: true

      # anything above what is set
      severity: error
  - Sonarqube:
      # Code Scanning is enabled
      enabled: true

      # anything above what is set
      severity: error

While I'm not perfectly happy with what I've put as an example above with what your new tool breakdown is
Being able to exclude findings in a particular tool type would be relevant in many cases, along with being able to adjust things like remediation time and exclusions that the IDs match between tools as different results per tool.

From there we should also make a repository based exclusion list, with all of the new required workflow endpoints someone could be using this a lot more widely and could need to manage these exclusions a bit more granular.
There are a lot of cases where a vulnerability is allowed from a security standpoint with appropriate risk management on one repo but not another.

While I don't want to pollute the actual policy file with per repo permissions, it might be correct to put an option for an external repository list
That file would contain the following

octocat-org/repo1:
  exclusions:
    id:
      - npm:jest
octocat-org/repo2:
  exclusions:
    advisories:
      - HSA-446m-mv8f-q348

So while we could in the main policy file globally exclude a vuln, we could exclude a vuln per repo for particular situations that should not have been dismissed.

Further we should consider an option for checking against dismissed findings within a short amount of time.
Something like,

dismissed-findings: 30
dimissed-findings-timeframe: 1 day

So if someone dismissed 30 findings within say the last 1 day, it would fail as if there is a mass of dismissals mostly just trying to bypass security issues that would be a failure.

On that same vein, we should also make it so we can disregard dismissals. Some tools are the master of truth on the findings for that tool controlled by another team. While we want everything to bubble up in GHAS we should consider making it so some code scanning tools bypassed items are just still counted against the findings without it being closed out by the tool itself.

Finally we should add an "Maximum" within the remediation block.
So say we have 1 high finding today, we have 7 days to remediate.
But come tomorrow when this could be retriggered. Code has been pushed to master introducing 30 more highs. Thats not within the acceptable risk range for moving forward. So we should set a "maximum" introduced within that timeframe.
I don't expect this to need to be turned on with all levels, but only the higher level findings.

remediate:
  errors:
    timeframe: 7 # after 7 days, error
    maximum: 5 # if we're still within 7 days and have 5 or more errors, error
  warnings: 
    timeframe: 30   # after 30 days, error
  all: 
    timeframe: 90        # after 90 days, error

[goffinf]

I would like to have a policy expiry date or period or both added at the policy level. The requirement is as follows.

Our implementation allows the name of centrally managed policies to be associated with a repository using GitHub custom properties. For example we might have 3 policies defined that are called 'default' (this applies to all repos that have not been assigned a different value in a custom property), 'strict' where we want to apply tighter remediation SLAs for specific repos and 'permissive', which is the opposite to strict.

Policy-as-code runs as a REQUIRED workflow so, a PR merge to the default branch will FAIL if policy violations exist. We have allowed, as part of the organisation level ruleset, for the merge to be allowed by a member of the Security Compliance team.

However, it may be the case that a repository is consistently in violation of policy and there are grounds for the repo owner to seek a 'dispensation' for remediation SLA violations for an agreed period (say 3 months). What we want to do here is configure a repo specific (permissive) policy so violations do not occur BUT that policy will EXPIRE at the same time as the dispensation does. So after that date that repo policy is NOT VALID and instead the DEFAULT policy is applied. This removes the need for some other out-of-band process to recognise that the dispensation has expired (which may cover other aspects of the SLDC) and thus so should the remediation SLA policy for unresolved vulnerabilities.

Does that make sense ?

Should be fairly easy to implement.

[goffinf]

FYI I have implemented a basic version of policy expiry within v2.6.0. I couldn't just add an expiry field at the same level as other top level fields since that would cause schema validation to fail, so for now I add it under the general.remediation key. Before I call the action I parse the custom policy looking for the expiry date and (if present) check the date against current date. If it's expired I load the default policy instead and output a warning to the console. If a custom policy doesn't have an expiry field, a warning is output to the console stating that is required, and again the default policy is loaded. As I said in my previous comment in this thread, a repo owner can request a custom policy and, if security agree, the name of that policy is added to the valid list of policy names held in a custom property and the repo has the policy name added as a custom property. So the basic logic is that, on PR merge, the required workflow runs where I use the GH API to check whether the target has a specific custom property assigned to it with a value that corresponds to the name of their approved policy, I then check that the name is present in the list of valid policy names and if so, check the expiry date and if its within date configure p-a-c with that policy and if not load the default. It seems to work pretty well and meets our current requirement, although it would be nice for it to be built into p-a-c itself.