From c32094d0b1fc3aa5160087d71bff36ed1779bc3a Mon Sep 17 00:00:00 2001 From: HampusM Date: Sun, 15 Aug 2021 13:41:37 +0200 Subject: Rewrote the commit author fingerprint function & implemented commit signature verification --- packages/api/src/commit.d.ts | 2 + packages/server/src/git/commit.ts | 123 ++++++++++++-------------- packages/server/src/git/error/types.ts | 6 ++ packages/server/src/routes/api/v1/repo/log.ts | 5 +- packages/server/src/routes/api/v1/repo/map.ts | 6 +- test/unit/commit.unit.test.ts | 9 ++ 6 files changed, 85 insertions(+), 66 deletions(-) diff --git a/packages/api/src/commit.d.ts b/packages/api/src/commit.d.ts index 0a76f8a..c9974f3 100644 --- a/packages/api/src/commit.d.ts +++ b/packages/api/src/commit.d.ts @@ -25,6 +25,7 @@ export interface Commit { message: string, author: CommitAuthor, isSigned: boolean, + signatureVerified: boolean | null, date: number, insertions: number, deletions: number, @@ -36,6 +37,7 @@ export type LogCommit = { id: string, author: CommitAuthor, isSigned: boolean, + signatureVerified: boolean | null, message: string, date: number, insertions: number, diff --git a/packages/server/src/git/commit.ts b/packages/server/src/git/commit.ts index d5d8eab..6b3c38b 100644 --- a/packages/server/src/git/commit.ts +++ b/packages/server/src/git/commit.ts @@ -3,11 +3,10 @@ import { Author } from "api"; import { Diff } from "./diff"; import { Repository } from "./repository"; import { Tree } from "./tree"; -import { createMessage, readKey, readSignature, verify } from "openpgp"; import { promisify } from "util"; -import { exec } from "child_process"; -import { findAsync } from "./misc"; -import { ErrorWhere, createError, CommitNotSignedError, FailedError } from "./error"; +import { exec, ExecException } from "child_process"; +import { ErrorWhere, createError, CommitNotSignedError, FailedError, NotInKeyringError } from "./error"; +import { createMessage, readKey, readSignature, verify } from "openpgp"; const pExec = promisify(exec); @@ -27,80 +26,44 @@ type DiffStats = { * A author of a commit */ export class CommitAuthor implements Author { - private _ng_commit: NodeGitCommit; + private _commit; - constructor(ng_commit: NodeGitCommit) { - this._ng_commit = ng_commit; + constructor(commit: Commit) { + this._commit = commit; } public get name(): string { - return this._ng_commit.author().name(); + return this._commit.ng_commit.author().name(); } public get email(): string { - return this._ng_commit.author().email(); + return this._commit.ng_commit.author().email(); } /** * Returns the public key fingerprint of the commit's signer. */ public async fingerprint(): Promise { - const basic_signature = await this._ng_commit.getSignature().catch(() => { + if(!await this._commit.isSigned()) { throw(createError(ErrorWhere.Commit, CommitNotSignedError)); - }); - - const message = await createMessage({ text: basic_signature.signedData }); - - const pub_keys_list = await pExec("gpg --list-public-keys"); - - if(pub_keys_list.stderr) { - throw(createError(ErrorWhere.Commit, FailedError, "get public keys from gpg!")); } - const pub_keys = pub_keys_list.stdout - .split("\n") - .slice(2, -1) - .join("\n") - .split(/^\n/gm); - - // Find a public key that matches the signature - const pub_key = await findAsync(pub_keys, async key => { - // Make sure the UID is the same as the commit author - const uid = key - .split("\n")[2] - .replace(/^uid\s*\[.*\]\s/, ""); - - if(uid !== `${this.name} <${this.email}>`) { - return false; - } - - // Get the public key as an armored key - const fingerprint = key.split("\n")[1].replace(/^\s*/, ""); - const key_export = await pExec(`gpg --armor --export ${fingerprint}`); + const key = await pExec(`gpg --list-public-keys ${this.email}`).catch((err: ExecException) => { + return { + stdout: null, + stderr: err.message.split("\n")[1] + }; + }); - if(key_export.stderr) { - throw(createError(ErrorWhere.Commit, FailedError, "export a public key from gpg!")); + if(key.stderr || !key.stdout) { + if(/^gpg: error reading key: No public key\n?/.test(key.stderr)) { + throw(createError(ErrorWhere.Commit, NotInKeyringError, this.email)); } - const signature = await readSignature({ armoredSignature: basic_signature.signature }); - - const verification = await verify({ - message: message, - verificationKeys: await readKey({ armoredKey: key_export.stdout }), - expectSigned: true, - signature: signature - }) - .then(result => result.signatures[0].verified) - .catch(() => Promise.resolve(false)); - - return verification; - }); - - if(!pub_key) { - throw(createError(ErrorWhere.Commit, FailedError, "find a public key matching the commit signature!")); + throw(createError(ErrorWhere.Commit, FailedError, `receive pgp key for '${this.email}'`)); } - return pub_key + return key.stdout .split("\n")[1] .replace(/^\s*/, ""); } @@ -110,9 +73,10 @@ export class CommitAuthor implements Author { * A representation of a commit */ export class Commit { - private _ng_commit: NodeGitCommit; private _owner: Repository; + public ng_commit: NodeGitCommit; + public id: string; public date: number; public message: string; @@ -122,7 +86,7 @@ export class Commit { * @param commit - An instance of a Nodegit commit */ constructor(owner: Repository, commit: NodeGitCommit) { - this._ng_commit = commit; + this.ng_commit = commit; this._owner = owner; this.id = commit.sha(); this.date = commit.time(); @@ -135,7 +99,7 @@ export class Commit { * @returns An instance of a commit author */ public author(): CommitAuthor { - return new CommitAuthor(this._ng_commit); + return new CommitAuthor(this); } /** @@ -144,7 +108,7 @@ export class Commit { * @returns An instance of a diff */ public async diff(): Promise { - return new Diff((await this._ng_commit.getDiff())[0]); + return new Diff((await this.ng_commit.getDiff())[0]); } /** @@ -153,7 +117,7 @@ export class Commit { * @returns A diff stats instance */ public async stats(): Promise { - const stats = await (await this._ng_commit.getDiff())[0].getStats(); + const stats = await (await this.ng_commit.getDiff())[0].getStats(); return { insertions: stats.insertions(), @@ -168,18 +132,49 @@ export class Commit { * @returns An instance of a tree */ public async tree(): Promise { - return new Tree(this._owner, await this._ng_commit.getTree()); + return new Tree(this._owner, await this.ng_commit.getTree()); } /** * Returns whether or not the commit is signed */ public isSigned(): Promise { - return this._ng_commit.getSignature() + return this.ng_commit.getSignature() .then(() => true) .catch(() => false); } + /** + * Verify the commit's pgp signature + * + * @returns Whether or not the signature is valid + */ + public async verifySignature(): Promise { + const fingerprint = await this.author().fingerprint(); + + const pub_key = await pExec(`gpg --armor --export ${fingerprint}`).catch((err: ExecException) => { + return { + stdout: null, + stderr: err.message + }; + }); + + if(pub_key.stderr || !pub_key.stdout) { + throw(createError(ErrorWhere.Commit, FailedError, "export a public key from gpg!")); + } + + const pgp_signature = await this.ng_commit.getSignature(); + + return await verify({ + message: await createMessage({ text: pgp_signature.signedData }), + verificationKeys: await readKey({ armoredKey: pub_key.stdout }), + expectSigned: true, + signature: await readSignature({ armoredSignature: pgp_signature.signature }) + }) + .then(result => result.signatures[0].verified) + .catch(() => Promise.resolve(false)); + } + /** * Lookup a commit * diff --git a/packages/server/src/git/error/types.ts b/packages/server/src/git/error/types.ts index b8f2de7..b8c860b 100644 --- a/packages/server/src/git/error/types.ts +++ b/packages/server/src/git/error/types.ts @@ -36,4 +36,10 @@ export class CommitNotSignedError extends ErrorType { constructor() { super(500, "Commit isn't signed!"); } +} + +export class NotInKeyringError extends ErrorType { + constructor(email: string) { + super(500, `A public key for '${email}' doesn't exist in the server pgp keyring!`); + } } \ No newline at end of file diff --git a/packages/server/src/routes/api/v1/repo/log.ts b/packages/server/src/routes/api/v1/repo/log.ts index cf53d6b..aaad042 100644 --- a/packages/server/src/routes/api/v1/repo/log.ts +++ b/packages/server/src/routes/api/v1/repo/log.ts @@ -43,6 +43,8 @@ export default function(fastify: FastifyInstance, opts: FastifyPluginOptions, do const stats = await commit.stats(); + const is_signed = await commit.isSigned(); + const data: APICommit = { message: commit.message, author: { @@ -50,7 +52,8 @@ export default function(fastify: FastifyInstance, opts: FastifyPluginOptions, do email: commit.author().email, fingerprint: await commit.author().fingerprint().catch(() => null) }, - isSigned: await commit.isSigned(), + isSigned: is_signed, + signatureVerified: is_signed ? await commit.verifySignature().catch(() => false) : null, date: commit.date, insertions: stats.insertions, deletions: stats.deletions, diff --git a/packages/server/src/routes/api/v1/repo/map.ts b/packages/server/src/routes/api/v1/repo/map.ts index 0544e4f..a544d1a 100644 --- a/packages/server/src/routes/api/v1/repo/map.ts +++ b/packages/server/src/routes/api/v1/repo/map.ts @@ -3,6 +3,9 @@ import { LogCommit } from "api"; export async function commitMap(commit: Commit): Promise { const stats = await commit.stats(); + + const is_signed = await commit.isSigned(); + return { id: commit.id, author: { @@ -10,7 +13,8 @@ export async function commitMap(commit: Commit): Promise { email: commit.author().email, fingerprint: await commit.author().fingerprint().catch(() => null) }, - isSigned: await commit.isSigned(), + isSigned: is_signed, + signatureVerified: is_signed ? await commit.verifySignature().catch(() => false) : null, message: commit.message, date: commit.date, insertions: stats.insertions, diff --git a/test/unit/commit.unit.test.ts b/test/unit/commit.unit.test.ts index 21c04db..d302ea8 100644 --- a/test/unit/commit.unit.test.ts +++ b/test/unit/commit.unit.test.ts @@ -120,5 +120,14 @@ describe("Commit", () => { expect(fingerprint).toBeDefined(); }); + + it("Should verify the commit pgp signature and respond true", async() => { + expect.assertions(2); + + const verified = await commit.verifySignature(); + + expect(verified).toBeDefined(); + expect(verified).toBeTruthy(); + }); }); }); \ No newline at end of file -- cgit v1.2.3-18-g5258