From 1f7ff522598ea639e5609d87bab5bd83f8d27cb9 Mon Sep 17 00:00:00 2001 From: Danny Grove Date: Wed, 16 Jul 2025 23:01:48 -0700 Subject: [PATCH 1/4] Generate new key with signing subkey Signed-off-by: Danny Grove --- signature/fixtures/.gitignore | 2 ++ .../fixtures/dir-img-valid-3/manifest.json | 26 ++++++++++++++++++ .../fixtures/dir-img-valid-3/signature-1 | Bin 0 -> 677 bytes signature/fixtures/public-key-3.gpg | Bin 0 -> 2190 bytes signature/fixtures/pubring.gpg | Bin 3581 -> 5823 bytes signature/fixtures/trustdb.gpg | Bin 1760 -> 1840 bytes signature/fixtures_info_test.go | 2 ++ signature/mechanism_test.go | 2 +- 8 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 signature/fixtures/dir-img-valid-3/manifest.json create mode 100644 signature/fixtures/dir-img-valid-3/signature-1 create mode 100644 signature/fixtures/public-key-3.gpg diff --git a/signature/fixtures/.gitignore b/signature/fixtures/.gitignore index 2772b97f7d..37fbda725f 100644 --- a/signature/fixtures/.gitignore +++ b/signature/fixtures/.gitignore @@ -1,6 +1,8 @@ /*.gpg~ /.gpg-v21-migrated /private-keys-v1.d +/openpgp-revocs.d /random_seed /gnupg_spawn_agent_sentinel.lock +/tofu.db /.#* diff --git a/signature/fixtures/dir-img-valid-3/manifest.json b/signature/fixtures/dir-img-valid-3/manifest.json new file mode 100644 index 0000000000..198da23f92 --- /dev/null +++ b/signature/fixtures/dir-img-valid-3/manifest.json @@ -0,0 +1,26 @@ +{ + "schemaVersion": 2, + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "config": { + "mediaType": "application/vnd.docker.container.image.v1+json", + "size": 7023, + "digest": "sha256:b5b2b2c507a0944348e0303114d8d93aaaa081732b86451d9bce1f432a537bc7" + }, + "layers": [ + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 32654, + "digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f" + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 16724, + "digest": "sha256:3c3a4604a545cdc127456d94e421cd355bca5b528f4a9c1905b15da2eb4a4c6b" + }, + { + "mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip", + "size": 73109, + "digest": "sha256:ec4b8955958665577945c89419d1af06b5f7636b4ac3da7f12184802ad867736" + } + ] +} \ No newline at end of file diff --git a/signature/fixtures/dir-img-valid-3/signature-1 b/signature/fixtures/dir-img-valid-3/signature-1 new file mode 100644 index 0000000000000000000000000000000000000000..5a05b8cdbd6ee139a29566be7195b54a838fbfb0 GIT binary patch literal 677 zcmV;W0$Tl}0h_?f%)rH1TT}C)_cn(BXZSvW2;&X`*>j zYNDZ;k(s5XVOmO}nSpVtL5hK4l1Zw0nwf!#k$I}ADaf*t$^wvc5=-)PGm{mP^Ycm) zGxJi56pAy`^Abx+i&8;~@(W5b^Yg&|s;%v0+|0tj$i*NlrpQvB|8#cY^4{&6mRzfV z1qdq>C@>hfI0axLy;9u&%UaIZX*`}*cVl;9Yqj%)YpyabOp5Zy-U?iPaI9Rr-%&+= z^TV_02ef{ar7b9pTfFK+p3|j_M@KfU)|fK$osx1!v}xhA7s`g9#0yhTwA=(^+q;jTHSm2{H@$ylPY<(b1&!ACqK!w zdT6~R=}+N-)GIQ{lNSp#Ol6My)3x?OUGlfrhpcaGIk7F4;n-xW(6@2Zo#jh3+jd!5 zM3{Q6EtARgmcQ}fE9b(DRp-~-d=N5sb*P!}2Ja&;D=!|KHjC|C<-s35F{SsyzTHgV zG`KAO`}3)jLaWYBI$ZQD!PIFD&*L}2{>sa*m^-W%ziVgyc>TZa#k#$x=Epn|@5*xu z4L@hlwDm=-*OuK^Z(m)gd%ZMJ%j%u3^R^nz+@LA4CZFdpb#D0H-`n=iZcp&N>hl2^ zGp&mjyl8TMb*q(aN=!5FFXOp)dM+NA_ow~emow9hwnSCLoRB+GDeP}|R`7Ap<((Ca L?e;#uaPBYwU$0_q literal 0 HcmV?d00001 diff --git a/signature/fixtures/public-key-3.gpg b/signature/fixtures/public-key-3.gpg new file mode 100644 index 0000000000000000000000000000000000000000..4b0010c5fbe0e7604ca2ed9fb5e5b0d26b894831 GIT binary patch literal 2190 zcmaLWWmFRiAII@+gbaqnNJ%H9o3tQ#DKV&VQ5cLy8i9@Oyfo5CsWb>8;b2U{AxKCg zNGmXj(I}z7;Hu}`^SpcBe$W5+>VH1Dz+`GC|CCT51kmH!Iaw<(DznIkInAe>ww*$H zU2a;Ioks+0sy}_9|J*lg_#yrl&<3r@(3)jJY$BsO;n2rjVAMH@+;f1nXgD-suNUp< z<{xvufnf~5eKc#TXO=(j89NWN5LX)DvIkV9@64H@T8Hhq@#=i4XL)#6j&XbnG9pB* zx9gEHbvGljLdkdQQ(Dvb!_TKG{Y_&r|?5NjMhV`g#w);laZdKbPI zOv5*nS^GK#MrD*op&g9#doZ3a4+qc76wh86Y6eYcp55h6@GXQHoXH!dUxm?mww27( zOvnlPi=ee7OBbK0ql+C;&~|rV;bYYDea$QP(22CRUCW&9G&5DO%frWt&e!72s+=S!IcA=_0 zh^4*t*&W2rB#iYHD78n%AEsZvV)}*-bSbI7AHv(gSXC zT&7NU4)IFY51dQuxCdai?0CHi>cvy#?YuIoMI-KtR;arup1{K}wURRoNkqJduLv5E=Ls`$3a8eYsTAIn;RMMrqz}9*A`azG^bdv&s z_axMW_5RyCc0b1y(kUG65YKA-?QJp1QrYBW=*rT~T{dl>q?E{gEfxui7^mjvo#ndy zV-5QL!fJrTNB31(^4nYE+hS4zv3Y;>Stc1OS6J=nh96}nR$Np&YcA#4BW0JGa!)cD z7{vLnVv=T^ZRZEmgbkxFm&?j3fAE(%6#LTgi9APMQRzNz6EMx{r>$5fz+g+bmyNhP z7e~0iGX?Xj08o97hfxabhq+Y;@s?ZAkCh{Klw86ejyXcVIFe0MXFx5Gy5|YX-x43L zza;v82S0@>eRA!%+Mf1vd_dA;z5Aa+l@h}-B;0zD!M2DXddF`MCh-*0YH29prpyF= zL#(G*7kucF5y}Mx*eALHH(H1e3d*yh3=(#S{jrw~* z=Fal58M(nL`|)fP#vQJOR)k_VhEDj5y<9^$hsO6>20P5M zSRjY~;@Xyy>g+pJQu&G6ietEY7(TFAwO|ADDaZ!8nS@0)mfnUMx7xA(#apa-`|B`g zp2LWcUTIszz7HpWYZ?i69 zzS&^uhvuVfod?p_sJzU$URk)xEWYRvarD_AHahHr&zRX`vs}&AeC%}n%|uy@_K;TH zP!$hdy$o6*kH{Z9tbHr0n79Vxe~u=aHiBw?zm+y4UzF{%07x^bz7L>!cX*4@vIqC*y4 zqMf}_DfRbYYDNd)?f5fEpTEQFI!=?l z|2+);kS+eM?n5>Ym@?5N2=@_HZq$4VMD@k* zfW9o{6roh}*qC0jaqvKwHC*dShrOluv_t=HP{x?4F(!QV>$LV}2*6`l&$ypbH2G9d zRNq9`D3(c#+6Q}w$O{B_?S^BmN>G)B&8=lp*;9JL&kkx*r^XGPP<0V~C>{r5rAC=n+>xW%VAUzGcW(yz`1MdqQmvQVt z?M*a-N$hbxrduZb)^>A}HV#;NyTURJqR*@7f0~0lQT*Soe#w0GAqGiWMA!;dt;Dx< zE}TEQW@NbZI`hb=JL1!YquvWQ(;6cku>7a2Ycb>}uMn~B>7!UzjY=%e61oitD;)Yh zUOj+u(tj4TpNA}hLk`joxO`#$04I&`Y5`X`-JCb%edYCDZu+P>d3`2X`D1V1*s&lH z&DsY&Y+C#&&9oZp9vPIRy@qOMqjmMIo`TQ{l(6d2y;*0$bR^-v JK^CSe{{n;MEO-C_ literal 0 HcmV?d00001 diff --git a/signature/fixtures/pubring.gpg b/signature/fixtures/pubring.gpg index 8cad08aff761cc200306ddeb2e9fe202aad30abf..887b6f6d36c39f79f5e635adf69fd675cc3a2a0f 100644 GIT binary patch delta 2248 zcmZY9X*kq<7YFc}v1AxohL9yqwwftxRF>P8lrV0^FxfNKFe8lhZ@VoayO7EjqOvrM z$U0f@7svyru)7meBfv0F zmsk6T>SM#I(~|hz`>frTU0Ap0Is3=^QNhc451yDk3d|n7Nxlen!d~EM&bFpDFtP1O z_}vaDW}m_A+9cT3ZCMM~%6D~2jd+~J^9GaN**4VjYwUN8>;raZH2XwdL6sS+6SkP< zL02)do}|uRKG_R6GD=QGM};eNciggKuSL->h53>S#8bDIQw56)Dark5DyR_Asqcl- z-|j1b;;kZSws!lkZUeEgYsmF54#|P6>KCa9Hp_e#XMaM__0jtpNa&<$G5y3qBV^2A z@3PpPz(R!io`z+{Nd%W)O9}1Sn7TAk4r`=TI(<(MTMW2i;EQ^EjqXIrD770w4nwh_ z#R&J$7iavYDnHFHDhRASOOWrOQFW@!8?R0V+?gt<6$Z3RxPW?n^$yJ-^9H$I>!dGL zw#9>pkiwAbKYYI2Z6Jx{c(PS*#=L}i?^8Bs_f?742JTLpS}W)v)Aj$I`TveG2f!l006QGa4LWz^ID3XixPOLO$V7VE6%fB`TWM$<3(W!f zwP=Vqn1ct(bqK;K%njw@fv_Keuz?{`V2DU0h#SiPr&&^(XYcHtJEQ9Y?NggG-DI}> z)#sMgSkz_tay`!rJ4oOIKZ+5=N6saYP8<@QK6#@5Fc+YnH`Ivhd6*G?)T*5hLn}YM z!|~C_rxLd4mAD`A^S)YCJ|#zP5hbYESlt;n>+Ys8HCU#{^uyd!AG#nRfbDW^YnFBR zSbL&n!udxTNyu7FTV2B7HLs1o?U7HK_?3YMXsf+kC@}&QHMAARbEPBQs>qrptFOBz zp|54?OaXR^Lg!1fgd0jaw5EjGw_aTD^8eUyo+7amVJ&#=m9fi@k@Fd>BdsvMD)ObT z3QDDFDJk%|nR9DGMghsG(Hn*WiYx^YgHNloH5)(mxqAz%K#K2t=GB<5E{=XxP(Bu) z|5uNFvV~T;!|K(@?X0Bo!@7HorQ++1oKhRH4}9i;c>-}BpG@~~p6pMTwTL}lrlzL# zmZ-X=@VV_hbAmai-TAZSm`!#sXZb7zftVpZttS~D9uoV;7bc|*!t}Up#he%3%B$Q= zv|oY0s~B=&<<)S4*bZ(1MET7$SLT2z(0u>0;tMFhR0ooVu84G+Ve6#eOqg-QjW}Sb|U3v`O z2Wg1C04FRB>R7VxHt-@5%XTbHV9tz-HW-*ib+o2Lk0WbPakBr$xCN%XRiZ5LS}4vLC|z zO#5l(>59U6{xj3KsO?Wg#PFaeIg_?7WIvx{aM%6x%dy9CMgxX51C`=jwW`?j`BbT} zL8FT?odS_f6^e4W;q>qgRjI8JUvZUsMuOCF-PVZg+OMaPybznLbod9#Zi zaFhNoH{k!h<;p7jVcTrVkxqH>h>S#k&=wxWSedi~^auhGri+bb2%8kZ|lB!YI3t8M)) zOJ2zu3ia~aqP?v7E3n$80p~OAhfR7GS428!Vi*{x3Hy|8kYC>)r0yhG)s*NnYy4ui z(Z`QgtrIZbn)(y42_KGqV6Hh>4=H?(yhQyO2t*OzU>|xOG?_^WI~)iL&-FLDMDmTV)XLhzt98WKYPbs#_OkeXJhJ z5KaiNS+SOKbeT|c!V$P#3LooJ1D?nJ(-`W9k^1-K=d9VYi>z(S&Zzf?w!06VORtZ{5u|nn7vh-ro}=P@GKL9W`V|C{J^U*OC>;1UTGfYl zH+vYmk&iAy!Zy=4MFSB;kh^|l)iEz5*Tikun~KxjV%#wk8fJWI8b5DK#E*o^8B|~I z5;BxZZRGUVU-ZpnC9l_2c~~y$t#lPclw;2LW}e+PrA!fXMTL__uqo5yRf?zu#<);d zTnOw8S_Hil$#{2zx2U~U%Iui}0HeX=+$2|ERi-_N(%-Z5f zepPoS9`s@mNIkfuy6tZO8v_{VLMRZ)%dqqzXX~uzuIpnMH_B&-nTl*?Ob>^uLnvba E0A17`f&c&j delta 56 xcmdnM_kdS`F})z2nVFH5k%@sJBXXtoL__h7>U~Tb|7I|5mSx$3YGu> diff --git a/signature/fixtures_info_test.go b/signature/fixtures_info_test.go index 2ab4ed05a3..9b6ab41013 100644 --- a/signature/fixtures_info_test.go +++ b/signature/fixtures_info_test.go @@ -19,6 +19,8 @@ const ( TestKeyFingerprintWithPassphrase = "F2B501009F78B0B340221A12A3CD242DA6028093" // TestPassphrase is the passphrase for TestKeyFingerprintWithPassphrase. TestPassphrase = "WithPassphrase123" + // TestKeyFingerprintWithSubkey is the fingerpring of the private key with a signing subkey in this directory. + TestKeyFingerprintWithSubkey = "92697D73924E7ADC91C14A001065C1A47B86FD50" ) var ( diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index 566993e737..caa7e6fa90 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -143,7 +143,7 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) { mech, keyIdentities, err = NewEphemeralGPGSigningMechanism(bytes.Join([][]byte{keyBlob, keyBlob}, nil)) require.NoError(t, err) defer mech.Close() - assert.Equal(t, []string{TestKeyFingerprintWithPassphrase, TestKeyFingerprint, TestKeyFingerprintWithPassphrase, TestKeyFingerprint}, keyIdentities) + assert.Equal(t, []string{TestKeyFingerprintWithPassphrase, TestKeyFingerprint, TestKeyFingerprintWithSubkey, TestKeyFingerprintWithPassphrase, TestKeyFingerprint, TestKeyFingerprintWithSubkey}, keyIdentities) // Two keys from two blobs: keyBlob1, err := os.ReadFile("./fixtures/public-key-1.gpg") From 728091314f6dabd7d2cd0c5c20c3d1e366b1d2e7 Mon Sep 17 00:00:00 2001 From: Danny Grove Date: Wed, 16 Jul 2025 22:30:45 -0700 Subject: [PATCH 2/4] Add support for validating signatures from PGP subkeys This change should update the gpgme and openpgp implmenetations to support validating signatures made by signing subkeys. It will close an open issue that was referenced specifically in [podman](containers/podman#16406), but should resolve this across a number of tools that use this library. Signed-off-by: Danny Grove --- signature/docker.go | 18 +++++ signature/mechanism.go | 4 +- signature/mechanism_gpgme.go | 56 ++++++++++++++ signature/mechanism_openpgp.go | 101 ++++++++++++++++++++++++- signature/policy_eval_signedby.go | 12 ++- signature/policy_eval_signedby_test.go | 7 ++ 6 files changed, 193 insertions(+), 5 deletions(-) diff --git a/signature/docker.go b/signature/docker.go index b313231a80..ac7e8df212 100644 --- a/signature/docker.go +++ b/signature/docker.go @@ -51,6 +51,24 @@ func SignDockerManifest(m []byte, dockerReference string, mech SigningMechanism, // using mech. func VerifyDockerManifestSignature(unverifiedSignature, unverifiedManifest []byte, expectedDockerReference string, mech SigningMechanism, expectedKeyIdentity string) (*Signature, error) { + _, signerKeyIdentity, err := mech.Verify(unverifiedSignature) + if err != nil { + return nil, err + } + + // If signerKeyIdentity is different from expectedKeyIdentity, check if it's a valid subkey + if signerKeyIdentity != expectedKeyIdentity { + // Implementation depends on mechanism-specific functionality to verify subkey relationships + // This could be a new method on the SigningMechanism interface or mechanism-specific implementation + isValidSubkey, err := isKeyOrValidSubkey(mech, signerKeyIdentity, expectedKeyIdentity) + if err != nil { + return nil, err + } + if !isValidSubkey { + return nil, fmt.Errorf("signature by key %s does not match expected key %s", + signerKeyIdentity, expectedKeyIdentity) + } + } sig, _, err := VerifyImageManifestSignatureUsingKeyIdentityList(unverifiedSignature, unverifiedManifest, expectedDockerReference, mech, []string{expectedKeyIdentity}) return sig, err } diff --git a/signature/mechanism.go b/signature/mechanism.go index 1d3fe0fdc9..a788753705 100644 --- a/signature/mechanism.go +++ b/signature/mechanism.go @@ -27,7 +27,9 @@ type SigningMechanism interface { // Sign creates a (non-detached) signature of input using keyIdentity. // Fails with a SigningNotSupportedError if the mechanism does not support signing. Sign(input []byte, keyIdentity string) ([]byte, error) - // Verify parses unverifiedSignature and returns the content and the signer's identity + // Verify parses unverifiedSignature and returns the content and the signer's identity. + // For GPG signatures, the signer's identity is the fingerprint of the key or subkey + // that created the signature, not necessarily the primary key fingerprint. Verify(unverifiedSignature []byte) (contents []byte, keyIdentity string, err error) // UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, // along with a short identifier of the key used for signing. diff --git a/signature/mechanism_gpgme.go b/signature/mechanism_gpgme.go index 8a8c5878a0..fe145b9361 100644 --- a/signature/mechanism_gpgme.go +++ b/signature/mechanism_gpgme.go @@ -204,3 +204,59 @@ func (m *gpgmeSigningMechanism) Verify(unverifiedSignature []byte) (contents []b func (m *gpgmeSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) { return gpgUntrustedSignatureContents(untrustedSignature) } + +// isSubkeyOf checks if signerKeyIdentity is a subkey of expectedKeyIdentity +func (m *gpgmeSigningMechanism) isSubkeyOf(signerKeyIdentity, expectedKeyIdentity string) (bool, error) { + // Load the key for expectedKeyIdentity + key, err := m.ctx.GetKey(expectedKeyIdentity, false) + if err != nil { + return false, err + } + + // Check if signerKeyIdentity is a subkey of the primary key + // Get the first subkey (which should be the primary key itself) + subkey := key.SubKeys() + + // Skip the primary key if needed and iterate through all subkeys + for subkey != nil { + // Check if the current subkey's fingerprint matches the signer's identity + if subkey.Fingerprint() == signerKeyIdentity { + return true, nil + } + + // Move to the next subkey + subkey = subkey.Next() + } + + // If we didn't find it as a subkey, check if it might be the primary key itself + // (in case the expectedKeyIdentity is actually a subkey) + key, err = m.ctx.GetKey(signerKeyIdentity, false) + if err != nil { + return false, err + } + + // Get the primary key fingerprint + primarySubkey := key.SubKeys() + if primarySubkey != nil && primarySubkey.Fingerprint() == expectedKeyIdentity { + return true, nil + } + + return false, nil +} + +// isKeyOrValidSubkey checks if the signerKeyIdentity is either the same as the expectedKeyIdentity, or is a valid +// subkey of the expectedKeyIdentity +func isKeyOrValidSubkey(mech SigningMechanism, signerKeyIdentity, expectedKeyIdentity string) (bool, error) { + // If they're the same, no need to check subkey relationship + if signerKeyIdentity == expectedKeyIdentity { + return true, nil + } + + // For gpgme mechanism, check subkey relationships + if gpgmeMech, ok := mech.(*gpgmeSigningMechanism); ok { + return gpgmeMech.isSubkeyOf(signerKeyIdentity, expectedKeyIdentity) + } + + // For other mechanisms, only accept exact matches + return false, nil +} diff --git a/signature/mechanism_openpgp.go b/signature/mechanism_openpgp.go index fea8590e1b..5071ee5529 100644 --- a/signature/mechanism_openpgp.go +++ b/signature/mechanism_openpgp.go @@ -166,7 +166,8 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [ } // Uppercase the fingerprint to be compatible with gpgme - return content, strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.PublicKey.Fingerprint)), nil + keyIdentity = strings.ToUpper(fmt.Sprintf("%x", md.SignedBy.PublicKey.Fingerprint)) + return content, keyIdentity, nil } // UntrustedSignatureContents returns UNTRUSTED contents of the signature WITHOUT ANY VERIFICATION, @@ -177,3 +178,101 @@ func (m *openpgpSigningMechanism) Verify(unverifiedSignature []byte) (contents [ func (m *openpgpSigningMechanism) UntrustedSignatureContents(untrustedSignature []byte) (untrustedContents []byte, shortKeyIdentifier string, err error) { return gpgUntrustedSignatureContents(untrustedSignature) } + +// isSubkeyOf checks if signerKeyIdentity is a subkey of expectedKeyIdentity +func (m *openpgpSigningMechanism) isSubkeyOf(signerKeyIdentity, expectedKeyIdentity string) (bool, error) { + // Convert fingerprints to lowercase for comparison + signerFingerprint := strings.ToLower(signerKeyIdentity) + expectedFingerprint := strings.ToLower(expectedKeyIdentity) + + // If they're the same, it's a match + if signerFingerprint == expectedFingerprint { + return true, nil + } + + // Find the entity with the expected fingerprint + var expectedEntity *openpgp.Entity + var signerEntity *openpgp.Entity + + for _, entity := range m.keyring { + // Check if this entity's primary key matches the expected fingerprint + if entity.PrimaryKey != nil { + primaryFingerprint := strings.ToLower(fmt.Sprintf("%x", entity.PrimaryKey.Fingerprint)) + if primaryFingerprint == expectedFingerprint { + expectedEntity = entity + } + if primaryFingerprint == signerFingerprint { + signerEntity = entity + } + } + + // Also check subkeys + for _, subkey := range entity.Subkeys { + if subkey.PublicKey != nil { + subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint)) + if subkeyFingerprint == expectedFingerprint { + expectedEntity = entity + } + if subkeyFingerprint == signerFingerprint { + signerEntity = entity + } + } + } + } + + // If we couldn't find either key, return false + if expectedEntity == nil || signerEntity == nil { + return false, nil + } + + // Case 1: Both keys belong to the same entity - this is valid + if expectedEntity == signerEntity { + return true, nil + } + + // Case 2: The signer is a subkey of the expected entity + if expectedEntity.PrimaryKey != nil { + expectedPrimaryFingerprint := strings.ToLower(fmt.Sprintf("%x", expectedEntity.PrimaryKey.Fingerprint)) + if expectedPrimaryFingerprint == expectedFingerprint { + // The expected key is a primary key, check if signer is one of its subkeys + for _, subkey := range expectedEntity.Subkeys { + if subkey.PublicKey != nil { + subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint)) + if subkeyFingerprint == signerFingerprint { + return true, nil + } + } + } + } + } + + // Case 3: The expected key is a subkey, and the signer is the primary key of the same entity + for _, subkey := range signerEntity.Subkeys { + if subkey.PublicKey != nil { + subkeyFingerprint := strings.ToLower(fmt.Sprintf("%x", subkey.PublicKey.Fingerprint)) + if subkeyFingerprint == expectedFingerprint { + // The expected key is a subkey of the signer's entity + return true, nil + } + } + } + + return false, nil +} + +// isKeyOrValidSubkey checks if the signerKeyIdentity is either the same as the expectedKeyIdentity, or is a valid +// subkey of the expectedKeyIdentity +func isKeyOrValidSubkey(mech SigningMechanism, signerKeyIdentity, expectedKeyIdentity string) (bool, error) { + // If they're the same, no need to check subkey relationship + if signerKeyIdentity == expectedKeyIdentity { + return true, nil + } + + // For openpgp mechanism, check subkey relationships + if openpgpMech, ok := mech.(*openpgpSigningMechanism); ok { + return openpgpMech.isSubkeyOf(signerKeyIdentity, expectedKeyIdentity) + } + + // For other mechanisms, only accept exact matches + return false, nil +} diff --git a/signature/policy_eval_signedby.go b/signature/policy_eval_signedby.go index e5c9329185..e398eff0f2 100644 --- a/signature/policy_eval_signedby.go +++ b/signature/policy_eval_signedby.go @@ -6,7 +6,6 @@ import ( "context" "errors" "fmt" - "slices" "github.com/containers/image/v5/internal/multierr" "github.com/containers/image/v5/internal/private" @@ -52,8 +51,15 @@ func (pr *prSignedBy) isSignatureAuthorAccepted(ctx context.Context, image priva signature, err := verifyAndExtractSignature(mech, sig, signatureAcceptanceRules{ validateKeyIdentity: func(keyIdentity string) error { - if slices.Contains(trustedIdentities, keyIdentity) { - return nil + // Iterate over the keys, validate if the provided keyIdentity is either the key itself or a valid + // subkey + for _, allowedKeyIdentity := range trustedIdentities { + if isValid, err := isKeyOrValidSubkey(mech, keyIdentity, allowedKeyIdentity); err != nil { + // Throwing an error shouldn't exit the program, we should just move to the next key + continue + } else if isValid { + return nil + } } // Coverage: We use a private GPG home directory and only import trusted keys, so this should // not be reachable. diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index bca2e722cf..38049bf801 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -277,4 +277,11 @@ func TestPRSignedByIsRunningImageAllowed(t *testing.T) { require.NoError(t, err) allowed, err = pr.isRunningImageAllowed(context.Background(), image) assertRunningRejectedPolicyRequirement(t, allowed, err) + + // 1 valid signature from a subkey + image = dirImageMock(t, "fixtures/dir-img-valid-3", "testing/manifest:latest") + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key-3.gpg", prm) + require.NoError(t, err) + allowed, err = pr.isRunningImageAllowed(context.Background(), image) + assertRunningAllowed(t, allowed, err) } From 41616310c85232d226b2d367b30dac3d7027ea5e Mon Sep 17 00:00:00 2001 From: Danny Grove Date: Fri, 18 Jul 2025 00:09:51 -0700 Subject: [PATCH 3/4] Add a new key with a revoked signing subkey after the signature time Signed-off-by: Danny Grove --- .../fixtures/dir-img-rev-subkey/signature-1 | Bin 0 -> 675 bytes signature/fixtures/public-key-4.gpg | Bin 0 -> 2647 bytes signature/fixtures/pubring.gpg | Bin 5823 -> 8530 bytes signature/fixtures/trustdb.gpg | Bin 1840 -> 1920 bytes signature/fixtures_info_test.go | 2 ++ signature/mechanism_test.go | 11 ++++++++++- 6 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 signature/fixtures/dir-img-rev-subkey/signature-1 create mode 100644 signature/fixtures/public-key-4.gpg diff --git a/signature/fixtures/dir-img-rev-subkey/signature-1 b/signature/fixtures/dir-img-rev-subkey/signature-1 new file mode 100644 index 0000000000000000000000000000000000000000..232941fb5faadd148aa63773cc20403a01852983 GIT binary patch literal 675 zcmV;U0$ly00h_?f%)rHXB}!?6uv?-Pv!wy9de*$3Zvl`yj?7m?3=lOQtr3F73 z|0*AgdFX#>@3TMC3jgQ6cv)W{v*@`{_PJjDUO5r z+OEex{q`Xx`CH&4S2cES@x;Pk{Va|_N9Q*4x9U7-Kjs2;!mOYDouZOO>Lr{&f@ zI<)IfzKq$)7eC!5ziBFUc=+j$W09?Y_G!fn2M#H&_}7za>)3QxG=1X4h{OVwt8oRB zZ{B|5bt-IcK*Pzs@*=6m?)m=Bx)FIFT^CI`5wb^r-Wtx=@jPy`>b@{&x;b+?=&%O= z4BEM7gRz0l%QHVY4mw3t^EHSq?9H>3y2rSD@pa|eSCju6&hFjTaAoSx7H#HDn=&|d J9D6^d3;-KDXsG}I literal 0 HcmV?d00001 diff --git a/signature/fixtures/public-key-4.gpg b/signature/fixtures/public-key-4.gpg new file mode 100644 index 0000000000000000000000000000000000000000..95a55c0d90029ba75e5f092dc5021bcd2aae198a GIT binary patch literal 2647 zcmai!Wmpr68i3gVY3VxDXhB*XJp}|QL258mdax0r8zck~VS-AH7&$^}l(dX4;h?lq z9&nUM%jn}d=Q+>4zwZ6}z2EoieclfPKvN)sx2yp4Wc9ec(j~9i{Zw(4HB0gM=xlQ} zQcB6hTy|+CISWm;YxSzJIlrp;-L%TJl{G-NK2di;7q+=JMl#Iht~Mx@$?0Nr`jYS| zA)Q)|=HQO?40+wWhmuMpj7oD^A62sJ1)s@Cm$XUj1#&C3BWzwsd$q#*jJ1@!ARV8k zRpzG4c{Uc%lb@I5!t_6yTTYZZ&2ubD9(uNQ+IFQ#cpOTMOQVAF(gdTQlKcnR@cash zq$;6r;xv$F0t4a_Db&O}W?_!+nC8OYOvRg_^I!(~TLD*W@&Hvlxs}c%>?tQB9N5o* zkxG%5<*P2Z#we+45mejm`I18-)3(Evd%QZmQ>jtnQ(65M#i*0(90vI_DO7~T>}s?U znP;Yu`Kq!Sk_A)i@WmdVf7Fd6vIb!sAZ=HUsSrWhtGuYlB5&sKZzHXf*JB?k;G0!q zL=&L=HXproUrrUG7Jr=_8$ z15$7R$pJuq0FX11jFyJ!-{tuPda@{#a;J~z1V!&|LXIp4^?0tKwiL9?wEOOZ!h1Q< zN}+0wj1Jvux(V&w%#VpSp;w2_jjz8WcHlmir3~XI%jiJ4?Q&7LQ-gR)!ELNE_b%`~ zYGyz&(_6^l=>100SgB%h_ZK#Wi1wP`o1jB$VWLJFth7aIDFEzlLv%K~K|iEdDJZ}Z zuHY8&ndmKiGkte8QU4G=l~k>(sZLC6sMM%WUG+5AU{Xk#acxxe6!hcF6$&@CFRT>O z4A#2uH10XuzNb}VtTB5|=y^b>?D^(#@0Iho4AnHZsr_+gi5&EuSKe2rq!SI4&&{d_ zZR%S0%l#jGKZv@dZTvmeV%xk{j!i#7xrNy|4t{S%V+&dQZdWiZtk(Q75 zGCaNWP#&{ChsT|Ak+{y2k`F^FK?z(!ii$RYCXq#76D-Pi6s<)JNzK1E|C6WtuRIsJ zSbw@;=O1!PqMk%|^mt23uysn+g*|jlt_?|)@sM_!`;m2sn6tQt7~27>Uqng;ww!zS zby9h8d+Z%#L<_5Tewj-=PhvcXzER9l7Q0qFV^e8dZ%X|vf6hn&{)RsVie_N_9M{kC zQI4|@yMxFA&-@hZ9|d313i}GI=fsOD8pO(uwvtxyy!=)0E@4mjEi-Au3{Ci+4Xyrp zT#bk$$zz>4BYFUO4zDQlrzeFf2CU@l83tKfp=AWB(w0vJjfE;DT@Q=tjMDu4EMzMn0*{5zdZ9LL@N2RUCI)i#OL zq77W!qIZB5&@A@xT%?(xYF(j}%-cZmmfqR=dJ6SnyvFza+E=!(?3AB}CZ@-!yHajD zG6PdcGt)T_O6)EmQ*sre!++-)4|qp$?N?x+{}I@abs!(`ze5s?q`R~W8afVX@_F<# z!9mhu$BK`t;0_D8c%Ra?lLpm_O{4=_sNayv&M{nT5`!|D_$v^>z#G*8u~?6w!OJVk zd{f$Vx%`M&8E7-Jf}%i?uJ&|%gO2!D;Uq87T2s;cneuuOvB#aTIdYY z=TcABJ}X?l@X>@|*o{L!u1<>28>e7FUB_|CP^D!ws@i3b{szL5V)A^ZnQ<--xPm_Wd=!O1i?ldM1b5hSy8 zJeJ5fIZU^IEuP$+g6sc?+j0l;00D&md_ckhh=Tjq5#KM{pM-3@&-XneX5q(u6V^h> ztUC58f4@`rzf}J_X~?BJqvgOz{m^UPr|l-9MZsxP1rTQKbk5gQrBhNl+Q3iQxeba4 zcj>P=4CS$Y^1LMH^XJt$!0DKYa`g+m?3PYla^tkzZ52q(l+lXaX=VNp&83kt$7Y4! zp5=RBa^ux!gTRv%p6+qjD44DNgKL3{bm0g3ex%PF_sPBzC-mzoz1ibM8rNhQ68=hv z2OCsA8r5LHK$s8#AFK<8UYx!N!Y-={wviXaBWbXp`)_-^7@D)L$znX}e)9D5lF9+Y zWOw^(LZD@4e~{}*1JA0ss(nF%JG%!}wmQCq=ABvj?#4qdFtLOk((z1hKwBFyO?x{e z9Py);uT80Qtk?~BOYA5oHVt?sz{mJyfTvmWSq3fp!*5~O_G|2?A7$$G~M}NBJg^6JA{Mb`!o#2^ETpev!O>7T# zRjuk3wIEWapm8CAuEtV#bETXh=LG0=z&`TkGL%=2C-g?za~0Z~=tc!*-b`~b7kQnL zeOr8}kb%y{n$cQ~cEmBkZgb+(L;m7G`U-c($FBFC*&9zk>5U<9E9)YVpFH^1nD(aU zB05|`I1Q5DwXB)Vh#oY?UTX2~x7UMO+n&KB0IZ7SU`T+IrLJpi-6r5;r1_+J?ZZL( JB_ZN`e*)8a>VW_N literal 0 HcmV?d00001 diff --git a/signature/fixtures/pubring.gpg b/signature/fixtures/pubring.gpg index 887b6f6d36c39f79f5e635adf69fd675cc3a2a0f..0ccff288e77a2ede2e0a10e90cb1efa6afbd64df 100644 GIT binary patch delta 2676 zcmZ{kS5OlQ7KW1m(xtac3tcf(HFOjZL_m-h2r500P(qPj3?PUI5tSNB05KF12u-?# z-h_*k07~26Gsy>7!((EuYG6M%QA8P;Tp|8hhEe;WXrIjaR0hgb%Ltb@>@$%mKHRuqBj}8M zKCnVCuiR~jC-q>67ykh`R4)0XY}uVq9}PK&q2I)K703=qPC5*hZwnc8lhdMQ$l1Nt zMd*VIyv7Ansr01zoJy=Jz$Z)GYFSMa#fhtS{^&$3_|b`?aE0KUwOh{Y(<4Llmxa(# zFa0v^Nzw_oHhzgD062gEAdv+CaQAfw0)9=crVT0p3AR84`FJ8+!Img@Gz#qw zwhVOkM1+DhgMWrs{r?G5-muC2u?O7PA`BsPMF-96?l;PHM+;oGr33wqIOUj46!ZHu zo5%u)Mys{^OO4ZZZzbj`@G^|$ndn1cr52snZWUh7jZqEPbYXMu)HF& zR>rhegRV-QQk;pPs|63pTwn%t>q-6Jp?(q z(ok26l2li&Q=7KzW2M8coI2%Eui_&Xz?UZ;Y2j2@F0LD@chhyuXS#J;ugXkk`k2&p zi&WnA+57r4w=sG8NdXI|{j6d|*c;#cPp-)aI%xk(6}MWn^loCx0&jiUjXq_n|1-^c z)2dpL`#i)YNpicZ`GMUQch>sN=&IBuM5cn2`e&DL;wxS>VoldDXSz!{*?UECZ%LyKK^w_6?aD7Rcn?T$l4jnruWw~ef;v#-qYU)$6WJJg!Y5t zcLVAniTvU!D)zw`^Qf1f60OU=sn|)HP#gawo&nJOWDRKiM?K!-43aVxb5+@IxI=vTh4XnYxW*;#`-#PhGpgY5Z}xBUXiZ{{|%W9 zo%$};I}AN#lDH?jnwub{VjQP1+)Q023W-!8IwX7$mo4Q?GIbH#_Dn{{@l}#8RPR-e z%$PpdG2(S;APY5IC1@#k+a$!!7Ar4WnSOpccq@dfOQ1&mML|-mF26OT{}~cqwfQz= zYZue;9)4PrW8BPvMT;MsF6V16a369y^G!?M`O11-W}Cy`neOAmdn61ib%3Lj1g6 zkS)kE?(R&KrIRFuuN0;bZsrI)*yzc^JS;{nZq+jwMXGe8Sz>kbek?5pj7JA zWbUnE#}jR`;%lkFUm7L=-_V`|G6BH6VzmEHs;{dcVGw|ck^Q$;Vo}Vejv)j4VGaIw zz9%}%T7R<@rhjpj6I!%G=g`iG#x&!Tm_gQB7a=*hCMylnFgEi*WeOB@u`(zQ?;X;A zdPYr{tk0Y$f{c@gHF7Ach`uz`pG>GTkohE$>?>7mA$2oL=`VNUR`Fb}+xCD(#M{er z?IA||T2J+l3KviOb+vIkX5sIbCuC;LQt{vpGoeLt7_9<+fu4bjg8Gqy@hMsjs!{a6 zkps+9srGEI2FBz2K5wL}aFoH;pX*;UFvGp(eN-pCcwpDh1C8T$c|ga%r`|LiUFp8w zBCV$)Z_Y0rD71Gx4#&IszUhp4d=zTe5I-cvV=|NB7~=^x;RhocLfCbOH+Q9NaZsFP z5iUqyVlA*+!#Hoe;rIw6SATgVrFU2FwV81lP~&SJMCq*w4l@$`=mm?CnoFR?$p@AR z4N=ePei66}mt)Q!EoxQ)+8dZ?ccV#~9*X{I6#80tteEq<=#s?Sg3qetrr6BtI%{?{~c8 zgPBI`_l(<#KjkuTQvX$_UVmf%eW}Z1K4RjUR97G8Z%%J6@)BLR8o=sCimHsdzZwiI$+!8E$C_e*@yk0gH*xJG59c)yY-#N3wOD~cPzaq{~3XU9aTQqr)3t) zyYq}5Pvxn^GhyD`Fr^rDoiQtEToSswDi(fn_&5Z=sDTk{p?Q&jV#I@QzUuO2ZOlHW zfb*{TF4!wXEdve$uJu-h!AdRvqPZalI;!BWyayKhwzXT~U~rGr_-$ILvwn{sN-5^i zZhLs6PhTH6$#f+w68W`SxJ9*nq{tI=S^7tATsr7XkiXfJARo)dqfBVjJguH-FN&v5 zMEn9m#}*?PQ3mIjU6W0ixdXCY;s0T=NBDMd)AZ&d4#Yvo^HiAJ>W}$oz@@d7Ke!nXd}$n;&bf(^_xqz?qK) z_nQnZ&`G)wIl4117S6ZT=pJON61vzIbk>P|V2ss#Si=Tq)Ewutt1{N^J-fF$qopXU zF8Z#x>SnfmYroQ=s9GpuVdCVTNKZ16TswXYgQc-Ylqqw-Q-*d>!shw8KZ+mUcaS$K zOBX1F#Cl7t+$UbYR)qQTaAyqM`UkbvG79y@`fWlN}f}Hg8~QU<%-2_{#Y9 zRMDjA|$VIn=&R3pf_4LutU0)_DxT?3@hN?p- GV*mg}_#cn} delta 36 scmZqR-@qrpm|l?1%*@Ej$i%>qQ8B@MqM`UkbvKrYf1NjPU}<0i0H2}?2mk;8 diff --git a/signature/fixtures_info_test.go b/signature/fixtures_info_test.go index 9b6ab41013..1f411dbf45 100644 --- a/signature/fixtures_info_test.go +++ b/signature/fixtures_info_test.go @@ -21,6 +21,8 @@ const ( TestPassphrase = "WithPassphrase123" // TestKeyFingerprintWithSubkey is the fingerpring of the private key with a signing subkey in this directory. TestKeyFingerprintWithSubkey = "92697D73924E7ADC91C14A001065C1A47B86FD50" + // TestKeyFingerprintWithRevokedSubkey is the fingerprint of the private key with a revoked signing subkey in this directory. + TestKeyFingerprintWithRevokedSubkey = "F501EDF1A5CD65421FCB51B57EB92DC581193558" ) var ( diff --git a/signature/mechanism_test.go b/signature/mechanism_test.go index caa7e6fa90..1d01b85fad 100644 --- a/signature/mechanism_test.go +++ b/signature/mechanism_test.go @@ -143,7 +143,16 @@ func TestNewEphemeralGPGSigningMechanism(t *testing.T) { mech, keyIdentities, err = NewEphemeralGPGSigningMechanism(bytes.Join([][]byte{keyBlob, keyBlob}, nil)) require.NoError(t, err) defer mech.Close() - assert.Equal(t, []string{TestKeyFingerprintWithPassphrase, TestKeyFingerprint, TestKeyFingerprintWithSubkey, TestKeyFingerprintWithPassphrase, TestKeyFingerprint, TestKeyFingerprintWithSubkey}, keyIdentities) + assert.Equal(t, []string{ + TestKeyFingerprintWithPassphrase, + TestKeyFingerprint, + TestKeyFingerprintWithSubkey, + TestKeyFingerprintWithRevokedSubkey, + TestKeyFingerprintWithPassphrase, + TestKeyFingerprint, + TestKeyFingerprintWithSubkey, + TestKeyFingerprintWithRevokedSubkey, + }, keyIdentities) // Two keys from two blobs: keyBlob1, err := os.ReadFile("./fixtures/public-key-1.gpg") From bd6e15e21476f4602efa08645fca0fcfb4564dd7 Mon Sep 17 00:00:00 2001 From: Danny Grove Date: Fri, 18 Jul 2025 00:10:36 -0700 Subject: [PATCH 4/4] Add test for revoked signing subkeys Signed-off-by: Danny Grove --- signature/policy_eval_signedby_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/signature/policy_eval_signedby_test.go b/signature/policy_eval_signedby_test.go index 38049bf801..bb29e04eaf 100644 --- a/signature/policy_eval_signedby_test.go +++ b/signature/policy_eval_signedby_test.go @@ -207,6 +207,15 @@ func TestPRSignedByIsSignatureAuthorAccepted(t *testing.T) { require.NoError(t, err) sar, parsedSig, err = pr.isSignatureAuthorAccepted(context.Background(), image, sig) assertSARRejectedPolicyRequirement(t, sar, parsedSig, err) + + // An invalid signature from a revoked signing subkey + image = dirImageMock(t, "fixtures/dir-img-rev-subkey", "testing/manifest:latest") + sig, err = os.ReadFile("fixtures/dir-img-rev-subkey/signature-1") + require.NoError(t, err) + pr, err = NewPRSignedByKeyPath(ktGPG, "fixtures/public-key-4.gpg", prm) + require.NoError(t, err) + sar, parsedSig, err = pr.isSignatureAuthorAccepted(context.Background(), image, sig) + assertSARRejected(t, sar, parsedSig, err) } // createInvalidSigDir creates a directory suitable for dirImageMock, in which image.Signatures()