bug#23513: package.el treats empty signatures as correct

bug#23513: package.el treats empty signatures as correct

[L. Dixon]

Hi!

I noticed an issue in package.el checking malformed and empty
signatures. It behaves as if malformed and empty signatures are
correct.

You can validate this by evaling the following lisp:

    (setq package-check-signature t) ;; or 'alllow-unsigned2
    (package--check-signature-content "a" "b") ;; => nil, no signal

The issue is a result of the following code (from package.el, 62d7aca,
current HEAD of master) in lines 1208-1223, the definition of
package--check-signature-content:

    (let (good-signatures had-fatal-error)
      ;; The .sig file may contain multiple signatures.  Success if one
      ;; of the signatures is good.
      (dolist (sig (epg-context-result-for context 'verify))
        ;; [elided... conditionally set good-signatures or had-fatal-error]
	)
      (when (and (null good-signatures) had-fatal-error)
        (package--display-verify-error context sig-file)
        (signal 'bad-signature (list sig-file))))pg-
	
epg-context-result-for returns nil for malformed or empty signatures;
in this case the body of the dolist never gets evaluated for any sig,
and so both good-signatures and had-fatal-error end up nil. The
signal doesn't get triggered and package--check-signature-content
returns normally.

I've include a patch and some additional cases for the test
suite. The new tests fail against HEAD of master and pass with the
patch applied.

This patch includes a new
test/lisp/emacs-lisp/package-resources/key.sec and signatures, since I
couldn't find the passphrase for the existing one and needed to sign
/test/lisp/emacs-lisp/package-resources/signed/archive-contents for
the new test. As a result, this patch contains binary differences and
so needs to be applied with git-apply. The passphrase for the new key
is 'passphrase'. Happy to use the old key if someone knows how.

I also deleted the skip-unless clause in the package-test-signed,
since the test runs normally without it. I may be misunderstanding
something here, but I'm worried that skipping this test will mask
similar issues or regressions.

Thanks,

Lizzie.

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index c05bb53..9fc2451 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1218,7 +1218,7 @@ package--check-signature-content
           (unless (and (eq package-check-signature 'allow-unsigned)
                        (eq (epg-signature-status sig) 'no-pubkey))
             (setq had-fatal-error t))))
-      (when (and (null good-signatures) had-fatal-error)
+      (when (or (null good-signatures) had-fatal-error)
         (package--display-verify-error context sig-file)
         (signal 'bad-signature (list sig-file)))
       good-signatures)))
diff --git a/test/lisp/emacs-lisp/package-resources/key.pub b/test/lisp/emacs-lisp/package-resources/key.pub
index a326d34..b3bd7a5 100644
--- a/test/lisp/emacs-lisp/package-resources/key.pub
+++ b/test/lisp/emacs-lisp/package-resources/key.pub
@@ -1,18 +1,30 @@
 -----BEGIN PGP PUBLIC KEY BLOCK-----
-Version: GnuPG v1.4.14 (GNU/Linux)
+Version: GnuPG v2
 
-mQENBFJNB8gBCACfbtpvYrM8V1HM0KFlIwatcEJugHqwOHpr/Z9mrCW0fxyQAW/d
-2L+3QVNsN9Tz/K9lLcBUgeR7rhVEzHNqhmhNj/HnikwGqXbIofhp+QbZmBKnAlCz
-d77kg8K9lozHtfTkm1gX/7DdPzQKmgi7WOzzi2395wGubeqJLvYaEcqVbI0Eob+E
-3CzRjNy/e/Tf3TJRW5etTcdZN6LVuIY7tNCHqlQZTwyycON/hfLTX6cLCnzDsqm/
-NxCuwn9aqP9aGRGfIu7Y+If3zTymvrXEPUN98OEID814bOKdx0uVTZRiSMbvuTGI
-8uMa/kpGX/78rqI61gbZV51RFoU7pT2tzwY/ABEBAAG0HkouIFIuIEhhY2tlciA8
-anJoQGV4YW1wbGUuY29tPokBOAQTAQIAIgUCUk0HyAIbAwYLCQgHAwIGFQgCCQoL
-BBYCAwECHgECF4AACgkQtpVAhgkYletuhQf+JAyHYhTZNxjq0UYlikuLX8EtYbXX
-PB+03J0B73SMzEai5XsiTU2ADxqxwr7pveVK1INf+IGLiiXBlQq+4DSOvQY4xLfp
-58jTOYRV1ECvlXK/JtvVOwufXREADaydf9l/MUxA5G2PPBWIuQknh3ysPSsx68OJ
-SzNHFwklLn0DKc4WloE/GLDpTzimnCg7QGzuUo3Iilpjdy8EvTdI5d3jx/mGJIwI
-goB+YZgyxSPM+GjDwh5DEwD7OexNqqa7RynnmU0epmlYyi9UufCHLwgiiEIzjpWi
-6+iF+CQ45ZAKncovByenIUv73J3ImOudrsskeAHBmahljv1he6uV9Egj2Q==
-=b5Kg
+mQENBFcy0X0BCADTEpqKxj/mPhlMReSTS4Tt+Z3FIWh9J/Ry9xOXejJaOf/0IK4p
+svA0fm4bIZA1sBtQw7KIu+oTVEllNIQG4qxVHHLqwQx+/F3Rk+dOk0Flk+zmBT2n
+F+4KCnnrK7MOjcOMNQept4YkgZd3GPkBFCAr5RPTqxy6wn7Y1/NDzuHDUvns1FpR
+GxRY5vyoghs1Yei6V1uGatNgxoEtNWMn2j60IPypnP961sGKZ8MHkeS0qeEVLbjI
+PZ/qAFSYSgKg4GaC4+aRL9iABYdroMsNW/yaYTTnYp25t0X7w+eG9eKZD8hsidTj
+E8ZFE/En0inCK2UhkzcAj3dAvzQJo1VV2S35ABEBAAG0HUouUi4gSGFja2VyIDxq
+cmhAZXhhbXBsZS5jb20+iQE3BBMBCAAhBQJXMtF9AhsDBQsJCAcCBhUICQoLAgQW
+AgMBAh4BAheAAAoJEE68tnACTKitvN8IAIw+/H6VM1yP4So6HrOcYAJgSR5prOWI
+c5kywJKGtdmc3DzniFxm5X5a2ARXpqaIq+5i0xQib+8SE173XsE68bNBe0OwsyRL
+BWr5Gqg7gviHk8+8FmytccPSIso3fXZYrG74LHzG93N6cdp6zfGJvxHNvuVg2Ufn
+kn9KmYfBcVHrYsouvPmbv7qjCVgrD8bUIr4maAtFocycxcOez5bZGhGiPVL+I4/C
+8+TpBbWWsoTXo7VNWa6dvGFBgja38WPGyshExbs/SMoCkHEnUcV6uUyIZstEugvs
+aAAjLk1LVPHs+juOls1JaCuxG7oquzNh9tSAZ2ZEG0bu0T5pkO4TTc65AQ0EVzLR
+fQEIANPWOPCkSJomBN4BMsOmQj1RiIPMFCRS+XNRhrsUiHY2vSvSujAkemvgzf0Z
+X8CYHMgo2hSH9ehcCUZryEBHcZDzkxS3E+/rk6YZhiEarWdT4O9Oi4v5ct224BLg
+h1oWBwa/ypCIF8ebtZTLkWe4jkaAjKMHpgwL/ndHRJXPIN8h3Zbb9j8v5C5Y1MkR
+Ppc2Pms0zQ13hIWTI925Ctc7/rS2mm1zpu3IUGRBHiX7hooVsrPuW9LQZTkULbJo
+7+CR007PalDWLbj+SKkProUBadxxox1WOhxVDX1QrCLOjxFPF8QnLGP7LRdYMqOe
+uEDObIKTNmk0Z8qq2uJubnxPvnMAEQEAAYkBHwQYAQgACQUCVzLRfQIbDAAKCRBO
+vLZwAkyorREAB/9c4dz/egis8m9cexeNtQ2OGrqoAt2zvJm1ke1T4j23xOa/8DiW
+la/DRaQQVQvb9r3KljKqiRFZGtU60rowgep+iLoYdlXoLDbq5nUWUYFjvf13qccE
+iZMbWuCn17npLYSrLd1ijYmgVGB8mPwHCLQZaXwp48uqkVHfjLJszKwBv/UAJfLO
+mQiYh549ZNFpYcjaShJ76tArr0SfS9mc3+RMR3jwAAg8wqf0DVIhzo7rBdbO1dZi
+9ZTQdQwnIwQao1SuWPtrRq/SWe/1XKRHBs59ZNgR1k3+FfxA5TZn5aNp8bEmHi5U
+y+J78lVsI2li7FH0OmdpnCqF7RnZ1OMbkwQQ
+=VM68
 -----END PGP PUBLIC KEY BLOCK-----
diff --git a/test/lisp/emacs-lisp/package-resources/key.sec b/test/lisp/emacs-lisp/package-resources/key.sec
index d21e6ae9a452ff9b7942e2a3310f0d43eb80527b..5021d12dc8e0cb3a6b52e6f6fda1dc99b4228bb8 100644
GIT binary patch
literal 2573
zcmV+o3i9=p1I7ebGSPhj2msR(nu^9h<~|urMdXu9gzfpA#UW^YC-idn6PJ22S~>sp
zAg(F0@HBpI8zGQ2up3arvWUCt6I4lMG=v7?tW_Lx>cI?t{9VzL=T4JBWs~gY1wE%1
z?g|Qd>npPkjl+yJ2dTG)B!QQA82JGdAS>k))2kf1!hYD-^Fz+z!&3R|)LKy+6j<i`
zsDc|cVd%P7TZU@WV8($hHDf2*KC~eGsht0M*1?Kr!v~S%w5j10Ex5=%pXvZqm`Vbm
z;AVp3=8-ShfCYzZpvw(g{F-4j=VG0?w?+HI=Z5v-nGeWpiPYm0#zhnHC(<dxD`g>*
zHvo@!K)*Bzqg7SeE%^Wu0RRF12Ll31&;hve+R`ZQ|C=VXs`1`Q;Rx(XEKkD`#i??(
zY`jn60G4)BCRl-=cu)Kr^dVbz2~6fZamON|x@yNTKz~BJc>i#B>?L7xFRt4oI_8Zz
zlo$X;L%g{f3i&4<A31ckbJ@g;TWrV+M=S7nrHsf=(&1`L;2@Jao9M4uUGZy?IjcC#
z6b;LFgqM=2oA5|3@gFPqOz2R*i5uc@@St7+!-0!?N>VMm4#!Kv9%`USaKH*kSjqJU
zH()})nR{bd8$JTp^Q%PJr68N#i^NL2OerwYbQhoVm{0RiLOxt#C6?-!{g>^o<yUkt
zPW$OMT?Z@4$Ppkl+1{56aJ=Xe8sz}&wXyK4+o!RQkxqAddoDJVVNd7HJTI~@;Am|b
zR&%6_d;gLbPsV9Tej8tPvJ2E?ih09z_7t@=N*=Pos}E^#T3H{PDD~k|>BYkHvsbS#
zgnU#i7ZVHPm8U9Jh#(jswdCw24pFOU181z3^Iws8y;uHc0lg&gv!pg96=>OgDOAgK
zIL`bgltQ!Ss*O~B-J8Ksh;!)q+;Hit{$pOt{fe1nC2kqu@?V1P7uz9_#qBM<Um3S}
zYX_Ka>p-0da{v_onO1N1ZE{Jm-WF529~ux?7Gy;?gN$vb>IS?pU;+q8{JuD0K%cE^
zkYHA8;oh+6BGH?S?V_L#02<)KmE&3zuGubL<KGS=<x@2#ewWEAwGU1N(i#O4d%J+3
zz_7ol=!psb1(vTw5mKCN7weX;Ud(XrhjLcLGAx^f$noG?YXg%4gT?G03~ECLs{O*i
zdz)y28FBv-5<+I(k{Z%mO?c-}&u?c}FuQ5uU};1JdFYzuKe?*HC3;KLxPqALCi^1$
zo)EA-#rEgj|3xr@JIiI6p6UqXFld`JtgjKWD|MxwoT0NFsjs~xv>i$=QZ67!VPk7$
zav(fva%ezhcwudDY-KKEZ*4w_0XGB_0SEvg1p-$x(R~6N0|g5S2nPZN6$l9m3jzcd
z0s{d89svRufB*^!5Kg?dZ~{!It-Rj|0E|BTew8y^kKrmh9<!Wa0$@oVX{_albD1*0
zl7_X}oZLL;h+Jmnep=WBSEi<jtL|dc6e4f$5))qcUcox?vq5`9u(Kpf1#0;ks5^rA
zhm+5|7Hq9?!_p$kH+^<ktZw)$e8%^4dU4u%&GCu95zW5kVA)6Kl7C8>hrw}C>tf0-
zy!o5Ix}ynLD-XuhBEBYQ3q_&KoW;YQ&z9L55u!a({v(gV^W^CTwU)Ak*Q2#fS+1SD
zVL^g6xA9}f%E&~;yFW<E0+4YhQN?<>Oo(R7M7j&?XaFNFO-oeq?D{*7md#0ME3q58
zD!Vgb_SAr9W<(oC?$JJJknR&r&Yc6s1XnWAeE|pn)7Cifq)3`31l|EM!=^$#QHX=g
z6eLplb5Vx76o_^<y(`kXFeG|w;LZIRU%;3g$SB$rhxO=O2}WzkKu2+q^OF>}6YuMj
zrWuAI8m(tj;O|b0i}`Zhw%`)rhguc~2EWRXh!@A3wUo<|XSj|=fQ+LDrVI=IcSl5(
z&mi9+-Im+-KQH7iSk%c8K9@E=Yc$ObcZ7wLBi*?Q*E{~Swwi5ortQd3WI-M!`-X}Y
zva{}6($Hl&6fLr7@8FTsPS0vk)-AaHNU0C5g#l^YaibkpIviCEeNe0-&W{mK7sMwl
zWBV-^STduYxIoTqf|E9BG-t}H+Tw0*d{4e}01*KI0saRA0>{~Nl3c7ox9$TMDFLq!
zmQI&z_U#RTep)=02&><__EP4A2K_!D%%m3YjoR>-_UjyrS#F%xm}PdEuD{?DqyC`X
z))fxKjx%>0=-D-?G&8)LvvVf-2}SmLHyv`s38O8yu#EDUQ+$d+&H()KJ%X{u*r4Q&
zam(P#gMHjV{?h*j<Cmx-1iB6o@7yF0#mhClY+YjQ+?b$b=86k7+JUGPq7K4nx!%7L
zAIkq{EcyTlXx)CLZduLo2-K*wZ1$JM16G)pgq4rtzHYZVBqYLe`G5qC55X+VGZ6Ln
zL%}C0+?8D%52HSB)T$s{<V99GiMXW~Qc-*ffu`|1rv;$kM1KE%DM@%a#CN#M@N$o)
z_ZFc$aNw0Q3hsEyC_L)vC#Whu?HrQ_@^@iJFASd<HVIIqAQ=&k-_Qwwr58_X)U5<w
zXC1r%85a~PJ?F*%m$^Cj%AgGUM?IkI_F;elH^p98j=_HA&2Du;&ee9kdK{UVYzM4S
z8m8j}AQLdTq3b4Gd#a%v>AKz2%mNq~WItrHAkVO1v#SC)|21`|5OspZUNa<KzQK|;
z6)iC2m~=+IHbdCD#L^9lP?#=z*EhajO$@Og9|Q^tV8o?4h`8qEqU`Na8h(Q6#uj`x
z_!{lLNXs<8MzKV)zr$OGSkQws9!*2eYIWnm4x(8>eT>83xXNU|%+w%%8tvTLn_Wvl
zxM^m{in!{rdSfvM0YIZ~0;KLy2Fhbok8|=VlTZ#y=7Mqzi2ePp$?^5?SX{PdM{7-I
zZXG+X;`c?4G$wo_;Y@HU)wL+qSNW4k^y9<q^%(4esCEdnX3}T6cw#uZXCZYi{IeS4
z8}&0!G*v2rE2n*%25x8_Yibg6tQu=AMMY7JhOVw}O^Y7VHvW(gZK-Z!HGhKC%$m<%
zF;<BI9|RZy2mlEM0#`E8eF7T{0162ZPQ12o0!*l_5da7OT;bgRdI+rYZ(Ms9jkOJq
z8oH<g-Lt%zwUO;p;yt&-=D+YbmX)u=MWhf_3)}X+%9b*!i4j>E)jHC;FoEiRh`Jbd
zRp=}>>gIJ8QGsK<{dcLy1c{RyTHvSGx#=y0t1aDPjftRCV0@VT2MDwoX?!W;%c_x4
z-;A<s%&Y;w^#CRE&Y1|9hn_uT(P?4G+Da08>d-5%M4wC9oZsY3M|ki62t2~4^bJxW
z&W`H^*3Q+|V)c~JbqpsX1RA4Mu2}nPMz7LY@AX`yM+VM)WY`hbP5u@9K;<@P<)dlw
ju_hiaRLkOf@>OgjX=3bA^g3s0oGOLw8QIk18<PYO&CT4H

literal 1888
zcmaJ?N3OJJ65Ml&x11LG8}C}0#fSG4-s~uf!c%w(PCswv0FzRzQYnQL5gC8}{0=WE
zn*H-o9zPFHi$Ea#7*hJfn~q@ocl`&7%4Kan<3B&jxIdJi1N)o&8zX=IrPv{2Hsk*N
z3xayrfq7ZCdA8yOAiSU3bfk;LMwn`$hTLmsz7>+bSL%E^>a-vkH!MPozQ30XV&nzk
zCQP`Iz1m!GBL>%-1s_(W$1HUu-Kss;Lv6Q+IyT*@%*j*xyz2>epV`10Bj|-2dNE8_
zvA>s}TixD4H(*tQFyiZRyT(v8OzhlugA*KeD*LmXb@NDbshFduhDx6&EJ1{Dd#!yT
zB@(=eK=)#*+=Akj*|Zt$TFPv;pw7LU1?P1Yj*ZlBGf3#d6vkUz=K+E2udtsZm(LF_
zS3Kpd?Mw=5Z{iNF;aC#s5!xK<NSerE<kK-;Uv!Ux%xiq>dF+9rLF{hjNH;0JyxC>e
zXsmi>5ruj3CuF7o-|jO-059<Xs4zG!!4QP$VJBU(N0rPJH&pW1epH&noZ29!iu2Ov
z75M%gku$nuc`+(m|E-vKstr-*yiuTFEX%O0Soq?=cOCj>1Kz5ycD4w4sNBG2P$jw@
zY&4{O!Ow4Rd^Ux2VV2)=PdwF^F}ZRFyD^t*qe=)6-`R0sa7-hgS^f!?ud#djlc46-
zDhYoTC8m2lH)nR>%_*nJi%Q4IgprX6qFE5Sfm*y8G)tmQS-cOvp;K^KykB!D=onPh
zHG4zdchYBX%Gwgl&cDARj;lAEXJOCZnuGMC5V)C)7mvg3S`}0c?wu8sO#M7RNYtHP
z>-%5ymwNZ5yW>^-+KMyF1Zl7CHjfJGr1=1IK<YzBF)_ZD^D)^4aNshG7M&6?$D_M?
z4Y#IyO!(xglqDv*s1!{JXFz^JC(WOqm>2o>^D(yf{y<G*US6B7;JR7TSIyV)@t)NO
z--jXQuYk#^JP^zeD>ri`X^?SUG<sWK1h*QJ{n%KJ4B!)CRuG%D6NRzfYpTVJk=bHS
zo}dIqn}wMmdte4LiB3>Hsm3XJ`FN+yoAsRLK;#_JVXhF?w__UbRtD|FM`84Fo&=|^
zmyaArr&uZ3Rk=t*2EB0l<F)3W^jOXA+fnMJ%8b?FyN}(Nq=7}m5u*2+gw88i3o*MV
z60xh^S&PyJ0kyl&k}pMOg~2Ad-I3ZG=)lcXt_@boG462BYDJgfYC}ziJxu3gcnka7
zF&k4Ja|oTI&!MauFMD#gdXEX99(N&U<x$K-cpD_6O{aJ-`!7p%pMJ7p#tf*+k+X#*
zEv<!&neaBe4(Zutgx7_Y6GdRD(G6G4rDTr;RkL8uHoES-+54$#B**m348qaKcwWt*
zY6ny6bNokLy`($wp%T|MgS*-}TjZ+`EJKg7-?*h9qPg}A5%Si#5?4b<2bqQ~zaPmh
z{f4Z<U-*jL=6G~z;&(2F8e~abKNEh<e5!roDBQRb$@3NuD$ww*?g#E$gFwBCe6{KV
zC`3@DkpMJZs8&g?G!eL3`HQ3izz{qE??XWmye6V?TILb56SY?Z$#zF5&f<AZZ208%
zK7Fj-UInW%K{2}O@zQR~o6WwwbUR4FLs?Ye*awaZm$1SR5~|YO&<iCJ7N)W_n&aD6
z+Td}#Y>TSpBy<fpDi(IYxsVYk?j^avUUUq>6ZRWOVh24_mX*_n68utq8z*Pn>T;63
zX`w|T_~HI`t?91wxC+>Gb^3>6<b@$_;OS6)TQmj~4>n<*K4+tCPm~W8PSdUhjZmpn
zJ&2~Pu{W7p4?*p$06bQTk??t1Hq|K0Eb!<7%A?jWF`r_!;t&Kwb6dyhb914!gw(~h
z@4)=J8dlZmqW(=mF&0fqQ}}*e9tjF1gJvamkXUABDnWhWkwO2a<jjEn*Nu`b{=eq@
E4<mzKkpKVy

diff --git a/test/lisp/emacs-lisp/package-resources/signed/archive-contents b/test/lisp/emacs-lisp/package-resources/signed/archive-contents
index 2a773ec..83eb870 100644
--- a/test/lisp/emacs-lisp/package-resources/signed/archive-contents
+++ b/test/lisp/emacs-lisp/package-resources/signed/archive-contents
@@ -4,4 +4,7 @@
 	nil "A package with good signature" single])
  (signed-bad .
        [(1 0)
-	nil "A package with bad signature" single]))
+	nil "A package with bad signature" single])
+ (signed-empty .
+       [(1 0)
+	nil "A package with an empty signature" single]))
diff --git a/test/lisp/emacs-lisp/package-resources/signed/archive-contents.sig b/test/lisp/emacs-lisp/package-resources/signed/archive-contents.sig
index 658edd3f60e620839fb4f93ee96fd9cbe1708c22..7801b5dc1a7ca413a7317240e2439756f23ddcde 100644
GIT binary patch
literal 287
zcmV+)0pR|L0UQJX0SEvF1p-$x*4F?E2@p=awr~PWsI4Q-2mripS%*>p4d8}}izsSx
z09hHJ8U(`X!<u=WCa&>~w(~v{-;9Jf^diL{>MHLL&m(vo8uezSDj*^?)^z0rL!`l7
z!ts*@d3SY-OKX*ToNgs;A=NouzwX%p5a&DAZ;~kqxC+<hjt2g5iS|Rgj=go-v%q#J
z*cMGB(A>j>rD6esvPu#GFs-L0ERk}gW;E5yndyI=BepVj(3S*b23u#NFqM>SJEndT
zSC@I%HN%GL=WC=5?yQOo$I&t&d_=|MJ`$}44Xo))*`e+%N9Rd%J4GU^TcdaEwI<65
l0@H3W3R8f$=A<=vpcl$_f|Usoys+$ECcO*%YZ1}36Net3fp7o-

literal 287
zcmV+)0pR|L0UQJX0RjL91p-n{5<CD32@tlGK!ynzmFoyI2mq3XTz|>YK#4VQKS=3Y
z4{6H*y$2N7$F>D9q<6}8W@)g4IY*tJ!pbhJKg4wt^n;h+JbBA;u0>elMy`2jx*VU|
zlh?jrV-3vBGP*mmNL_W?AnwVn6@Q@p*wV7I(C;5R08N&Z^~?UH#j<8F5FJwK_IikR
zqO6N|z0JA$Na2JaN;v+>WxkjrGTxewi33ub)Z`?zmIs&s$<Kt5PpRHDa{~FWptFnc
zq%tNyN$>-6F-;flCj9#nf{*7e-CNTm^_t3+^EM%2i`||6aGGRI7U7YNOYtV}Ps|)h
ltRz>cs){>Y*0(}mOOk(tIMaz=$yPRH;b7jH|DihEO^(>ki^l)}

diff --git a/test/lisp/emacs-lisp/package-resources/signed/signed-bad-1.0.el.sig b/test/lisp/emacs-lisp/package-resources/signed/signed-bad-1.0.el.sig
index 747918794cab396b0b16c3d02530f45329593e8a..0803d129514565c17b1d490a6751deae9ba98c0b 100644
GIT binary patch
literal 287
zcmV+)0pR|L0UQJX0SEvF1p-$x*5UvP2@p=awr~PWsIAwA2mKd=eJs30OK~ba=WYKb
zA40)hmr4P&3H8xkwD|?aFZUrO$6zf&Y;?;dXS?GW@Qr&Ulm|{1%i^fPV}P{{FlzPf
z%TQ;SS#A}>Ad6|eLj%kG`<kcfCox=|Bck64(7FQ&Huxz_f<|7RyA)^S7AQ1Ot0)YC
z&oKsm7WnI5QLGEcn=3o7Rn*(rj>><vU+@vh)I(u>p;S_5NOs6LBiBp9MOq(R#})Ni
zIV|E6*o6fWBswI+f{rhacb3=-rnxkOf`(CtrUhireeq}QeC7Boom!q)-tH3y^E%dy
ld>L-}SVw1AD7Q+ux571(vR=snM^Ignd(QDM;n~T-E;^4-g8Kjf

literal 287
zcmV+)0pR|L0UQJX0RjL91p-n{5<mb72@tlGK!ynzmFoas2mKk=hK7Z)!69lH;@4h&
zK+B`umUCTZm`|Tr*O1x`vixeQK?}Ao98uq4ElHZw*MKb7b>t*q3BFN4;I!%1v33I9
z2p^^!^J_nB)#mYv#@QdKcy&CjbuXo!;>HueViXfL?7<R06#NK$D?UUGLB=-eG3M{f
z6e|Ix+uw@od<<5Tyt^%suI)3YCvbXn;3+@_eGih3^=oe4oz(`07!BFT#a@+&Na}oQ
z^~2oK0{@UfP>zjQ*<5?Ehl0XP=fm8^A1oYry!UqNH^J<2@A-$ubXim+sOzut6I3Tw
lH2e2uU;QtGzo)(RqQbFg5?sfObwr5XeEV{EVAln2=dFOHjDY|E

diff --git a/test/lisp/emacs-lisp/package-resources/signed/signed-empty-1.0.el b/test/lisp/emacs-lisp/package-resources/signed/signed-empty-1.0.el
new file mode 100644
index 0000000..f23d144
--- /dev/null
+++ b/test/lisp/emacs-lisp/package-resources/signed/signed-empty-1.0.el
@@ -0,0 +1,33 @@
+;;; signed-empty.el --- A single-file package with an empty signature
+
+;; Author: J. R. Hacker <jrh@example.com>
+;; Version: 1.0
+;; Keywords: frobnicate
+;; URL: http://doodles.au
+
+;;; Commentary:
+
+;; This package provides a minor mode to frobnicate and/or bifurcate
+;; any flanges you desire. To activate it, type "C-M-r M-3 butterfly"
+;; and all your dreams will come true.
+
+;;; Code:
+
+(defgroup signed-empty nil "Simply a file"
+  :group 'lisp)
+
+(defcustom signed-empty-super-sunday t
+  "How great is this?"
+  :type 'boolean
+  :group 'signed-empty)
+
+(defvar signed-empty-sudo-sandwich nil
+  "Make a sandwich?")
+
+;;;###autoload
+(define-minor-mode signed-empty-mode
+  "It does good things to stuff")
+
+(provide 'signed-empty)
+
+;;; signed-empty.el ends here
diff --git a/test/lisp/emacs-lisp/package-resources/signed/signed-empty-1.0.el.sig b/test/lisp/emacs-lisp/package-resources/signed/signed-empty-1.0.el.sig
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
diff --git a/test/lisp/emacs-lisp/package-resources/signed/signed-good-1.0.el.sig b/test/lisp/emacs-lisp/package-resources/signed/signed-good-1.0.el.sig
index 747918794cab396b0b16c3d02530f45329593e8a..0803d129514565c17b1d490a6751deae9ba98c0b 100644
GIT binary patch
literal 287
zcmV+)0pR|L0UQJX0SEvF1p-$x*5UvP2@p=awr~PWsIAwA2mKd=eJs30OK~ba=WYKb
zA40)hmr4P&3H8xkwD|?aFZUrO$6zf&Y;?;dXS?GW@Qr&Ulm|{1%i^fPV}P{{FlzPf
z%TQ;SS#A}>Ad6|eLj%kG`<kcfCox=|Bck64(7FQ&Huxz_f<|7RyA)^S7AQ1Ot0)YC
z&oKsm7WnI5QLGEcn=3o7Rn*(rj>><vU+@vh)I(u>p;S_5NOs6LBiBp9MOq(R#})Ni
zIV|E6*o6fWBswI+f{rhacb3=-rnxkOf`(CtrUhireeq}QeC7Boom!q)-tH3y^E%dy
ld>L-}SVw1AD7Q+ux571(vR=snM^Ignd(QDM;n~T-E;^4-g8Kjf

literal 287
zcmV+)0pR|L0UQJX0RjL91p-n{5<mb72@tlGK!ynzmFoas2mKk=hK7Z)!69lH;@4h&
zK+B`umUCTZm`|Tr*O1x`vixeQK?}Ao98uq4ElHZw*MKb7b>t*q3BFN4;I!%1v33I9
z2p^^!^J_nB)#mYv#@QdKcy&CjbuXo!;>HueViXfL?7<R06#NK$D?UUGLB=-eG3M{f
z6e|Ix+uw@od<<5Tyt^%suI)3YCvbXn;3+@_eGih3^=oe4oz(`07!BFT#a@+&Na}oQ
z^~2oK0{@UfP>zjQ*<5?Ehl0XP=fm8^A1oYry!UqNH^J<2@A-$ubXim+sOzut6I3Tw
lH2e2uU;QtGzo)(RqQbFg5?sfObwr5XeEV{EVAln2=dFOHjDY|E

diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el
index 70e129c..67da2e1 100644
--- a/test/lisp/emacs-lisp/package-tests.el
+++ b/test/lisp/emacs-lisp/package-tests.el
@@ -459,15 +459,6 @@ package-test-desc-version-string
 
 (ert-deftest package-test-signed ()
   "Test verifying package signature."
-  (skip-unless (ignore-errors
-		 (let ((homedir (make-temp-file "package-test" t)))
-		   (unwind-protect
-		       (let ((process-environment
-			      (cons (format "HOME=%s" homedir)
-				    process-environment)))
-			 (epg-check-configuration (epg-configuration))
-			 (epg-find-configuration 'OpenPGP))
-		     (delete-directory homedir t)))))
   (let* ((keyring (expand-file-name "key.pub" package-test-data-dir))
 	 (package-test-data-dir
 	   (expand-file-name "package-resources/signed" package-test-file-dir)))
@@ -476,6 +467,7 @@ package-test-desc-version-string
       (package-import-keyring keyring)
       (package-refresh-contents)
       (should (package-install 'signed-good))
+      (should-error (package-install 'signed-empty))
       (should-error (package-install 'signed-bad))
       ;; Check if the installed package status is updated.
       (let ((buf (package-list-packages)))

bug#23513: package.el treats empty signatures as correct

[Paul Eggert]

Thanks for the bug report and fix! The code fix is so simple that copyright
papers are not needed, so I installed it and will boldly mark this bug as done.

The test case is a bit much to accept without copyright assignment; is that
something you and your employer would be willing to do? If so please let me know
and I'll start the ball rolling on that.

I've never messed with those test-case signatures either but if I had to guess
the passphrase I would guess "test0123456789", the string used in
test/lisp/epg-tests.el's epg-tests-passphrase-callback function.

bug#23513: package.el treats empty signatures as correct

[Dmitry Gutov]

On 05/11/2016 12:39 PM, L. Dixon wrote:

> I also deleted the skip-unless clause in the package-test-signed,
> since the test runs normally without it. I may be misunderstanding
> something here, but I'm worried that skipping this test will mask
> similar issues or regressions.

That's definitely a cause for concern. Glenn, does Hydra lack the 
necessary libraries to support the package signature check?

Why do we skip this test there? It seems important.

bug#23513: package.el treats empty signatures as correct

[Glenn Morris]

Dmitry Gutov wrote:

> On 05/11/2016 12:39 PM, L. Dixon wrote:
>
>> I also deleted the skip-unless clause in the package-test-signed,
>> since the test runs normally without it. I may be misunderstanding
>> something here, but I'm worried that skipping this test will mask
>> similar issues or regressions.

No, that stuff is there for a reason. Please don't delete it just
becauses it's not needed on your system.

> That's definitely a cause for concern. Glenn, does Hydra lack the
> necessary libraries to support the package signature check?

Hydra's "gnupg" package is from the 2.0 series, and only provides a
"gpg2" executable. epg-config--program-alist requires something from the
2.1 series. So (epg-find-configuration 'OpenPGP) fails with "no usable
configuration".

I have added "gnupg1" to the requirements for the coverage build in an
effort to get a "gpg" executable. We'll see if this helps.

(It would be easier to see if this worked if the coverage job wasn't
currently failing, as it has been for two weeks, due to network-stream
changes that cause a test failure - bug#23508. This is a repeated pattern
that makes me think people don't actually pay much attention to the
coverage job.)

bug#23513: package.el treats empty signatures as correct

[Dmitry Gutov]

On 05/16/2016 09:39 PM, Glenn Morris wrote:

>> That's definitely a cause for concern. Glenn, does Hydra lack the
>> necessary libraries to support the package signature check?
>
> Hydra's "gnupg" package is from the 2.0 series, and only provides a
> "gpg2" executable. epg-config--program-alist requires something from the
> 2.1 series. So (epg-find-configuration 'OpenPGP) fails with "no usable
> configuration".
>
> I have added "gnupg1" to the requirements for the coverage build in an
> effort to get a "gpg" executable. We'll see if this helps.

Thanks.

Ideally, we'd have something like (skip-unless (or (getenv "HYDRA") 
(ignore-errors ...)), to make sure the tests like that are _not_ skipped 
on the CI.

Individual contributors may not have gpg installed (although there's a 
case to be made that the package tests should just fail for them), but 
the CI is our last "line of defense", especially for important tests.

> (It would be easier to see if this worked if the coverage job wasn't
> currently failing, as it has been for two weeks, due to network-stream
> changes that cause a test failure - bug#23508. This is a repeated pattern
> that makes me think people don't actually pay much attention to the
> coverage job.)

I've noticed this failure when running tests locally, but it's far from 
my area of expertise.

I think using a separate mailing list for the build status notifications 
might be a mistake. I'm not subscribed to it (not sure why; maybe I've 
missed the announcement), and apparently not many other people are.

There's not a lot traffic there, why not just send it to emacs-devel?