Skip to content

Commit 677fc17

Browse files
loosebazookaloosebazooka
authored
Merge pull request #625 from sigstore/fix-tuf-invalid-sigs
Be more permissive for signatures in TUF metadata
2 parents 7076d94 + 1564719 commit 677fc17

File tree

8 files changed

+356
-7
lines changed

8 files changed

+356
-7
lines changed

‎renovate.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
{
2222
"matchPackagePrefixes": ["info.picocli"],
2323
"groupName": "picocli"
24+
},
25+
{
26+
"matchPackagePrefixes": ["io.github.netmikey.logunit"],
27+
"groupName": "logunit"
2428
}
2529
]
2630
}

‎sigstore-java/build.gradle.kts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ dependencies {
5757
testImplementation("com.squareup.okhttp3:mockwebserver:4.12.0")
5858
testImplementation("net.sourceforge.htmlunit:htmlunit:2.70.0")
5959
testImplementation("org.eclipse.jetty:jetty-server:11.0.20")
60+
61+
testImplementation("io.github.netmikey.logunit:logunit-core:2.0.0")
62+
testRuntimeOnly("io.github.netmikey.logunit:logunit-jul:2.0.0")
6063
}
6164

6265
protobuf {

‎sigstore-java/src/main/java/dev/sigstore/tuf/Updater.java

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
import java.util.Locale;
3838
import java.util.Map;
3939
import java.util.Optional;
40+
import java.util.logging.Level;
41+
import java.util.logging.Logger;
42+
import org.bouncycastle.util.encoders.DecoderException;
4043
import org.bouncycastle.util.encoders.Hex;
4144

4245
/**
@@ -52,6 +55,8 @@ public class Updater {
5255
// spec.
5356
private static final int MAX_UPDATES = 1024;
5457

58+
private static final Logger log = Logger.getLogger(Updater.class.getName());
59+
5560
private Clock clock;
5661
private Verifiers.Supplier verifiers;
5762
private MetaFetcher fetcher;
@@ -197,8 +202,7 @@ void verifyDelegate(
197202
Map<String, Key> publicKeys,
198203
Role role,
199204
byte[] verificationMaterial)
200-
throws SignatureVerificationException, NoSuchAlgorithmException, InvalidKeyException,
201-
InvalidKeySpecException, IOException {
205+
throws InvalidKeySpecException, IOException, NoSuchAlgorithmException {
202206
// use set to not count the same key multiple times towards the threshold.
203207
var goodSigs = new HashSet<>(role.getKeyids().size() * 4 / 3);
204208
// role.getKeyIds() defines the keys allowed to sign for this role.
@@ -221,13 +225,35 @@ void verifyDelegate(
221225
} else {
222226
pubKey = Keys.constructTufPublicKey(Hex.decode(publicKeyContents), key.getScheme());
223227
}
224-
byte[] signatureBytes = Hex.decode(signature.getSignature());
225228
try {
229+
// while we error on keys that are not readable, we are intentionally more permissive
230+
// about signatures. If for ANY reason (except unparsed keys) we cannot validate a
231+
// signature, we continue as long as we find enough valid signatures within the
232+
// threshold. We still warn the user as this could be an indicator of data issues
233+
byte[] signatureBytes = Hex.decode(signature.getSignature());
226234
if (verifiers.newVerifier(pubKey).verify(verificationMaterial, signatureBytes)) {
227235
goodSigs.add(signature.getKeyId());
228236
}
229237
} catch (SignatureException e) {
230-
throw new TufException(e);
238+
log.log(
239+
Level.FINE,
240+
() ->
241+
String.format(
242+
Locale.ROOT,
243+
"TUF: ignored unverifiable signature: '%s' for keyid: '%s'",
244+
signature.getSignature(),
245+
signature.getKeyId()));
246+
} catch (DecoderException | NoSuchAlgorithmException | InvalidKeyException e) {
247+
log.log(
248+
Level.WARNING,
249+
e,
250+
() ->
251+
String.format(
252+
Locale.ROOT,
253+
"TUF: ignored invalid signature: '%s' for keyid: '%s', because '%s'",
254+
signature.getSignature(),
255+
keyid,
256+
e.getMessage()));
231257
}
232258
}
233259
}

‎sigstore-java/src/test/java/dev/sigstore/tuf/UpdaterTest.java

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717

1818
import static dev.sigstore.testkit.tuf.TestResources.UPDATER_REAL_TRUSTED_ROOT;
1919
import static dev.sigstore.testkit.tuf.TestResources.UPDATER_SYNTHETIC_TRUSTED_ROOT;
20-
import static org.junit.jupiter.api.Assertions.*;
20+
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertNotNull;
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
import static org.junit.jupiter.api.Assertions.assertTrue;
24+
import static org.junit.jupiter.api.Assertions.fail;
2125

2226
import com.google.common.collect.ImmutableList;
2327
import com.google.common.collect.ImmutableMap;
@@ -27,8 +31,19 @@
2731
import dev.sigstore.encryption.signers.Verifier;
2832
import dev.sigstore.encryption.signers.Verifiers;
2933
import dev.sigstore.testkit.tuf.TestResources;
30-
import dev.sigstore.tuf.model.*;
34+
import dev.sigstore.tuf.model.Hashes;
35+
import dev.sigstore.tuf.model.ImmutableKey;
36+
import dev.sigstore.tuf.model.ImmutableRootRole;
37+
import dev.sigstore.tuf.model.ImmutableSignature;
38+
import dev.sigstore.tuf.model.Key;
39+
import dev.sigstore.tuf.model.Role;
3140
import dev.sigstore.tuf.model.Root;
41+
import dev.sigstore.tuf.model.Signature;
42+
import dev.sigstore.tuf.model.Snapshot;
43+
import dev.sigstore.tuf.model.TargetMeta;
44+
import dev.sigstore.tuf.model.Targets;
45+
import dev.sigstore.tuf.model.Timestamp;
46+
import io.github.netmikey.logunit.api.LogCapturer;
3247
import java.io.File;
3348
import java.io.IOException;
3449
import java.net.URL;
@@ -57,8 +72,13 @@
5772
import org.eclipse.jetty.util.resource.Resource;
5873
import org.jetbrains.annotations.NotNull;
5974
import org.jetbrains.annotations.Nullable;
60-
import org.junit.jupiter.api.*;
75+
import org.junit.jupiter.api.AfterAll;
76+
import org.junit.jupiter.api.AfterEach;
77+
import org.junit.jupiter.api.BeforeAll;
78+
import org.junit.jupiter.api.Test;
79+
import org.junit.jupiter.api.extension.RegisterExtension;
6180
import org.junit.jupiter.api.io.TempDir;
81+
import org.slf4j.event.Level;
6282

6383
class UpdaterTest {
6484

@@ -69,6 +89,9 @@ class UpdaterTest {
6989
@TempDir Path localStorePath;
7090
@TempDir static Path localMirrorPath;
7191

92+
@RegisterExtension
93+
LogCapturer logs = LogCapturer.create().captureForType(Updater.class, Level.DEBUG);
94+
7295
@BeforeAll
7396
static void startRemoteResourceServer() throws Exception {
7497
remote = new Server();
@@ -142,6 +165,31 @@ public void testRootUpdate_newRootHasUnknownFields() throws Exception {
142165
assertEquals(5, root.getSignedMeta().getVersion());
143166
}
144167

168+
@Test
169+
public void testRootUpdate_newRootHasEmptySignatures() throws Exception {
170+
setupMirror("synthetic/root-update-with-empty-signature", "2.root.json");
171+
var updater = createTimeStaticUpdater(localStorePath, UPDATER_SYNTHETIC_TRUSTED_ROOT);
172+
173+
updater.updateRoot();
174+
Root root = TestResources.loadRoot(localStorePath.resolve("root.json"));
175+
assertEquals(2, root.getSignedMeta().getVersion());
176+
logs.assertContains(
177+
"TUF: ignored unverifiable signature: '' for keyid: '0b5108e406f6d2f59ef767797b314be99d35903950ba43a2d51216eeeb8da98c'");
178+
}
179+
180+
@Test
181+
public void testRootUpdate_newRootHasInvalidSignatures() throws Exception {
182+
setupMirror("synthetic/root-update-with-invalid-signature", "2.root.json");
183+
var updater = createTimeStaticUpdater(localStorePath, UPDATER_SYNTHETIC_TRUSTED_ROOT);
184+
185+
updater.updateRoot();
186+
Root root = TestResources.loadRoot(localStorePath.resolve("root.json"));
187+
assertEquals(2, root.getSignedMeta().getVersion());
188+
logs.getEvents();
189+
logs.assertContains(
190+
"TUF: ignored invalid signature: 'abcd123' for keyid: '0b5108e406f6d2f59ef767797b314be99d35903950ba43a2d51216eeeb8da98c', because 'exception decoding Hex string: String index out of range: 7'");
191+
}
192+
145193
@Test
146194
public void testRootUpdate_expiredRoot()
147195
throws IOException, NoSuchAlgorithmException, InvalidKeySpecException, InvalidKeyException {

‎sigstore-java/src/test/resources/dev/sigstore/tuf/synthetic/root-update-with-empty-signature/2.root.json

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
{
2+
"signed": {
3+
"_type": "root",
4+
"spec_version": "1.0",
5+
"version": 2,
6+
"expires": "2023-05-13T14:35:58Z",
7+
"keys": {
8+
"0b5108e406f6d2f59ef767797b314be99d35903950ba43a2d51216eeeb8da98c": {
9+
"keytype": "ecdsa-sha2-nistp256",
10+
"scheme": "ecdsa-sha2-nistp256",
11+
"keyid_hash_algorithms": [
12+
"sha256",
13+
"sha512"
14+
],
15+
"keyval": {
16+
"public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEKzH3HI+8f9hYlrwNynmWtYrdp7kT\n5B13ZcaQJd2gbMw3MXUwAMWksxAjNXXXselrztKQLKEJkj0CRPiXFhtdWg==\n-----END PUBLIC KEY-----\n"
17+
}
18+
},
19+
"7aecf5f0720acfb4fa873896ba05a2d8914f5b6ca90d26ac8bc0f1e491378740": {
20+
"keytype": "ecdsa-sha2-nistp256",
21+
"scheme": "ecdsa-sha2-nistp256",
22+
"keyid_hash_algorithms": [
23+
"sha256",
24+
"sha512"
25+
],
26+
"keyval": {
27+
"public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEs1Stkp5CNyERUPWDa9KF47KjECsx\noobAYi8NUUh5+0Rl34nYR3Y/2IQWu8l2pi9f73Qqsq3kk1cGQMCKRJu1wA==\n-----END PUBLIC KEY-----\n"
28+
}
29+
},
30+
"9354bd3deaa572ed06306ddfad457037918534ece677cf962526a6fd40112d7a": {
31+
"keytype": "ecdsa-sha2-nistp256",
32+
"scheme": "ecdsa-sha2-nistp256",
33+
"keyid_hash_algorithms": [
34+
"sha256",
35+
"sha512"
36+
],
37+
"keyval": {
38+
"public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEJsV+S1syZdtx5HjiFN5YqRAqD2By\n4R0xDtXptW+UJlJQdfQCGAHvqtpac0edkcWVREhktEqIMbCaYSd75E/JRA==\n-----END PUBLIC KEY-----\n"
39+
}
40+
},
41+
"a041140325d05d8a7643d5649a8c4296f8e6b020fb73bf83c52319b1a7230a40": {
42+
"keytype": "ecdsa-sha2-nistp256",
43+
"scheme": "ecdsa-sha2-nistp256",
44+
"keyid_hash_algorithms": [
45+
"sha256",
46+
"sha512"
47+
],
48+
"keyval": {
49+
"public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEEoZaB1Hu8VvuqgHvwX1mAITts2Zi\ntHhs3suizfA/XDmetnA9BoXhPpLmPJ1n+47xr4Gdr5mcrBzLbM+WcXIs9Q==\n-----END PUBLIC KEY-----\n"
50+
}
51+
},
52+
"a9c5c80b93210eeb34e6264b4b261ff6899d4dbfb8e308f8546722a2bae30687": {
53+
"keytype": "ecdsa-sha2-nistp256",
54+
"scheme": "ecdsa-sha2-nistp256",
55+
"keyid_hash_algorithms": [
56+
"sha256",
57+
"sha512"
58+
],
59+
"keyval": {
60+
"public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbGNtqWi9Xu7romi12qG+fHYj4SCp\nUCKAOJxXKagVyQNlS6TdJCMHWOJ+0BReT1lQsw6J/SMtc9a5J6Vj7fksBw==\n-----END PUBLIC KEY-----\n"
61+
}
62+
},
63+
"fca39ff47a3a91605f2c56501e84b4fe3b9a66b96a022275e866bd19353f93e6": {
64+
"keytype": "ecdsa-sha2-nistp256",
65+
"scheme": "ecdsa-sha2-nistp256",
66+
"keyid_hash_algorithms": [
67+
"sha256",
68+
"sha512"
69+
],
70+
"keyval": {
71+
"public": "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEfcbhZ0zElnB5dqJBzKiVlofRXBh/\n2snZw32WDcUvl3+7UEtRvmTGZSaAxYCGmAc1EO2j5MGk5wkNkuwiVesd0g==\n-----END PUBLIC KEY-----\n"
72+
}
73+
}
74+
},
75+
"roles": {
76+
"root": {
77+
"keyids": [
78+
"fca39ff47a3a91605f2c56501e84b4fe3b9a66b96a022275e866bd19353f93e6",
79+
"0b5108e406f6d2f59ef767797b314be99d35903950ba43a2d51216eeeb8da98c",
80+
"a041140325d05d8a7643d5649a8c4296f8e6b020fb73bf83c52319b1a7230a40"
81+
],
82+
"threshold": 2
83+
},
84+
"snapshot": {
85+
"keyids": [
86+
"9354bd3deaa572ed06306ddfad457037918534ece677cf962526a6fd40112d7a"
87+
],
88+
"threshold": 1
89+
},
90+
"targets": {
91+
"keyids": [
92+
"a9c5c80b93210eeb34e6264b4b261ff6899d4dbfb8e308f8546722a2bae30687"
93+
],
94+
"threshold": 1
95+
},
96+
"timestamp": {
97+
"keyids": [
98+
"7aecf5f0720acfb4fa873896ba05a2d8914f5b6ca90d26ac8bc0f1e491378740"
99+
],
100+
"threshold": 1
101+
}
102+
},
103+
"consistent_snapshot": true
104+
},
105+
"signatures": [
106+
{
107+
"keyid": "0b5108e406f6d2f59ef767797b314be99d35903950ba43a2d51216eeeb8da98c",
108+
"sig": ""
109+
},
110+
{
111+
"keyid": "a041140325d05d8a7643d5649a8c4296f8e6b020fb73bf83c52319b1a7230a40",
112+
"sig": "3046022100845e6b95ccf906b7c44e5993384ecca0efefb0ce9495e9d125856ef4640c5906022100fc4ae0c7f5d32dcccb76b87240f8795d176b10497cced966aac4b8e3db71d0fa"
113+
},
114+
{
115+
"keyid": "fca39ff47a3a91605f2c56501e84b4fe3b9a66b96a022275e866bd19353f93e6",
116+
"sig": "3045022024637aad4a82ec9416527d2bd54255c56b86ff0c1a8a316d0282ce8f0e18d797022100f51cffa088083bc3c76fe0a26746b99bf49a3b19c4692a12133872a477b6f226"
117+
}
118+
]
119+
}

‎sigstore-java/src/test/resources/dev/sigstore/tuf/synthetic/root-update-with-empty-signature/README.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# Setup test data
2+
3+
```shell
4+
cp ../test-template/2.root.json 2.root.json
5+
```
6+
7+
edit the values of signatures so they are wrong, but still match the threshold
8+
```diff
9+
"signatures": [
10+
{
11+
"keyid": "0b5108e406f6d2f59ef767797b314be99d35903950ba43a2d51216eeeb8da98c",
12+
+ "sig": ""
13+
- "sig": "304502204ee7d150bbbf40dc641d1a208be4708be14022da6a86883d2c5a7282eda2659802210095a15450c1e63ff20bd5164979007fbea8a7deea68ebba7a67f8cd2901b686ca"
14+
},
15+
```

0 commit comments

Comments
 (0)