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

SRE-474 - Add risk for each unsigned commit #11

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
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
Next Next commit
SRE-474 - Add risk for each unsigned commit
  • Loading branch information
Erich Smith committed Mar 1, 2023
commit 0e13b9a869f7b20050be4e92f8ecb0bccf4111d5
3 changes: 3 additions & 0 deletions defaultConfig.json
Original file line number Diff line number Diff line change
@@ -46,6 +46,9 @@
"verifyGpg": {
"missingValue": 0.5
},
"verifyAllGpg": {
"perUnverifiedValue": 1
},
"gitleaksFindings": {
"perFindingValue": 10
}
9 changes: 8 additions & 1 deletion src/gather.ts
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ import { Config, Risk, RiskCategory } from './types';

export async function localSCMRisk(): Promise<RiskCategory> {
const checks: Promise<Risk>[] = [];
const defaultRiskValue = 5;
const defaultRiskValue = 0;

const config = getConfig();

@@ -23,6 +23,12 @@ export async function localSCMRisk(): Promise<RiskCategory> {
}
if (config.facts.scm.gitPath && config.facts.scm.gpgPath) {
checks.push(scm.gpgVerifyRecentCommitsCheck(scmCheckValues.verifyGpg));
checks.push(
scm.gpgVerifyAllCommitsCheck(
config.flags.mergeRef,
scmCheckValues.verifyAllGpg
)
);
}

checks.push(
@@ -34,6 +40,7 @@ export async function localSCMRisk(): Promise<RiskCategory> {

// gather risks
const risks = await Promise.all(checks);
console.log({ risks, defaultRiskValue, section: 'localSCMRisk' });

return {
title: 'SCM Risk',
99 changes: 98 additions & 1 deletion src/scm.ts
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ import {
SCMValuesEnforceGPGCheck,
SCMValuesGitCheck,
SCMValuesGitleaksFindingsCheck,
SCMValuesVerifyAllGPGCheck,
SCMValuesVerifyGPGCheck,
} from './types';

@@ -26,7 +27,7 @@ export async function gitRepoDirCheck(
let description = 'Missing - no repo found!';

if (await fs.pathExists(path.join(dir, '.git'))) {
value = -5;
value = -1;
description = 'Repo found. 🎉';
} else {
recommendations.push('Version code in a Git repository.');
@@ -44,6 +45,9 @@ export async function gitRepoDirCheck(
);
}

// TODO: remove this, or replace with a GitHub-aware check
// This doesn't work in CI/CD environments, and is not a good proxy for
// whether the repo is actually enforcing GPG signing.
export async function gitConfigGPGCheck(
checkValues: SCMValuesEnforceGPGCheck,
cmdRunner: any = undefined
@@ -75,6 +79,10 @@ export async function gitConfigGPGCheck(
);
}

// This is a best-effort verification of the top two commits (accounts for merge
// commits), and is proxy indicator that the PR is likely using signed
// commits. Does not unduly penalize for individual unsigned commits, but does
// reward in the presence of at least one recently signed commit.
export async function gpgVerifyRecentCommitsCheck(
checkValues: SCMValuesVerifyGPGCheck,
cmdRunner: any = undefined
@@ -107,6 +115,68 @@ export async function gpgVerifyRecentCommitsCheck(
);
}

// This is a thorough verification of signed commits for all commits in the PR.
// It will add risk for each unsigned, signed-but-expired, or signed-but-revoked
// commit.
export async function gpgVerifyAllCommitsCheck(
mergeRef: string,
checkValues: SCMValuesVerifyAllGPGCheck,
cmdRunner: any = undefined
): Promise<Risk> {
console.error({ mergeRef, checkValues });
const check = 'verifyGpgSigningAllCommits';
const recommendations: string[] = [];
const perUnverifiedValue = checkValues.perUnverifiedValue;

const validityLog = await gpgVerifyAllCommits(
`HEAD...${mergeRef}`,
cmdRunner
);
console.error({ validityLog });
// produces lines like:
// "G 3b8d0 John Doe commit message" # good signature
// "N 771cd John Doe commit message" # no signature
// "X ac731 John Smith commit message" # good signature that has expired
// "Y dead0 Jane Smith commit message" # good signature, key expired
// "R a001c John Doe commit message" # good signature, key revoked
// "U b10cc Jane Doe commit message" # good signature, key unknown
// "E 21a1c John Doe commit message" # signature cannot be checked, missing key

let description = `All commits in ${mergeRef} are signed.`;
let value = -1;

const unsignedCommits = validityLog
.split('\n')
.map((l) => l.split(' '))
.filter((e) => ['N', 'X', 'Y', 'R'].includes(e[0]));

if (unsignedCommits.length > 0) {
description = `Found ${unsignedCommits.length} unsigned commits since ${mergeRef}.`;
// add risk for each unsigned commit
value = perUnverifiedValue * unsignedCommits.length;
for (const splitLog of unsignedCommits) {
recommendations.push(
`Commit '${splitLog.join(
' '
)}' is unsigned or has an invalid signature.`
);
}
recommendations.push(
`Sign your commits with a verified GPG key. See https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-emGem for more details on the concise git log '%G?' output.`
);
}

return formatRisk(
{
check,
value,
description,
recommendations,
},
riskCategory,
check
);
}
export async function gpgVerifyCommit(
gitref: string,
cmdRunner: any = undefined
@@ -115,6 +185,33 @@ export async function gpgVerifyCommit(
return Boolean(/NEWSIG/.exec(cmd.stderr)); // git puts this on stderr for some reason
}

export async function gpgVerifyAllCommits(
gitref: string,
cmdRunner: any = undefined
): Promise<string> {
// per https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-emGem,
// produces lines like:
// "G 3b8d0 John Doe commit message" # good signature
// "X ac731 John Smith commit message" # good signature that has expired
// "Y dead0 Jane Smith commit message" # good signature, key expired
// "R a001c John Doe commit message" # good signature, key revoked
// "U b10cc Jane Doe commit message" # good signature, key unknown
// "E 21a1c John Doe commit message" # signature cannot be checked, missing key
// "N 771cd John Doe commit message" # no signature
//
// NOTE: All values in the first column of log output, other than 'N'
// rely on local GPG keys, so it is the users' responsibility to ensure
// that their local GPG keys are up to date and trusted if they want to
// rely on these values, prior to executing Peril.

const cmd = await runCmd(
`git log --pretty="format:%G? %h %aN %s" ${gitref} 2>&1`,
cmdRunner,
{ shell: true }
);
return cmd.stdout;
}

export async function gatherFacts(
cmdRunner: any = undefined,
config: Config = getConfig()
5 changes: 5 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
@@ -84,6 +84,7 @@ export type SCMValues = {
git: SCMValuesGitCheck;
enforceGpg: SCMValuesEnforceGPGCheck;
verifyGpg: SCMValuesVerifyGPGCheck;
verifyAllGpg: SCMValuesVerifyAllGPGCheck;
gitleaksFindings: SCMValuesGitleaksFindingsCheck;
};
};
@@ -96,6 +97,10 @@ export type SCMValuesVerifyGPGCheck = {
missingValue: number;
};

export type SCMValuesVerifyAllGPGCheck = {
perUnverifiedValue: number;
};

export type SCMValuesGitleaksFindingsCheck = {
perFindingValue: number;
};