From 721547a3022f891962dd1ff7873c3066818d94a4 Mon Sep 17 00:00:00 2001 From: Matthias Valvekens Date: Wed, 17 Jul 2024 00:25:01 +0200 Subject: [PATCH] Align Ed448 CMS structure with RFC 8419 --- pyhanko/sign/attributes.py | 11 ++++++- pyhanko/sign/general.py | 5 ++-- pyhanko/sign/signers/pdf_cms.py | 18 +++++++---- pyhanko/sign/validation/utils.py | 18 +++++++++-- .../data/pdf/ed448-disallowed-hash.pdf | Bin 0 -> 10508 bytes .../data/pdf/ed448-shake256-nolen.pdf | Bin 0 -> 10490 bytes pyhanko_tests/test_cms.py | 28 ++++++++++++++++++ pyhanko_tests/test_signing.py | 2 +- 8 files changed, 70 insertions(+), 12 deletions(-) create mode 100644 pyhanko_tests/data/pdf/ed448-disallowed-hash.pdf create mode 100644 pyhanko_tests/data/pdf/ed448-shake256-nolen.pdf diff --git a/pyhanko/sign/attributes.py b/pyhanko/sign/attributes.py index 8f1c6070..5cd04098 100644 --- a/pyhanko/sign/attributes.py +++ b/pyhanko/sign/attributes.py @@ -130,10 +130,19 @@ def __init__( async def build_attr_value( self, dry_run=False ) -> cms.CMSAlgorithmProtection: + digest_algorithm_args = {'algorithm': self.digest_algo} + if self.digest_algo == 'shake256': + # RFC 8419 requirement + mech = self.signature_algo + if mech['algorithm'].native == 'ed448': + digest_algorithm_args = { + 'algorithm': 'shake256_len', + 'parameters': core.Integer(512), + } return cms.CMSAlgorithmProtection( { 'digest_algorithm': algos.DigestAlgorithm( - {'algorithm': self.digest_algo} + digest_algorithm_args ), 'signature_algorithm': self.signature_algo, } diff --git a/pyhanko/sign/general.py b/pyhanko/sign/general.py index 4b7213e8..f00b447c 100644 --- a/pyhanko/sign/general.py +++ b/pyhanko/sign/general.py @@ -348,8 +348,9 @@ class UnacceptableSignerError(SigningError): def get_pyca_cryptography_hash(algorithm) -> Union[hashes.HashAlgorithm]: - if algorithm.lower() == 'shake256': - # force the output length to 64 bytes = 512 bits + if algorithm.lower() in ('shake256', 'shake256_len'): + # force the output length to 64 bytes = 512 bits. We don't + # support any other lengths because those can't be valid in CMS return hashes.SHAKE256(digest_size=64) else: return getattr(hashes, algorithm.upper())() diff --git a/pyhanko/sign/signers/pdf_cms.py b/pyhanko/sign/signers/pdf_cms.py index 047d404e..61ee8bb3 100644 --- a/pyhanko/sign/signers/pdf_cms.py +++ b/pyhanko/sign/signers/pdf_cms.py @@ -589,9 +589,17 @@ def signer_info(self, digest_algorithm: str, signed_attrs, signature): :return: An :class:`.asn1crypto.cms.SignerInfo` object. """ - digest_algorithm_obj = algos.DigestAlgorithm( - {'algorithm': digest_algorithm} - ) + + digest_algorithm_args = {'algorithm': digest_algorithm} + if digest_algorithm == 'shake256': + # RFC 8419 requirement + mech = self.get_signature_mechanism_for_digest('shake256') + if mech['algorithm'].native == 'ed448': + digest_algorithm_args = { + 'algorithm': 'shake256_len', + 'parameters': core.Integer(512), + } + digest_algorithm_obj = algos.DigestAlgorithm(digest_algorithm_args) signing_cert = self.signing_cert if signing_cert is None: @@ -635,10 +643,8 @@ def _package_signature( encap_content_info, ) -> cms.ContentInfo: encap_content_info = encap_content_info or {'content_type': 'data'} - digest_algorithm_obj = algos.DigestAlgorithm( - {'algorithm': digest_algorithm} - ) sig_info = self.signer_info(digest_algorithm, signed_attrs, signature) + digest_algorithm_obj = sig_info['digest_algorithm'] if unsigned_attrs is not None: sig_info['unsigned_attrs'] = unsigned_attrs diff --git a/pyhanko/sign/validation/utils.py b/pyhanko/sign/validation/utils.py index 259dcccb..126c9b1f 100644 --- a/pyhanko/sign/validation/utils.py +++ b/pyhanko/sign/validation/utils.py @@ -2,7 +2,7 @@ from datetime import datetime from typing import Optional -from asn1crypto import algos, cms, keys, x509 +from asn1crypto import algos, cms, core, keys, x509 from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import padding from cryptography.hazmat.primitives.asymmetric.dsa import DSAPublicKey @@ -33,6 +33,21 @@ def _ensure_digest_match( signature_algo: algos.SignedDigestAlgorithm, message_digest_algo: algos.DigestAlgorithm, ) -> AlgorithmUsageConstraint: + if signature_algo['algorithm'].native == 'ed448': + # be a bit more tolerant here, also don't check parameters because + # we only support one length anyway + algo = message_digest_algo['algorithm'].native + if algo in ('shake256', 'shake256_len'): + return AlgorithmUsageConstraint(allowed=True) + else: + return AlgorithmUsageConstraint( + allowed=False, + failure_reason=( + f"Digest algorithm {algo} " + f"does not match value implied by signature algorithm ed448" + ), + ) + try: sig_hash_algo_obj = algos.DigestAlgorithm( {'algorithm': signature_algo.hash_algo} @@ -128,7 +143,6 @@ def signature_algorithm_allowed( DEFAULT_WEAK_HASH_ALGORITHMS = frozenset({'sha1', 'md5', 'md2'}) - DEFAULT_ALGORITHM_USAGE_POLICY = CMSAlgorithmUsagePolicy.lift_policy( DisallowWeakAlgorithmsPolicy(DEFAULT_WEAK_HASH_ALGORITHMS) ) diff --git a/pyhanko_tests/data/pdf/ed448-disallowed-hash.pdf b/pyhanko_tests/data/pdf/ed448-disallowed-hash.pdf new file mode 100644 index 0000000000000000000000000000000000000000..c6e5acf89084cbfc7e0c561db7a92e9794d37358 GIT binary patch literal 10508 zcmeHN+io1k5q;mU7%&hZBM_(W7ebWLSJl&3uB zA%OEi`GuSs?kq2&>m)HL_+H|^oO)2?aLcs)K|9H9+A?c?!Lw`v6B4$$)8!Kj;0smF+#u6ON)Hyyv3 zO;^oRNgbbUn$qI+eBJQP-J@@en_ zU$62(w_3bjPP)}bsDHP*n6HO>lA`ym8&&B3{O7_7*X1_??UUEf*S(p@SReFu zj(S7g*-5v$cd$4a&o56+L*X4aBVN7RwzmmG;MG=*A`?RTl3A!}YnJw5H$PgxY_ukk z)q2^DPaqx-`x)?(4R_7qi~f!*eK_qe?|#)CA1|8k7t7=67tJC0Kslt<-Hb4-^Vza{ zF=CA#vF%S|w3fPg(QMl=L;tjyU$j~KK^wOp$|-$GZoM4Oj=Lpv!Admk!D0b_*}Oj) zs(mth-$9q&8mskqx$f1|Mvv~?Dc{zuL*KtYy5`PTJh=^%<)SW@CynVxZ@=r7tJz}S zv|^Wy+HwZd)BOs1jM}fCeEa9-X@7YhBDs6h9WPG1<%Qs=U-yp;^N{2lDH!9U_Kyb- zo4YU9>(kW_%!(hFw)FS*?OS}9*=j)_i{%kD;tl81@p4MnN;|9Q*~|6G@fVkLJm}uc zdfo5Rc!CM>#puC<&!VmPpS0DVpzSlLJZc|x>+uxu)A)W>EVOEi=-0>f^(g*G%wqoYf7gSt4fFW|q4~6d?=}Zbul*)q$e}1U`?rhY`BdH=~X+R=$dhc7e10W26p zV+<*x(H3%$)gP&7fH*p|s>&Nyf$YNnnXZpo}xzWXnu4S}^5JF%q%?vTiFI zS>jwP%p$EaBx51VEyB-`F@*6}S&x3CQ`z>Dn8RveDr6LTNJZ)}4(mxs;urV*-}xQ> z*8%fZ*i(O8;@9$~N?u(dCM6b8@vLnUFutqBa^RP-scP>^Hb#2*h0rP|Bc!!MITRDp1{9-hkK zxzSWn;J3XmTZv~^{W=?IxX4BuxGR30s7AuWQ!hLCaz(uGp{GF1ki_-XwrJof1}vAg zSPsv$&QS{Gbe7h~5Jc9{RBB_PX|`5nG$cfmH-_FsCgD?bSmv6QF(SmGSTxXy>zX7f zS27&OBV3GtCuNv1hvY-5STxDJ4Z32S?6nI%SK*Qs&|C7vBg>&^uS&2oX;!$eh)lt3 z#4p0#8ly4SWAq9pSHk(;dj*ygo(@d!y?Masn~|%sqT!H06IP;>DP%a1bYQ*7 zBWSWS(YXjXW5vQVb@VaAGir4#JIGR;ixQDHRC2x~E(#ZjYYS{~nnD|&%)3;1gm$>@ zkqhyCj$bz+mrY!YU_~I8OaV{`=X1hVuC(wRm}sI!@O%#B8nP#n@da|hy0Gmzj*-t} z<%bOTo4~ICTmkuz1t|$g0;pg?d*OAl8N7nk=m3{I#E_AObYwbdBV^XbMQ@U?HWKnM z?`0~%3!9R&K?Dy@0?q(*g73jq6b}ZO!6A)VA;AKyb*eazl*DrdxLZ+l!~>f$Z0tPT zN+Ep((2vZ7<<8Z>GJHqEZ4y$G5K3v!0d4TSs8dZ5LdXkANndgn=&L0mCgB%?_Z-I@ zCF!5TuU}kJ+%fRwz0kH;~2UkHFfW^gw`5|&Xj(`6>g8l@qnFPzLt$sZ(Bc|4KGXj3% zW&rr~|KVk{CmHe?_}Y!!47Aq3Gn^py$iYW6_HV##rW|)3R_?ha@Zg#ZJg!eopH*|> zloDjCUe+A;qW#=IH471nsA$J7%LNyr2Ejx=DA=B7&%a>N2DdPVzvmmfNh83}^ z0Jv9$1C(507bIenfho~p*{~1EDgkB+EMBag8NAeV^I zcsN^fTw+6BU5UbATOYwL1oreexC2uLBB~H_!oh==G2)!&_UG0Dw-&gyz^w&tEpTgr zTMOJ;;MM}4Yyp%L#CPlNU_3wSsKkhCV;Y>rss8r`g?oLtAFnQTcQQFYl&71lRoX}zah3`KZLxlvs zy7~dUp01Up%8j8afNCIyFX_)q0EY62OH}}Wdh~EK6ae9nnYwT4ckX|Ab~>KC>ekKk z?r1jO-}(1H{<+i4ru#eJ>qqR-Y3g3izIwmxp1l9V;pF|RiJ$I#`CxSaZ13#k^aN@* zXD7$=)!y0u&Y0eG57*S(?lir~`qlo<>ASDS^H+<8?Fzli@v?9D4u)7xU+f*^x)}g> z`#V(bve&jKa@l<=cTwHaa_@c1L@UHSNZeb!o3F=b_vWkL4I|`kHCfJ1*Ho2ruFC19 zk>;!2Uaj59;-tlcmhVbZt=IXJrySn(?acoqFnp_W7`UVnpX367$Xq1`lUXva5T z7wsEaB~fiR$Zp!S(%0K?l7Lc}L2fAQqNGRP&lGdI2h;Q6WV>r>3S(Tgc>Qiu-X=7Gq0JjbCWQ2sS*U1ZmiBQsIbFPLv?h`H zV%81MAs!F=9x%v;yXNquzavW@j{3{{Uv+0^)8_l>>}>Q!b4Wf=4rz8*Ev)DHa@L&; zSfdAQ^V1lurEX4|O&NOVpEi@LGHX95vVSBn7;M)yKXigPbW<) zcG;jUmoPou&!NYl{rcIre{P=lm&*{z-J9-gdeP0U1PA@Rf25hmB;QEE7$3BMJb2vP zf4Nv(%zt23{J^xOzqfDS;>(QZQ~H|DPN@(hoQ#IE5nU_otfFTx7w2bRZ0UH=y&3nq z->3Ej6XJ`(qeq`aTk$_>tAByEFQD?EebOz4BfwAN`&qg2_%#k(O=}MV*TcWv1b#pG zJ_e}*BH*fCAJ;dd_#-ik{+GWugRu>h$rPdayn*jF2Tia2DqzT=C^q|^$K4sB;6@Do zds@$ir}IWq)2k}f$fn96Ir%BPx4DmRj+tKiT;Jv3O7ri>qtkAI(KTFRo({U>1v;Vt zr_YN)`?Tr7chG*odfp#3U-7wM_!XcePj!>@qf0?On;nNDsddTIHmi=fzndTTMQC&ytQ# zsAvikI(QEc@WqQPqDq(^9{_|}T1NEUk|rh^(Qh)W$;7Y^};DNQfqH42?u4;Zt;&=9-l;BE+ItG|-9bnj|S#G91Sv zT#SGxWtcIC zHBj+a#^gRmuVHc}obSC?U^?OH!1Ugm*BE^}a#dC|91>{4N|Z8%3(8f3OE>#|(9j<%iLVPdr z>o(-FiE9z82;`C}01DxJPT0zo7M=qWO|%G}Pk~(P)f2dLg6nhh@wP2^L_jQ^k2KNjz78yA?%8Jg_Ol#?Hg76qc_5 z`jMHi+_@T9hVMwYO~TS7gi_jbKpQ+S>QqyN5Y~mHq%S!O)YXy@lkf|{yTma^i}X+7 z*Jl~ky!6ZqYvt`|wR%d=Z^bfj%wHMH82Ae!e9dl&UuyUOR!pqg;J7r6ocQNWmhKtn zhJM$wpvKp^FV=AIRPfTXaE@EMulrxsrCd+Yd;${H& z^#9>yv?p1wXW(nMax>6c1J7`R*dqrY(b&5Ix0!O>d04sUmcWA>GVr*5Y5Js^6Q`6Q zTXkD=*o*da*EB@$0(Jv?{giwbz7G~ijCQ+!V!GhIJ)5t;}F6;L8AXS~fsQ9r!b%gEpZ+WzCIs342|ST@!c) zD0an$ii7b8Z;Le?QOXfzSk16v=nVdY(fLvC|6FAw2%D4$zp?))p-r>~bA;XkgJbYgr=@ z0hE`4h!$8Fs>0zB8Bv0C1uQ7g9067pjR@F=GaaULP#{pB<&I>_hqq>(DGS44H<618 zpr|rcsd07{1OhyR-3em@{JPT2dhl0m0T?)I5FttIFKDBHbCrk*^fh9y8wHo40KWhx zF%~X?7|Gxf{4qx6GY(W(C^@1{pbzFIM8-WpE;(>nA-I-k7(0)M*;m{br+89;GM;hN zQ{ao3I-L=bz^3eZ2B=lfR1t6?oXshyjCEULXB})rAvi++_TcW{odxbJaA$!#3*1@Y z&H{H9xU;~W1%9RlkUS9IExLo@QmsoQ<}^qa@yMc2QDUq2a+@ZjRr@!al?kiKyIvKt|hp}C(|*%&yJ>knWs5adH( zzxM+eJzXnHc^c~+0Lpk+zmi|R@LwlGY`ydU)04-8b-E9zI8|R|>pcAO@?v=Ws#`Qi z-RXF;zw_^Z{Bx%nkM?)I*H74!i`2awfAxOWJ$wI$!{hg_j{Ruo%SVHUmwT7z7w1sB zxja9c%=a$$cZM|5JzP_9yVLX{i&y(Q7w^6rPF_tLwkz~5$6(*^9W*f;o$MXtx@rJ- z`#Y4}ve&jqY1w@%cah7|V*B7*CR!mLK;pst-DEMmd@z~+Zrwue=Et+~#e#Bhma1HA zwKSRU_G;}OPtRLCX!))r)q0(OahL0{o?mQKz1G3y@B$sMzJrUsC)3gRH`5-Bt}*YIrn#z1ZK`Xv>tLv@Een!9xDr&9p2l;Z^iQX;~79cl{0WBREn-*5#&A>vFvF zx+-7EDv8{>wd|@)D}A#J2Txn6UF$S1%7NL+;aX;IaU@uMohhFRc^iB