Hi,
attached my attempt to make my previous patches a bit nicer, it
applies on top of the previous set and adds a "struct debsig_ctx" to
avoid passing three args (originID, deb, deb_fs) to the functions.
As always, feedback welcome :)
Thanks,
Michael
>From 6db400f2d938dc967e657d29e483420636d5080d Mon Sep 17 00:00:00 2001
From: Michael Vogt <m...@ubuntu.com>
Date: Wed, 20 Aug 2014 14:13:29 +0200
Subject: [PATCH] use new "struct debsig_ctx" instead of a global state
---
Makefile | 4 +--
ar-parse.c | 20 +++++++-------
debsig-verify.c | 82 ++++++++++++++++++++++++++-------------------------------
debsig.h | 15 ++++++++---
gpg-parse.c | 16 +++++------
misc.c | 4 +--
6 files changed, 71 insertions(+), 70 deletions(-)
diff --git a/Makefile b/Makefile
index 2dc0256..6a72b72 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,5 @@
CC = gcc
-CFLAGS = -Wall -Wextra -g -O2
+DS_CFLAGS = -Wall -Wextra -g3 -O2
#TESTING=1
@@ -31,7 +31,7 @@ MANPAGES = debsig-verify.1
all: $(PROGRAM) $(MANPAGES)
$(PROGRAM): $(OBJS)
- $(CC) $(MK_CFLAGS) $(CFLAGS) $(OBJS) $(MK_LDFLAGS) $(LDFLAGS) -o $@
+ $(CC) $(MK_CFLAGS) $(DS_CFLAGS) $(OBJS) $(MK_LDFLAGS) $(LDFLAGS) -o $@
install: all
install -d -m755 $(DESTDIR)/usr/bin
diff --git a/ar-parse.c b/ar-parse.c
index 5ff558a..8138e55 100644
--- a/ar-parse.c
+++ b/ar-parse.c
@@ -39,7 +39,7 @@
* nothing important is going to be zero length anyway, so we treat it as
* "non-existant". */
off_t
-findMember(const char *deb, const char *name, FILE *deb_fs)
+findMember(const struct debsig_ctx *ds_ctx, const char *name)
{
char magic[SARMAG+1];
struct ar_hdr arh;
@@ -53,12 +53,14 @@ findMember(const char *deb, const char *name, FILE *deb_fs)
}
/* This shouldn't happen, but... */
- if (deb_fs == NULL)
+ if (ds_ctx == NULL)
+ ds_fail_printf(DS_FAIL_INTERNAL, "findMember: called while ds_ctx == NULL");
+ if (ds_ctx->deb_fs == NULL)
ds_fail_printf(DS_FAIL_INTERNAL, "findMember: called while deb_fs == NULL");
- rewind(deb_fs);
+ rewind(ds_ctx->deb_fs);
- if (!fgets(magic,sizeof(magic),deb_fs))
+ if (!fgets(magic,sizeof(magic), ds_ctx->deb_fs))
ds_fail_printf(DS_FAIL_INTERNAL, "findMember: failure to read package (%s)",
strerror(errno));
@@ -68,9 +70,9 @@ findMember(const char *deb, const char *name, FILE *deb_fs)
return 0;
}
- while(!feof(deb_fs)) {
- if (fread(&arh, 1, sizeof(arh),deb_fs) != sizeof(arh)) {
- if (ferror(deb_fs))
+ while(!feof(ds_ctx->deb_fs)) {
+ if (fread(&arh, 1, sizeof(arh), ds_ctx->deb_fs) != sizeof(arh)) {
+ if (ferror(ds_ctx->deb_fs))
ds_fail_printf(DS_FAIL_INTERNAL, "findMember: error while parsing archive header (%s)",
strerror(errno));
return 0;
@@ -80,7 +82,7 @@ findMember(const char *deb, const char *name, FILE *deb_fs)
ds_fail_printf(DS_FAIL_INTERNAL, "findMember: archive appears to be corrupt, fmag incorrect");
dpkg_ar_normalize_name(&arh);
- mem_len = dpkg_ar_member_get_size(deb, &arh);
+ mem_len = dpkg_ar_member_get_size(ds_ctx->deb, &arh);
/*
* If all looks well, then we return the length of the member, and
@@ -97,7 +99,7 @@ findMember(const char *deb, const char *name, FILE *deb_fs)
return mem_len;
/* fseek to the start of the next member, and try again */
- if (fseek(deb_fs, mem_len + (mem_len & 1), SEEK_CUR) == -1 && ferror(deb_fs))
+ if (fseek(ds_ctx->deb_fs, mem_len + (mem_len & 1), SEEK_CUR) == -1 && ferror(ds_ctx->deb_fs))
ds_fail_printf(DS_FAIL_INTERNAL,
"findMember: error during file seek (%s)", strerror(errno));
}
diff --git a/debsig-verify.c b/debsig-verify.c
index 069cc7c..f5042a2 100644
--- a/debsig-verify.c
+++ b/debsig-verify.c
@@ -41,7 +41,7 @@ char *ver_magic_member = "debian-binary";
char *ver_ctrl_members[] = { CTAR(), CTAR(.gz), CTAR(.xz), 0 };
char *ver_data_members[] = { DTAR(), DTAR(.gz), DTAR(.xz), DTAR(.bz2), DTAR(.lzma), 0 };
-static int checkSelRules(const char *originID, struct group *grp, const char *deb, FILE *deb_fs) {
+static int checkSelRules(struct debsig_ctx *ds_ctx, struct group *grp) {
int opt_count = 0;
struct match *mtc;
int len;
@@ -53,8 +53,8 @@ static int checkSelRules(const char *originID, struct group *grp, const char *de
/* If we have an ID for this match, check to make sure it exists, and
* matches the signature we are about to check. */
if (mtc->id) {
- char *m_id = getKeyID(originID, mtc);
- char *d_id = getSigKeyID(deb, mtc->name);
+ char *m_id = getKeyID(ds_ctx->originID, mtc);
+ char *d_id = getSigKeyID(ds_ctx, mtc->name);
if (m_id == NULL || d_id == NULL || strcmp(m_id, d_id))
return 0;
}
@@ -64,7 +64,7 @@ static int checkSelRules(const char *originID, struct group *grp, const char *de
* specified, don't we?
*/
- len = checkSigExist(deb, mtc->name, deb_fs);
+ len = checkSigExist(ds_ctx, mtc->name);
/* If the member exists and we reject it, fail now. Also, if it
* doesn't exist, and we require it, fail as well. */
@@ -104,7 +104,7 @@ passthrough(FILE *in, FILE *out, off_t len)
return len;
}
-static int verifyGroupRules(const char *originID, struct group *grp, const char *deb, FILE *deb_fs) {
+static int verifyGroupRules(struct debsig_ctx *ds_ctx, struct group *grp) {
FILE *fp;
char tmp_sig[32] = {'\0'}, tmp_data[32] = {'\0'};
int opt_count = 0, t, i, fd;
@@ -131,23 +131,23 @@ static int verifyGroupRules(const char *originID, struct group *grp, const char
/* Now, let's find all the members we need to check and cat them into a
* single temp file. This is what we pass to gpg. */
- if (!(len = findMember(deb, ver_magic_member, deb_fs)))
+ if (!(len = findMember(ds_ctx, ver_magic_member)))
goto fail_and_close;
- len = passthrough(deb_fs, fp, len);
+ len = passthrough(ds_ctx->deb_fs, fp, len);
for (i = 0; ver_ctrl_members[i]; i++) {
- if (!(len = findMember(deb, ver_ctrl_members[i], deb_fs)))
+ if (!(len = findMember(ds_ctx, ver_ctrl_members[i])))
continue;
- len = passthrough(deb_fs, fp, len);
+ len = passthrough(ds_ctx->deb_fs, fp, len);
break;
}
if (!ver_ctrl_members[i])
goto fail_and_close;
for (i = 0; ver_data_members[i]; i++) {
- if (!(len = findMember(deb, ver_data_members[i], deb_fs)))
+ if (!(len = findMember(ds_ctx, ver_data_members[i])))
continue;
- len = passthrough(deb_fs, fp, len);
+ len = passthrough(ds_ctx->deb_fs, fp, len);
break;
}
if (!ver_data_members[i])
@@ -163,14 +163,14 @@ static int verifyGroupRules(const char *originID, struct group *grp, const char
/* If we have an ID for this match, check to make sure it exists, and
* matches the signature we are about to check. */
if (mtc->id) {
- char *m_id = getKeyID(originID, mtc);
- char *d_id = getSigKeyID(deb, mtc->name);
+ char *m_id = getKeyID(ds_ctx->originID, mtc);
+ char *d_id = getSigKeyID(ds_ctx, mtc->name);
if (m_id == NULL || d_id == NULL || strcmp(m_id, d_id))
goto fail_and_close;
}
/* This will also position deb_fs to the start of the member */
- len = checkSigExist(deb, mtc->name, deb_fs);
+ len = checkSigExist(ds_ctx, mtc->name);
/* If the member exists and we reject it, die now. Also, if it
* doesn't exist, and we require it, die as well. */
@@ -190,11 +190,11 @@ static int verifyGroupRules(const char *originID, struct group *grp, const char
goto fail_and_close;
}
- len = passthrough(deb_fs, fp, len);
+ len = passthrough(ds_ctx->deb_fs, fp, len);
fclose(fp);
/* Now, let's check with gpg on this one */
- t = gpgVerify(originID, tmp_data, mtc, tmp_sig);
+ t = gpgVerify(ds_ctx->originID, tmp_data, mtc, tmp_sig);
fd = -1;
unlink(tmp_sig);
@@ -229,41 +229,36 @@ fail_and_close:
return 0;
}
-static int checkIsDeb(const char *deb) {
- int i, res = 0;
+static int checkIsDeb(struct debsig_ctx *ds_ctx) {
+ int i;
const char *member;
- FILE *deb_fs = fopen(deb, "r");
-
- if (!findMember(deb, ver_magic_member, deb_fs)) {
+ if (!findMember(ds_ctx, ver_magic_member)) {
ds_printf(DS_LEV_VER, "Missing archive magic member %s", ver_magic_member);
- goto out;
+ return 0;
}
for (i = 0; (member = ver_ctrl_members[i]); i++)
- if (findMember(deb, member, deb_fs))
+ if (findMember(ds_ctx, member))
break;
if (!member) {
ds_printf(DS_LEV_VER, "Missing archive control member, checked:");
for (i = 0; (member = ver_ctrl_members[i]); i++)
ds_printf(DS_LEV_VER, " %s", member);
- goto out;
+ return 0;
}
for (i = 0; (member = ver_data_members[i]); i++)
- if (findMember(deb, member, deb_fs))
+ if (findMember(ds_ctx, member))
break;
if (!member) {
ds_printf(DS_LEV_VER, "Missing archive data member, checked:");
for (i = 0; (member = ver_data_members[i]); i++)
ds_printf(DS_LEV_VER, " %s", member);
- goto out;
+ return 0;
}
- res = 1;
- out:
- fclose(deb_fs);
- return res;
+ return 1;
}
static void outputVersion(void) {
@@ -314,7 +309,6 @@ int main(int argc, char *argv[]) {
struct dirent *pd_ent;
struct group *grp;
int i, list_only = 0;
- FILE *deb_fs;
dpkg_set_progname(argv[0]);
@@ -364,25 +358,25 @@ int main(int argc, char *argv[]) {
if (i + 1 != argc) /* There should only be one arg left */
outputUsage();
- const char *deb = argv[i];
+ struct debsig_ctx ds_ctx;
+ ds_ctx.deb = argv[i];
- if ((deb_fs = fopen(deb, "r")) == NULL)
- ds_fail_printf(DS_FAIL_INTERNAL, "could not open %s (%s)", deb, strerror(errno));
+ if ((ds_ctx.deb_fs = fopen(ds_ctx.deb, "r")) == NULL)
+ ds_fail_printf(DS_FAIL_INTERNAL, "could not open %s (%s)", ds_ctx.deb, strerror(errno));
if (!list_only)
- ds_printf(DS_LEV_VER, "Starting verification for: %s", deb);
+ ds_printf(DS_LEV_VER, "Starting verification for: %s", ds_ctx.deb);
- if (!checkIsDeb(deb))
- ds_fail_printf(DS_FAIL_INTERNAL, "%s does not appear to be a deb format package", deb);
+ if (!checkIsDeb(&ds_ctx))
+ ds_fail_printf(DS_FAIL_INTERNAL, "%s does not appear to be a deb format package", ds_ctx.deb);
- if ((tmpID = getSigKeyID(deb, "origin")) == NULL)
+ if ((tmpID = getSigKeyID(&ds_ctx, "origin")) == NULL)
ds_fail_printf(DS_FAIL_NOSIGS, "Origin Signature check failed. This deb might not be signed.\n");
- char originID[2048];
- strncpy(originID, tmpID, sizeof(originID));
+ strncpy(ds_ctx.originID, tmpID, sizeof(ds_ctx.originID));
/* Now we have an ID, let's check the policy to use */
- snprintf(buf, sizeof(buf) - 1, DEBSIG_POLICIES_DIR_FMT, rootdir, originID);
+ snprintf(buf, sizeof(buf) - 1, DEBSIG_POLICIES_DIR_FMT, rootdir, ds_ctx.originID);
if ((pd = opendir(buf)) == NULL)
ds_fail_printf(DS_FAIL_UNKNOWN_ORIGIN,
"Could not open Origin dir %s: %s\n", buf, strerror(errno));
@@ -412,7 +406,7 @@ int main(int argc, char *argv[]) {
/* Now let's see if this policy's selection is useful for this .deb */
ds_printf(DS_LEV_VER, " Checking Selection group(s).");
for (grp = pol->sels; grp != NULL; grp = grp->next) {
- if (!checkSelRules(originID, grp, deb, deb_fs)) {
+ if (!checkSelRules(&ds_ctx, grp)) {
clear_policy();
ds_printf(DS_LEV_VER, " Selection group failed checks.");
pol = NULL;
@@ -444,9 +438,9 @@ int main(int argc, char *argv[]) {
ds_printf(DS_LEV_VER, " Checking Verification group(s).");
for (grp = pol->vers; grp; grp = grp->next) {
- if (!verifyGroupRules(originID, grp, deb, deb_fs)) {
+ if (!verifyGroupRules(&ds_ctx, grp)) {
ds_printf(DS_LEV_VER, " Verification group failed checks.");
- ds_fail_printf(DS_FAIL_BADSIG, "Failed verification for %s.", deb);
+ ds_fail_printf(DS_FAIL_BADSIG, "Failed verification for %s.", ds_ctx.deb);
}
}
diff --git a/debsig.h b/debsig.h
index 367fb9a..ea6edb7 100644
--- a/debsig.h
+++ b/debsig.h
@@ -60,11 +60,20 @@ struct policy {
struct group *vers;
};
+// the debsig context
+struct debsig_ctx {
+ char *deb;
+ FILE *deb_fs;
+ char originID[2048];
+};
+
struct policy *parsePolicyFile(const char *filename);
-off_t findMember(const char *deb, const char *name, FILE *deb_fs);
-off_t checkSigExist(const char *deb, const char *name, FILE *deb_fs);
+
+off_t findMember(const struct debsig_ctx *ds_ctx, const char *name);
+off_t checkSigExist(const struct debsig_ctx *ds_ctx, const char *name);
+char *getSigKeyID (const struct debsig_ctx *ds_ctx, const char *type);
+
char *getKeyID (const char *originID, const struct match *mtc);
-char *getSigKeyID (const char *deb, const char *type);
int gpgVerify(const char *originID, const char *data, struct match *mtc, const char *sig);
void clear_policy(void);
diff --git a/gpg-parse.c b/gpg-parse.c
index 7c5b27c..146807c 100644
--- a/gpg-parse.c
+++ b/gpg-parse.c
@@ -97,19 +97,16 @@ char *getKeyID (const char *originID, const struct match *mtc) {
return ret;
}
-char *getSigKeyID (const char *deb, const char *type) {
+char *getSigKeyID (const struct debsig_ctx *ds_ctx, const char *type) {
char buf[2048];
int pread[2], pwrite[2], t;
pid_t pid;
FILE *ds_read, *ds_write;
char *c, *ret = NULL;
- FILE *deb_fs = fopen(deb, "r");
- off_t len = checkSigExist(deb, type, deb_fs);
- if (!len) {
- fclose(deb_fs);
+ off_t len = checkSigExist(ds_ctx, type);
+ if (!len)
return NULL;
- }
gpg_init();
@@ -138,8 +135,8 @@ char *getSigKeyID (const char *deb, const char *type) {
/* First, let's feed gpg our signature. Don't forget, our call to
* checkSigExist() above positioned the deb_fs file pointer already. */
do {
- t = fread(buf, 1, sizeof(buf), deb_fs);
- if (ferror(deb_fs))
+ t = fread(buf, 1, sizeof(buf), ds_ctx->deb_fs);
+ if (ferror(ds_ctx->deb_fs))
ds_fail_printf(DS_FAIL_INTERNAL, "getSigKeyID: error reading signature (%s)",
strerror(errno));
@@ -148,7 +145,7 @@ char *getSigKeyID (const char *deb, const char *type) {
else
fwrite(buf, 1, t, ds_write);
len -= t;
- } while(len > 0 || !feof(deb_fs));
+ } while(len > 0 || !feof(ds_ctx->deb_fs));
if (ferror(ds_write))
ds_fail_printf(DS_FAIL_INTERNAL, "error writing to gpg");
@@ -179,7 +176,6 @@ char *getSigKeyID (const char *deb, const char *type) {
else
ds_printf(DS_LEV_DEBUG, " getSigKeyID: got %s for %s key", ret, type);
- fclose(deb_fs);
return ret;
}
diff --git a/misc.c b/misc.c
index 03b6bd5..9219024 100644
--- a/misc.c
+++ b/misc.c
@@ -43,7 +43,7 @@ void ds_printf(int level, const char *fmt, ...) {
}
off_t
-checkSigExist(const char *deb, const char *name, FILE *deb_fs)
+checkSigExist(const struct debsig_ctx *ds_ctx, const char *name)
{
char buf[16];
@@ -54,5 +54,5 @@ checkSigExist(const char *deb, const char *name, FILE *deb_fs)
snprintf(buf, sizeof(buf) - 1, "_gpg%s", name);
- return findMember(deb, buf, deb_fs);
+ return findMember(ds_ctx, buf);
}
--
2.0.0.rc0
