On Sun, Aug 31, 2014 at 06:10:36AM +0200, Guillem Jover wrote:
> Hi!
Hello,
> On Tue, 2014-08-19 at 11:23:41 +0200, Michael Vogt wrote:
[..]
> Some comments on the points raised in the review, although it's true that
> dpkg itself should only be dealing with “trusted” data, otherwise you are
> going to be happily giving root accesss away, dpkg-deb does not, so it
> must be picky and suspicious when parsing .deb packages. And for most (if
> not all) of the dpkg .deb parsing code I've either rewritten or at least
> extensively reviewed it by now, that obviously does not mean there will
> be no bugs, but besides code staring, unit tests, functional tests [F],
> code checkers like clang, cppcheck and coverity among others do help. So
> I do trust more the dpkg code than the debsig-verify code. Precisely one
> of the reasons for taking it over was to update its .deb format support,
> including LFS. Of course debsig-verify code should be considered more
> sensitive, because it's not just about inspecting, but about deciding
> to end up giving direct root access to possibly untrusted packages.
[..]
thanks for these comments, that is good to know!
> Regarding adoption of debsig-verify, I'm planning to work on updating
> the layout of the signatures, and to properly integrate this into dpkg
> proper. Once I start those discussions, I'll try to make sure to keep you
> and Colin Watson on the loop, as you guys seem to be interested in
> this?
Yes, please keep us in the loop.
> > Attached are two patches that add some additional error checking.
>
> I'll review and merge those in few days, after I finish up some other
> stuff, thanks!
Thanks, that is much appreciated.
> > I also started with the removal of the global state
> > (attached as well). However it is not very elegant and I wonder if it would
> > make more sense to have a
> > """
> > struct ds_ctx {
> > char *deb,
> > FILE *deb_fs,
> > char *originID
> > }
> > """
> > that is passed around as the context instead of my current approach.
>
> Ah, yeah, I thought I had started doing something like that already,
> but I cannot find any branch or stashed change, so either I just
> thought about it or I discarded it at the time. Anyway I'll check
> it out in few days.
[..]
Great, looking forward for your feedback. I guess I need to rework the
coding style a bit (based on the previous review I had) but I guess
its best if I wait for further feedback.
Attached are my remaining patches that add _FILE_OFFSET_BITS 64, add
a README and add a (simple) integration test (with a test origin key).
The test is not using a test framework currently, I'm happy to use
whatever you suggest, shunit2 seems like a good one but I have no
strong preferences either way.
Feedback welcome, and I hope my latest stuff does not contain silly
(debconf jetlag) issues :)
Cheers,
Michael>From 1a4ee2063424f94f4d481f737870892bcf50e8aa Mon Sep 17 00:00:00 2001
From: Michael Vogt <m...@ubuntu.com>
Date: Thu, 21 Aug 2014 08:30:22 +0200
Subject: [PATCH 7/9] add _FILE_OFFSET_BITS 64
---
debsig.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/debsig.h b/debsig.h
index ea6edb7..39e78ab 100644
--- a/debsig.h
+++ b/debsig.h
@@ -16,6 +16,7 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
+#define _FILE_OFFSET_BITS 64
#define DEBSIG_POLICIES_DIR_FMT "%s"DEBSIG_POLICIES_DIR"/%s"
#define DEBSIG_KEYRINGS_FMT "%s"DEBSIG_KEYRINGS_DIR"/%s/%s"
--
2.0.0.rc0
>From 79318503b0039b4705019e0308544ceee7f24305 Mon Sep 17 00:00:00 2001
From: Michael Vogt <m...@ubuntu.com>
Date: Thu, 21 Aug 2014 08:36:18 +0200
Subject: [PATCH 8/9] add README
---
README | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 README
diff --git a/README b/README
new file mode 100644
index 0000000..150a35f
--- /dev/null
+++ b/README
@@ -0,0 +1,22 @@
+= Debian package signature verification tool =
+
+This tool inspects and verifies binary package digital signatures based
+on predetermined policies, complementing repository signatures or allowing
+to verify the authenticity of a package even after download when detached
+from a repository.
+
+== How to build ==
+
+Ensure the build-dependencies are instaleld by running
+```
+$ dpkg-checkbuilddeps debian/control
+```
+
+then type:
+```
+$ make
+```
+
+== Testing ==
+
+No automatic testsuite yet, manual testing needs to be performed.
--
2.0.0.rc0
>From df421bdccf43ae520f676c9d1da0ab5788f1e3a0 Mon Sep 17 00:00:00 2001
From: Michael Vogt <m...@ubuntu.com>
Date: Tue, 2 Sep 2014 09:52:48 +0200
Subject: [PATCH 9/9] add simple integration test
---
Makefile | 3 +
README | 6 +-
testing/keyrings/FAD46790DE88C7E2/pubring.gpg | Bin 0 -> 1245 bytes
testing/keyrings/FAD46790DE88C7E2/secring.gpg | Bin 0 -> 2547 bytes
testing/policies/FAD46790DE88C7E2/generic.pol | 22 +++++++
testing/test_debsig | 79 ++++++++++++++++++++++++++
6 files changed, 109 insertions(+), 1 deletion(-)
create mode 100644 testing/keyrings/FAD46790DE88C7E2/pubring.gpg
create mode 100644 testing/keyrings/FAD46790DE88C7E2/secring.gpg
create mode 100644 testing/policies/FAD46790DE88C7E2/generic.pol
create mode 100755 testing/test_debsig
diff --git a/Makefile b/Makefile
index 6a72b72..eebebaf 100644
--- a/Makefile
+++ b/Makefile
@@ -49,6 +49,9 @@ check:
ln -s /usr/share/keyrings/debian-keyring.gpg testing/keyrings/7CD73F641E04EC2D/
# XXX: Do some actual testing here.
+test:
+ ./testing/test_debsig
+
clean:
rm -f debsig-verify $(OBJS) $(MANPAGES)
rm -f testing/keyrings/7CD73F641E04EC2D/debian-keyring.gpg
diff --git a/README b/README
index 150a35f..40c1587 100644
--- a/README
+++ b/README
@@ -19,4 +19,8 @@ $ make
== Testing ==
-No automatic testsuite yet, manual testing needs to be performed.
+Run:
+```
+$ make test
+```
+
diff --git a/testing/keyrings/FAD46790DE88C7E2/pubring.gpg b/testing/keyrings/FAD46790DE88C7E2/pubring.gpg
new file mode 100644
index 0000000000000000000000000000000000000000..149108fe8b0b36968fa6d562a016f0977f87bc22
GIT binary patch
literal 1245
zcmV<31S0#H0SyFH1!%Sb2msyiZv!0jSLNi<yH019h*FF7o04KrkqjUs!^`-YFPy41
zj|#m<AtCDg+<PN#T`qRyvRN77Us4BL_7><s^3Lt+@8E*xh6z*YrCA-C0cEc2OdeY4
z4?==6=0`VcFNoz6v>I~OuavMz{4?3NWQzO!5fPaZyrxh6of9vanb#GrRkP+DHlRqc
z)$Yo3gnYICSS^Y@RR=&K$9;d(-TxmdGN^pFSz~lU$dYdv(;ANtdL3K&l)4>m#uE~~
zaJtdn{1C|3(7Gi=zFffmDL_A;64AVN9tHV8rqVQUPz)cajE_Rb&OT%qejqlD=xbQo
z0*3-=N4#e=u?<ZN?gao50RRECRzziDb7^NFNp5syXL4b5X>V>IRAqB?AWLO=AShI5
zX>%ZJWqBZJb0B7Kav&;nWpi|CZf7bWZ*FXPDIh##Wnyz_XDxJPb96vucwudDY-KKE
zZ*4w_0XPH`0RjLb1p-tBXtn|y0|pBT2nPcK1{DYb2?`4Y76JnS0v-VZ7k~f?2@v|!
zXOP~A$KroE2mpZmJD$uIDNP11k*4H31gB=iqn0$^oupGeY2T$P?D#-Nw;W~NR`I=m
zabr^^$%!w@6s_C%LLrTAl>)rJ{Tx!M*ulEVNgq~V=;0?7@u<8CC28YpZbBzp&?M(0
zCMRS?l%tiAR;dy8EFR<5(bkEMhySTV0Io>QdN}>Bi8!IOk0Fi0qL2izM--e}PJ$c_
zx^ZTZ7&XZt?lR)};lpEqZ-~1Toaxt|ZxksNO^uW;`3jX|IDL<EA;dh$=$|=1^uutL
zf<O<acb>QOe)tPC+OFQ0Ao%q2Zgff++xfE0#q$c`1zPybnYJ|oTiz&f9~rg>H2={X
zQv!-GSEsN700X%J4FpsLXtn_e0LOvLtTzBzfz-yZcxK?TNZI=fPZxlnM{zi@3Y7WR
zNg`&|i&S;H;o>VffARb6_}7LZUPS`or6wfn3D;KDAK_FV&#&?i4MP9~rT}Vk9&rT)
z5X#idEpgsbKo1Yl-3oqVkJT=x%UE@#m;lDQE8P@GM)Qv9H8XzktvaC&+yI-8++it^
zK3dpk#b&dhB@yPm9k8{92m++q8de2g-8yl|W>*C4Wd(5`oUc#fIFY%>_2t;L9r??X
z<zE^VIqBf$sY$K}OvmfqbP%FCTUX)7fywt%`z${Ban2)}zjjrDjz>i#cb;n^t~lt0
za){4~W?0O;zTcJcIZ8@)wEz(T00D^s9|RZy0ssjG0#pTPwgMXr0162Z`qXET-iXKI
zD&q(Lc!wzc;E}Md{3GBszf{6#wOPce0K`!%!@oqSk$QipyKPl)i5)#I2#vm;!xn5M
zU{0n$%#vR6+N>ZY2Uw4<ocl%7#S7AWj31!cUnbw0)DXR;Gm0fE8S<}a>IgSxbDS0~
zV@{cy&AlyKji<JDbmJl=E9FB6zr$5?r8IAAlm$VQ)2OlXMQ-cfO&O0VGz8r<lhgev
zcq;KZl#hA!NY4D}+nQRJBVwzt$8pZ>O_QC?MF^LW)N5y%NdpQ(m*F`v&`t%M6V$ht
zv{j%Bd|?L@k0c}r{5jRA8&Rc)MXfkQkt_&$P*UzAGOE~Y#CL}Z(j1S5&~R)U_rDIR
HumS)BvkNn!
literal 0
HcmV?d00001
diff --git a/testing/keyrings/FAD46790DE88C7E2/secring.gpg b/testing/keyrings/FAD46790DE88C7E2/secring.gpg
new file mode 100644
index 0000000000000000000000000000000000000000..6bb40a6afad1aff6beb43f945021d7c148cf7889
GIT binary patch
literal 2547
zcmV<P2@Lj?1DFI<1!%Sb2msyiZv!0jSLNi<yH019h*FF7o04KrkqjUs!^`-YFPy41
zj|#m<AtCDg+<PN#T`qRyvRN77Us4BL_7><s^3Lt+@8E*xh6z*YrCA-C0cEc2OdeY4
z4?==6=0`VcFNoz6v>I~OuavMz{4?3NWQzO!5fPaZyrxh6of9vanb#GrRkP+DHlRqc
z)$Yo3gnYICSS^Y@RR=&K$9;d(-TxmdGN^pFSz~lU$dYdv(;ANtdL3K&l)4>m#uE~~
zaJtdn{1C|3(7Gi=zFffmDL_A;64AVN9tHV8rqVQUPz)cajE_Rb&OT%qejqlD=xbQo
z0*3-=N4#e=u?<ZN?gao50RRC22l@iaNSsxD?69}=qPKDiAH80f>7QNeiT()l5-2NV
zSD+-T{@$O*4P~lXHB7n#4n+bmd0K{)1g@?UFEfC!(pOF;Ldb*Pbs7T|_@c$aY9&9i
zr1@53YTWVgmkSF1ybh~JL%1SmOkqouW&8GzC_;n`{em&JV1e<$s*oNUDXQ9fM04A;
z`tGjbLXTO7qc?uXVnB>n;?VKKvpFQ(Az=XU_+%f)#<S=I;b6RA{M~H!D3Zf~DL<jU
zT<T=5ooR>7HE*)JJheT^`1Z0o)M7(q--RJ6Y(lQx_H{!dr_7+Put%a(q;=HNN@Jd}
zwi9*Ff|V~DCS)oii*R8#6~P1m->P?N*|RUY0>$t~h!!f3qb{7$cHJtf^o}j*#hL<|
z>A|GX^)ySjr=#T4&Q6U;b;<|Mt{XlBnV(qmk+2&<em2LKzf+A5CQ|`*-6!pF0Q2y+
zVQ-Zh8&S3M|2rc|WJFlO{@%uZAeWdcU!7@A)co#$VH-$MGU{UpX>>}#1OWa95TYLk
zgP;g_TW>d#M=BbN1oyACsy?%7sA6DI!egsI@J`o6BfzH4TU)TehkQM=ud-{KTGy!R
ze1KMh4dXU?y~x6sP;ic@%rq;=$uztz<UgjW*rPdkTKmk%SG#EY3Dr-A9&qbF<m@p8
zI`2wD2259$ne&+^k5O8U$VCJI?kGpHhqOdE8^TpL`l^~=QPkquB|jy4Ta_1#x}(1v
z{GJ0+sxY549D+V2D=B+q-C)TKXa-42AM(h;($=9)OH(Ea#u2(->V7M<ZmPVw%~R#z
zpn`Wf6Z@^nFzvycs|J{g-DZL&Y4=s2JB2Dm*M|o#Quys;h+CwN?sA*dLjSZ@L}g-g
zX=flwZggd5a$$67Z*Cw|Wpi{OOJ#W=C{$=^b0BMFc_3+XAZBlJAS!fab98BLXDT3X
zZftoeAUtGcVsmL{Ep%mbbU<ZzVQp}1WiDfHZ9a(sI0O>`0stZf0#pTPwgMXi1`7!Y
z2Ll2I6$k<e3JU}l0s{d89svRufB*^!5c<?-klu*L;(s{^0D$~Ep3D|0O$INKrsO*W
zr)I>XmNeg;q*Fa<-=!+-_&`Rt9A({B@x6a>V^b!{i7(0&t=srQA&qX80=&Nc98#&+
z!Me#wA68)K;U^XGsJsd#Y2#~dLML0$B<CY0CuBvGqm_|XsS)-p9^=;0)`^aX|EWU&
zu1L*#IQ_4QIH9zUA&tSJkOZ(t6r5X5f*cLHab}PhHOV0EGUEB+!()JNh`SV=>DQib
z6e$%=jg&6=3YB9xeUEb?#5~C8pE*DD!*G^@Ko6&Pp11UV_zN`JuHKg*`1JE`bV?c9
z`LfK#^9tbwTKLSFwlxA<-Y9V&8MX&B|Ir&$0*WwKr?3J50G$Jv1XKlRwgCtL$AQbN
zHvn0I)W)!QX5g|&+4~Dm7l5BfaX7IGl=;_5B4*W#RCT-I;ww3S@%!!g*M=cpMFQca
zCM4?#*H+aZ;Zz^buksHKLjVM(0BUj`aRmhs%GAs)ao$ou4-e4Y3Vvgc)h?&YSaqeC
z0LHp2-4sYh^N#8@Gk)={I-w5S0Gp58VJVS5TG(gBX0xCr5$3%eu(gE<0;Jj+Rs~<(
zI&sKmR|M>31#usouTSDQk-5k9<=C|y`OA~#Um6uT>EPz6Nv;P>$Lrp75TZI;SK-Hj
z$@f$HEI#>h&Lf(?c2$9nM@1xeo@*kmIOv6Ph|h^;Sj@b>-<9z>N=kOM01*KI0RRX4
z2Vrqv`yT7C#%A49ZuaC6mg3hf!(dWGDc7Ho6(-XJl^wauutBuMECNnj!QX(B!l7p^
zo*wtB4Cu4FM);!S&4{<Ij>APXN94iBL|?1E9gjkLHx0c>_Zsr=1izq(^xy3r{*-;k
zql==aCu}mQ`N2%pHz0H1sQHS<PE&LNwZ<c`ARqQ{?AWq6#h3gy>kyq&7O5Z%#gesr
ztiF3m3b=!NN&jx_)Ws)=(Rh1EA*gvf>fiL&Q>c9HR-l>dd#nG<8A0QV$}qbYU$<eR
z`<NhA*z!(8K)W2sk6xswn{g(+HA4gtyhfub$xOn|hmY70<UlNdXW=s5WREHL1OUyC
zPFD`Ltp+@X5j6%;a3<8Le9vo@GA;<QcNA{tWoL`nbh<lL5H@@@L05T*N6dQClRo97
z<a>uYJi6w>Dv3o70&U;owtrppZKro9P%watzz!g+I*(U(U8%Py^FOW`#@sjYyH0te
z{%2JYu75j(#8cqG)_XNf*b<MsW={kF_;y8ROnF7iiuJ8Qcjp&AB(J!dOQv(PQDIr;
z<60>$B^}YkA-o;bfI_8&?yu3Wo~_~W+T+}bL&7)~oftjhli=_tJ#xQQ!NTta0Wq{x
z*HaXJs$$408dCf8YrfU)h}uo`HVvYMjPqocs*PkakUy(qq8#8aKMax7V2Rbc1OVE`
z#orbU-6Q@<*h+c$ITpw&*G(9j3mQe%hL}v(#e?DA7rF1RcYDkmkm@%D5mX8A`e2O%
z6wFe4ZbTQ{NSH{E`_p~f@!7+)l@E`6oZ#x@(@&f@c%MbKfdAsgMn~AkOb<5ks6dD3
zF4KpcWM4BJ!4#r26+eDhRBohWT!~61i2)x37y$wR2?YXF1!%Sc8w>yn2@v|!XOP~A
z$KopE2mg47DE;7(u&(?g;5ENg!f3Tw#Hj$pQ7gm0M5&Q_f2X@`Rd9(NJuV21zMjJt
zY$ae$ra;V+Uh>+kASDM_kFT8jMbpI#(tL~`pxIw0-<s4Ay`?jXB`X>7uW0HBH)eC3
z7A|8>nVZeMEnAJJwsv&mA|)&3LkGXZRdb~@Z)=nVL6p;|vGPT3>)uTnk0~?+-7}NZ
z{V8}V@i~-_dG$!n{OQ}8T9+eYtFXs$&h1T;oy|oEmypzJXPHR@3PP9RIWf>q1)LMq
zx0ke4pbLCq2NRDZBnbRD)u<a$rH4hWI7E>w2zpRb?jtg)*lfgihYHdhkA~22Y#aB#
J4yv#M008`s!-)U@
literal 0
HcmV?d00001
diff --git a/testing/policies/FAD46790DE88C7E2/generic.pol b/testing/policies/FAD46790DE88C7E2/generic.pol
new file mode 100644
index 0000000..998e813
--- /dev/null
+++ b/testing/policies/FAD46790DE88C7E2/generic.pol
@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+<!DOCTYPE Policy SYSTEM "http://www.debian.org/debsig/1.0/policy.dtd";>
+<Policy xmlns="http://www.debian.org/debsig/1.0/";>
+
+ <!-- This is mainly a sanity check, since our filename is that of the ID
+ anyway. -->
+ <Origin Name="Debsig" id="FAD46790DE88C7E2" Description="Debsig testing"/>
+
+ <!-- This is required to match in order for this policy to be used. We
+ reject the release Type, since we want a different rule set for
+ that. -->
+ <Selection>
+ <Required Type="origin" File="pubring.gpg" id="FAD46790DE88C7E2"/>
+ </Selection>
+
+ <!-- Once we decide to use this policy, this must pass in order to verify
+ the package. -->
+ <Verification MinOptional="0">
+ <Required Type="origin" File="pubring.gpg" id="FAD46790DE88C7E2"/>
+ </Verification>
+
+</Policy>
diff --git a/testing/test_debsig b/testing/test_debsig
new file mode 100755
index 0000000..518a681
--- /dev/null
+++ b/testing/test_debsig
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+set -e
+
+test_success() {
+ printf "Testing for succes $@:"
+ if $@ > output 2>&1; then
+ printf " PASS\n"
+ else
+ printf " FAIL\n"
+ fi
+}
+
+test_failure() {
+ printf "Testing for failure $@:"
+ if ! $@ > output 2>&1; then
+ printf " PASS\n"
+ else
+ printf " FAIL\n"
+ fi
+}
+
+setup_tmp() {
+ BASEDIR="$(dirname $(readlink -f $0))"
+ TMPDIR="$(mktemp -d)"
+ trap 'rm -rf "$TMPDIR"' EXIT HUP INT QUIT KILL TERM
+}
+
+setup_debsig_home() {
+ cd "$TMPDIR"
+ mkdir -p etc/debsig/
+ mkdir -p usr/share/debsig/
+
+ cp -ar $BASEDIR/policies/ etc/debsig/
+ cp -ar $BASEDIR/keyrings/ usr/share/debsig
+}
+
+make_test_deb() {
+ cd "$TMPDIR"
+
+ # make deb
+ DEBDIR="debsig-test1_1.0"
+ mkdir -p $DEBDIR/DEBIAN
+ cat > $DEBDIR/DEBIAN/control <<EOF
+Package: debsig-test1
+Architecture: all
+Version: 1.0
+Maintainer: Bot <b...@example.com>
+Description: Signature Test Package
+EOF
+ mkdir -p $DEBDIR/usr/share/doc/
+ printf "Debsig testing deb" > $DEBDIR/usr/share/doc/README
+ dpkg-deb -b $DEBDIR .
+ DEB="$(ls *.deb)"
+}
+
+make_signed_deb() {
+ # make signature
+ ar p "$DEB" | gpg --home $BASEDIR/keyrings/FAD46790DE88C7E2/ --detach-sig > _gpgorigin
+ ar q "$DEB" _gpgorigin
+ rm -f _gpgorigin
+}
+
+test_unsigned_deb_does_not_verify() {
+ test_failure "$BASEDIR/../debsig-verify" -v -d --rootdir "$TMPDIR" "$DEB"
+}
+
+test_signed_deb_verifies() {
+ test_success "$BASEDIR/../debsig-verify" -v -d --rootdir "$TMPDIR" "$DEB"
+}
+
+setup_tmp
+setup_debsig_home
+
+make_test_deb
+test_unsigned_deb_does_not_verify
+
+make_signed_deb
+test_signed_deb_verifies
--
2.0.0.rc0
