Skip to content

Commit 7e1f93b

Browse files
authored
[MRESOLVER-301] Addendum (apache#436)
Just simplify, cleanup things, and use fingerprint instead of phased out keyID. --- https://issues.apache.org/jira/browse/MRESOLVER-301
1 parent e152bf3 commit 7e1f93b

File tree

7 files changed

+130
-56
lines changed

7 files changed

+130
-56
lines changed

maven-resolver-generator-gnupg/pom.xml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,8 @@
3535
<Automatic-Module-Name>org.apache.maven.resolver.generator.gnupg</Automatic-Module-Name>
3636
<Bundle-SymbolicName>${Automatic-Module-Name}</Bundle-SymbolicName>
3737

38-
<bouncycastleVersion>1.77</bouncycastleVersion>
39-
4038
<javaVersion>17</javaVersion>
39+
<bouncycastleVersion>1.77</bouncycastleVersion>
4140
</properties>
4241

4342
<dependencies>

maven-resolver-generator-gnupg/src/main/java/org/eclipse/aether/generator/gnupg/GnupgConfigurationKeys.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,13 @@ private GnupgConfigurationKeys() {}
4545
public static final boolean DEFAULT_ENABLED = false;
4646

4747
/**
48-
* The PGP KeyID, optional. If not set, first secret key found will be used.
48+
* The PGP Key fingerprint as hex string (40 characters long), optional. If not set, first secret key found will
49+
* be used.
4950
*
5051
* @configurationSource {@link RepositorySystemSession#getConfigProperties()}
51-
* @configurationType {@link Long}
52+
* @configurationType {@link String}
5253
*/
53-
public static final String CONFIG_PROP_KEY_ID = CONFIG_PROPS_PREFIX + "keyId";
54+
public static final String CONFIG_PROP_KEY_FINGERPRINT = CONFIG_PROPS_PREFIX + "keyFingerprint";
5455

5556
/**
5657
* The path to the OpenPGP transferable secret key file. If relative, is resolved from local repository root.
@@ -86,7 +87,7 @@ private GnupgConfigurationKeys() {}
8687
public static final String RESOLVER_GPG_KEY = "RESOLVER_GPG_KEY";
8788

8889
/**
89-
* Env variable name to pass in key ID.
90+
* Env variable name to pass in key fingerprint (hex encoded, 40 characters long).
9091
*/
91-
public static final String RESOLVER_GPG_KEY_ID = "RESOLVER_GPG_KEY_ID";
92+
public static final String RESOLVER_GPG_KEY_FINGERPRINT = "RESOLVER_GPG_KEY_FINGERPRINT";
9293
}

maven-resolver-generator-gnupg/src/main/java/org/eclipse/aether/generator/gnupg/GnupgSignatureArtifactGeneratorFactory.java

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,11 @@
2727
import java.io.UncheckedIOException;
2828
import java.time.LocalDateTime;
2929
import java.time.ZoneId;
30+
import java.util.Arrays;
3031
import java.util.Collection;
32+
import java.util.List;
3133
import java.util.Map;
3234
import java.util.function.Predicate;
33-
import java.util.stream.Collectors;
3435

3536
import org.bouncycastle.bcpg.SymmetricKeyAlgorithmTags;
3637
import org.bouncycastle.openpgp.*;
@@ -65,9 +66,9 @@ default byte[] loadKeyRingMaterial(RepositorySystemSession session) throws IOExc
6566
}
6667

6768
/**
68-
* Returns the key ID, or {@code null}.
69+
* Returns the key fingerprint, or {@code null}.
6970
*/
70-
default Long loadKeyId(RepositorySystemSession session) throws IOException {
71+
default byte[] loadKeyFingerprint(RepositorySystemSession session) throws IOException {
7172
return null;
7273
}
7374

@@ -93,45 +94,38 @@ public GnupgSignatureArtifactGeneratorFactory(
9394

9495
@Override
9596
public ArtifactGenerator newInstance(RepositorySystemSession session, InstallRequest request) {
96-
return createArtifactGenerator(session, request.getArtifacts());
97+
return null;
9798
}
9899

99100
@Override
100101
public ArtifactGenerator newInstance(RepositorySystemSession session, DeployRequest request) {
101-
return createArtifactGenerator(session, request.getArtifacts());
102-
}
103-
104-
@Override
105-
public float getPriority() {
106-
return 100;
107-
}
108-
109-
private ArtifactGenerator createArtifactGenerator(RepositorySystemSession session, Collection<Artifact> artifacts) {
110102
final boolean enabled = ConfigUtils.getBoolean(
111103
session, GnupgConfigurationKeys.DEFAULT_ENABLED, GnupgConfigurationKeys.CONFIG_PROP_ENABLED);
112104
if (!enabled) {
113105
return null;
114106
}
115107

116108
try {
117-
return doCreateSigner(session, artifacts, artifactPredicateFactory.newInstance(session)::hasChecksums);
109+
return doCreateArtifactGenerator(
110+
session, request.getArtifacts(), artifactPredicateFactory.newInstance(session)::hasChecksums);
118111
} catch (IOException e) {
119112
throw new UncheckedIOException(e);
120113
}
121114
}
122115

123-
private Collection<Loader> loaders(RepositorySystemSession session) {
124-
boolean interactive = ConfigUtils.getBoolean(
125-
session, ConfigurationProperties.DEFAULT_INTERACTIVE, ConfigurationProperties.INTERACTIVE);
126-
return loaders.values().stream()
127-
.filter(l -> interactive || !l.isInteractive())
128-
.collect(Collectors.toList());
116+
@Override
117+
public float getPriority() {
118+
return 100;
129119
}
130120

131-
private GnupgSignatureArtifactGenerator doCreateSigner(
121+
private GnupgSignatureArtifactGenerator doCreateArtifactGenerator(
132122
RepositorySystemSession session, Collection<Artifact> artifacts, Predicate<Artifact> artifactPredicate)
133123
throws IOException {
134-
Collection<Loader> loaders = loaders(session);
124+
boolean interactive = ConfigUtils.getBoolean(
125+
session, ConfigurationProperties.DEFAULT_INTERACTIVE, ConfigurationProperties.INTERACTIVE);
126+
List<Loader> loaders = this.loaders.values().stream()
127+
.filter(l -> interactive || !l.isInteractive())
128+
.toList();
135129

136130
byte[] keyRingMaterial = null;
137131
for (Loader loader : loaders) {
@@ -144,10 +138,10 @@ private GnupgSignatureArtifactGenerator doCreateSigner(
144138
throw new IllegalArgumentException("Key ring material not found");
145139
}
146140

147-
Long keyId = null;
141+
byte[] fingerprint = null;
148142
for (Loader loader : loaders) {
149-
keyId = loader.loadKeyId(session);
150-
if (keyId != null) {
143+
fingerprint = loader.loadKeyFingerprint(session);
144+
if (fingerprint != null) {
151145
break;
152146
}
153147
}
@@ -158,12 +152,10 @@ private GnupgSignatureArtifactGenerator doCreateSigner(
158152
new BcKeyFingerprintCalculator());
159153

160154
PGPSecretKey secretKey = null;
161-
if (keyId != null) {
162-
secretKey = pgpSecretKeyRingCollection.getSecretKey(keyId);
163-
} else {
164-
for (PGPSecretKeyRing ring : pgpSecretKeyRingCollection) {
165-
for (PGPSecretKey key : ring) {
166-
if (!key.isPrivateKeyEmpty()) {
155+
for (PGPSecretKeyRing ring : pgpSecretKeyRingCollection) {
156+
for (PGPSecretKey key : ring) {
157+
if (!key.isPrivateKeyEmpty()) {
158+
if (fingerprint == null || Arrays.equals(fingerprint, key.getFingerprint())) {
167159
secretKey = key;
168160
break;
169161
}

maven-resolver-generator-gnupg/src/main/java/org/eclipse/aether/generator/gnupg/loaders/GpgConfLoader.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.nio.file.Path;
2727
import java.nio.file.Paths;
2828

29+
import org.bouncycastle.util.encoders.Hex;
2930
import org.eclipse.aether.RepositorySystemSession;
3031
import org.eclipse.aether.generator.gnupg.GnupgConfigurationKeys;
3132
import org.eclipse.aether.generator.gnupg.GnupgSignatureArtifactGeneratorFactory;
@@ -34,7 +35,7 @@
3435
import org.slf4j.Logger;
3536
import org.slf4j.LoggerFactory;
3637

37-
import static org.eclipse.aether.generator.gnupg.GnupgConfigurationKeys.CONFIG_PROP_KEY_ID;
38+
import static org.eclipse.aether.generator.gnupg.GnupgConfigurationKeys.CONFIG_PROP_KEY_FINGERPRINT;
3839

3940
/**
4041
* Loader that looks for configuration.
@@ -78,10 +79,15 @@ public byte[] loadKeyRingMaterial(RepositorySystemSession session) throws IOExce
7879
}
7980

8081
@Override
81-
public Long loadKeyId(RepositorySystemSession session) {
82-
String keyIdStr = ConfigUtils.getString(session, null, CONFIG_PROP_KEY_ID);
83-
if (keyIdStr != null) {
84-
return Long.parseLong(keyIdStr);
82+
public byte[] loadKeyFingerprint(RepositorySystemSession session) {
83+
String keyFingerprint = ConfigUtils.getString(session, null, CONFIG_PROP_KEY_FINGERPRINT);
84+
if (keyFingerprint != null) {
85+
if (keyFingerprint.trim().length() == 40) {
86+
return Hex.decode(keyFingerprint);
87+
} else {
88+
throw new IllegalArgumentException(
89+
"Key fingerprint configuration is wrong (hex encoded, 40 characters)");
90+
}
8591
}
8692
return null;
8793
}

maven-resolver-generator-gnupg/src/main/java/org/eclipse/aether/generator/gnupg/loaders/GpgEnvLoader.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323

2424
import java.nio.charset.StandardCharsets;
2525

26+
import org.bouncycastle.util.encoders.Hex;
2627
import org.eclipse.aether.RepositorySystemSession;
27-
import org.eclipse.aether.generator.gnupg.GnupgConfigurationKeys;
2828
import org.eclipse.aether.generator.gnupg.GnupgSignatureArtifactGeneratorFactory;
2929
import org.eclipse.aether.util.ConfigUtils;
3030
import org.eclipse.sisu.Priority;
3131

32-
import static org.eclipse.aether.generator.gnupg.GnupgConfigurationKeys.RESOLVER_GPG_KEY_ID;
32+
import static org.eclipse.aether.generator.gnupg.GnupgConfigurationKeys.*;
3333

3434
/**
3535
* Loader that looks for environment variables.
@@ -48,26 +48,30 @@ public boolean isInteractive() {
4848

4949
@Override
5050
public byte[] loadKeyRingMaterial(RepositorySystemSession session) {
51-
String keyMaterial = ConfigUtils.getString(session, null, "env." + GnupgConfigurationKeys.RESOLVER_GPG_KEY);
51+
String keyMaterial = ConfigUtils.getString(session, null, "env." + RESOLVER_GPG_KEY);
5252
if (keyMaterial != null) {
5353
return keyMaterial.getBytes(StandardCharsets.UTF_8);
5454
}
5555
return null;
5656
}
5757

5858
@Override
59-
public Long loadKeyId(RepositorySystemSession session) {
60-
String keyIdStr = ConfigUtils.getString(session, null, "env." + RESOLVER_GPG_KEY_ID);
61-
if (keyIdStr != null) {
62-
return Long.parseLong(keyIdStr);
59+
public byte[] loadKeyFingerprint(RepositorySystemSession session) {
60+
String keyFingerprint = ConfigUtils.getString(session, null, "env." + RESOLVER_GPG_KEY_FINGERPRINT);
61+
if (keyFingerprint != null) {
62+
if (keyFingerprint.trim().length() == 40) {
63+
return Hex.decode(keyFingerprint);
64+
} else {
65+
throw new IllegalArgumentException(
66+
"Key fingerprint configuration is wrong (hex encoded, 40 characters)");
67+
}
6368
}
6469
return null;
6570
}
6671

6772
@Override
6873
public char[] loadPassword(RepositorySystemSession session, long keyId) {
69-
String keyPassword =
70-
ConfigUtils.getString(session, null, "env." + GnupgConfigurationKeys.RESOLVER_GPG_KEY_PASS);
74+
String keyPassword = ConfigUtils.getString(session, null, "env." + RESOLVER_GPG_KEY_PASS);
7175
if (keyPassword != null) {
7276
return keyPassword.toCharArray();
7377
}

maven-resolver-generator-gnupg/src/test/java/org/eclipse/aether/generator/gnupg/GpgSignerFactoryTest.java

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.Collections;
2727
import java.util.LinkedHashMap;
2828

29+
import org.bouncycastle.util.encoders.DecoderException;
2930
import org.eclipse.aether.ConfigurationProperties;
3031
import org.eclipse.aether.DefaultRepositorySystemSession;
3132
import org.eclipse.aether.RepositorySystemSession;
@@ -40,9 +41,7 @@
4041
import org.junit.jupiter.api.Disabled;
4142
import org.junit.jupiter.api.Test;
4243

43-
import static org.junit.jupiter.api.Assertions.assertEquals;
44-
import static org.junit.jupiter.api.Assertions.assertNotNull;
45-
import static org.junit.jupiter.api.Assertions.assertTrue;
44+
import static org.junit.jupiter.api.Assertions.*;
4645
import static org.mockito.ArgumentMatchers.any;
4746
import static org.mockito.Mockito.mock;
4847
import static org.mockito.Mockito.when;
@@ -72,12 +71,20 @@ private GnupgSignatureArtifactGeneratorFactory createFactory() throws Exception
7271
}
7372

7473
private RepositorySystemSession createSession(Path keyFilePath, String keyPass, boolean interactive) {
74+
return createSession(keyFilePath, keyPass, null, interactive);
75+
}
76+
77+
private RepositorySystemSession createSession(
78+
Path keyFilePath, String keyPass, String keyFingerprint, boolean interactive) {
7579
DefaultRepositorySystemSession session = TestUtils.newSession();
7680
session.setConfigProperty(GnupgConfigurationKeys.CONFIG_PROP_ENABLED, Boolean.TRUE);
7781
session.setConfigProperty(GnupgConfigurationKeys.CONFIG_PROP_KEY_FILE_PATH, keyFilePath.toString());
7882
if (keyPass != null) {
7983
session.setConfigProperty("env." + GnupgConfigurationKeys.RESOLVER_GPG_KEY_PASS, keyPass);
8084
}
85+
if (keyFingerprint != null) {
86+
session.setConfigProperty("env." + GnupgConfigurationKeys.RESOLVER_GPG_KEY_FINGERPRINT, keyFingerprint);
87+
}
8188
session.setConfigProperty(ConfigurationProperties.INTERACTIVE, interactive);
8289
return session;
8390
}
@@ -171,6 +178,71 @@ void signNonInteractive() throws Exception {
171178
}
172179
}
173180

181+
@Test
182+
void signNonInteractiveWithSelectedKey() throws Exception {
183+
Path keyFile =
184+
Paths.get("src/test/resources/gpg-signing/gpg-secret.key").toAbsolutePath();
185+
String keyPass = "TheBigSecret";
186+
GnupgSignatureArtifactGeneratorFactory factory = createFactory();
187+
try (ArtifactGenerator signer = factory.newInstance(
188+
createSession(keyFile, keyPass, "6D27BDA430672EC700BA7DBD0A32C01AE8785B6E", false),
189+
new DeployRequest())) {
190+
assertNotNull(signer);
191+
Path artifactPath = Paths.get("src/test/resources/gpg-signing/artifact.txt");
192+
Collection<? extends Artifact> signatures = signer.generate(Collections.singleton(
193+
new DefaultArtifact("org.apache.maven.resolver:test:1.0").setPath(artifactPath)));
194+
195+
// one signature expected for one relevant artifact
196+
assertEquals(1, signatures.size());
197+
Path signaturePath = signatures.iterator().next().getPath();
198+
199+
// cannot assert file size due OS differences, so just count the lines instead: those should be same
200+
assertEquals(8, Files.lines(signaturePath).count());
201+
202+
// TODO: validate the signature
203+
}
204+
}
205+
206+
@Test
207+
void signNonInteractiveWithSelectedKeyWrongFingerprint() throws Exception {
208+
Path keyFile =
209+
Paths.get("src/test/resources/gpg-signing/gpg-secret.key").toAbsolutePath();
210+
String keyPass = "TheBigSecret";
211+
GnupgSignatureArtifactGeneratorFactory factory = createFactory();
212+
213+
assertThrows(
214+
IllegalArgumentException.class,
215+
() -> factory.newInstance(
216+
createSession(keyFile, keyPass, "FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", false),
217+
new DeployRequest()));
218+
}
219+
220+
@Test
221+
void signNonInteractiveWithSelectedKeyMalformedFingerprint1() throws Exception {
222+
Path keyFile =
223+
Paths.get("src/test/resources/gpg-signing/gpg-secret.key").toAbsolutePath();
224+
String keyPass = "TheBigSecret";
225+
GnupgSignatureArtifactGeneratorFactory factory = createFactory();
226+
227+
assertThrows(
228+
IllegalArgumentException.class,
229+
() -> factory.newInstance(createSession(keyFile, keyPass, "abcd", false), new DeployRequest()));
230+
}
231+
232+
@Test
233+
void signNonInteractiveWithSelectedKeyMalformedFingerprint2() throws Exception {
234+
Path keyFile =
235+
Paths.get("src/test/resources/gpg-signing/gpg-secret.key").toAbsolutePath();
236+
String keyPass = "TheBigSecret";
237+
GnupgSignatureArtifactGeneratorFactory factory = createFactory();
238+
239+
assertThrows(
240+
DecoderException.class,
241+
() -> factory.newInstance(
242+
createSession(keyFile, keyPass, "ZAZUFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", false),
243+
new DeployRequest()));
244+
}
245+
174246
/**
175247
* This test is disabled by default as it is interactive and would use Gpg Agent.
176248
* <p>

src/site/markdown/configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ under the License.
5151
| 24. | `"aether.generator.gpg.agentSocketLocations"` | `String` | The GnuPG agent socket(s) to try. Comma separated list of socket paths. If relative, will be resolved from user home directory. | `".gnupg/S.gpg-agent"` | 2.0.0 | No | Session Configuration |
5252
| 25. | `"aether.generator.gpg.enabled"` | `Boolean` | Whether GnuPG signer is enabled. | `false` | 2.0.0 | No | Session Configuration |
5353
| 26. | `"aether.generator.gpg.keyFilePath"` | `String` | The path to the OpenPGP transferable secret key file. If relative, is resolved from local repository root. | `"maven-signing-key.key"` | 2.0.0 | No | Session Configuration |
54-
| 27. | `"aether.generator.gpg.keyId"` | `Long` | The PGP KeyID, optional. If not set, first secret key found will be used. | - | 2.0.0 | No | Session Configuration |
54+
| 27. | `"aether.generator.gpg.keyFingerprint"` | `String` | The PGP Key fingerprint as hex string (40 characters long), optional. If not set, first secret key found will be used. | - | 2.0.0 | No | Session Configuration |
5555
| 28. | `"aether.interactive"` | `java.lang.Boolean` | A flag indicating whether interaction with the user is allowed. | `false` | | No | Session Configuration |
5656
| 29. | `"aether.layout.maven2.checksumAlgorithms"` | `java.lang.String` | Comma-separated list of checksum algorithms with which checksums are validated (downloaded) and generated (uploaded) with this layout. Resolver by default supports following algorithms: MD5, SHA-1, SHA-256 and SHA-512. New algorithms can be added by implementing ChecksumAlgorithmFactory component. | `"SHA-1,MD5"` | 1.8.0 | Yes | Session Configuration |
5757
| 30. | `"aether.lrm.enhanced.localPrefix"` | `java.lang.String` | The prefix to use for locally installed artifacts. | `"installed"` | 1.8.1 | No | Session Configuration |

0 commit comments

Comments
 (0)