Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added two new criteria #35

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions baseline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,55 @@ criteria:
in a separate repository and fetched during
a specific well-documented pipeline step.
control_mappings: # TODO
security_insights_value: # TODO
scorecard_probe: # TODO
- id: OSPS-53

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with this one as worded fairly strongly.

I would reword it "cryptographically attested" -- We've seen multiple issues with just signing a piece of software.

maturity_level: 2
category: Build & Release
criteria: |
All released software assets MUST be signed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not nearly rigorous enough. Anyone can sign anything. The goal here is to cryptographically tie some element of the software (e.g. it's build) to the approved maintainers of that software. As written it's way too weak.

with a cryptographic signature.
objective: |
Provide users with a mechanism to verify the
authenticity and integrity of released
software assets, reducing the risk of
tampering or unauthorized modifications.
implementation: |
Sign all released software assets at build
time with a cryptographic signature, such
as a GPG or PGP signature.
SecurityCRob marked this conversation as resolved.
Show resolved Hide resolved

SecurityCRob marked this conversation as resolved.
Show resolved Hide resolved
It is recommended that this signature is
generated as part of the build and release
pipeline to ensure that it is consistent and
automated.
control_mappings: # TODO
security_insights_value: # TODO
scorecard_probe:
- releasesAreSigned
- id: OSPS-54
maturity_level: 3
category: Quality
criteria: |
All changes to the project's codebase MUST
require a passing status check from a

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too vague. Who decides what the passing status check is?

This is why I like SSDF's PW7 and PW8

Review and/or Analyze Human-Readable
Code to Identify Vulnerabilities and Verify
Compliance with Security Requirements
(PW.7): Help identify vulnerabilities so that
they can be corrected before the software is
released to prevent exploitation. Using
automated methods lowers the effort and
resources needed to detect vulnerabilities.
Human-readable code includes source code,
scripts, and any other form of code that an
organization deems human-readable.

Test Executable Code to Identify
Vulnerabilities and Verify Compliance with
Security Requirements (PW.8): Help identify
vulnerabilities so that they can be corrected
before the software is released in order to
prevent exploitation. Using automated
methods lowers the effort and resources
needed to detect vulnerabilities and improves
traceability and repeatability. Executable code
includes binaries, directly executed bytecode
and source code, and any other form of code
that an organization deems executable.

SSDF instead of explicitly calling out SAST states it a bit more broadly and also highlights that it's up to the organization or system's security needs to determine what to do when an issue arises.

Other docs like NIST 800-53 include similar but a bit more specific wording, e.g.

) DEVELOPER TESTING AND EVALUATION | STATIC CODE ANALYSIS
Require the developer of the system, system component, or system service to employ
static code analysis tools to identify common flaws and document the results of the
analysis.
Discussion: Static code analysis provides a technology and methodology for security reviews
and includes checking for weaknesses in the code as well as for the incorporation of libraries
or other included code with known vulnerabilities or that are out-of-date and not supported.
Static code analysis can be used to identify vulnerabilities and enforce secure coding
practices. It is most effective when used early in the development process, when each code
change can automatically be scanned for potential weaknesses. Static code analysis can
provide clear remediation guidance and identify defects for developers to fix. Evidence of
the correct implementation of static analysis can include aggregate defect density for critical
defect types, evidence that defects were inspected by developers or security professionals,
and evidence that defects were remediated. A high density of ignored findings, commonly
referred to as false positives, indicates a potential problem with the analysis process or the
analysis tool. In such cases, organizations weigh the validity of the evidence against evidence
from other sources.

I would push for us to use this sort of wording. As written it's:

  • Vague: What counts as a SAST tool? There's basic linting, there's cyclomatic complexity checking, and all sorts of other stuff.
  • Too strict for every change to a codebase: There are lots of changes to a code base that don't require static analysis, e.g. documentation changes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For PW 7.2 SSDF does call out the use of static analysis tools and is somewhat specific about how to handle the results "Example 4: Use a static analysis tool to automatically check code for vulnerabilities and compliance with the organization’s secure coding standards with a human reviewing the issues reported by the tool and remediating them as necessary."

I'd agree that we should probably call out specific examples of what these tools are.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vague: What counts as a SAST tool? There's basic linting, there's cyclomatic complexity checking, and all sorts of other stuff. @mlieberman85

Agreed that this needs to be defined. We have a lexicon definition of SCA, but currently lacking one for SAST.

Too strict for every change to a codebase: There are lots of changes to a code base that don't require static analysis, e.g. documentation changes @mlieberman85

Our lexicon segments the codebase from the documentation (We can also improve this if needed).

  - term: Codebase
    definition: |
      The collection of source code and related
      assets that make up the project. The codebase
      includes all files necessary to build and
      test the software. Lives in the repository,
      sometimes alongside documentation and CI/CD
      pipelines. The contents of the codebase are
      the primary deliverable in a release.

Static Application Security Testing (SAST)
tool.
objective: |
Identify and address defects and security weaknesses
in the project's codebase early in the
development process, reducing the risk of
shipping insecure software.
implementation: |
Create a status check in the project's
version control system that runs a Static
Application Security Testing (SAST) tool on
all changes to the codebase. Require that the
status check passes before changes can be
merged.
control_mappings: # TODO
security_insights_value: # TODO
scorecard_probe: # sastToolRunsOnAllCommits
- id: OSPS-70
maturity_level: 3
category: Access Control
Expand Down