Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameter 'strictNoSignature' to make missing signatures explicit in keys map #44

Merged
merged 12 commits into from
Nov 15, 2019
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/main/java/org/simplify4u/plugins/ArtifactInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,8 @@ private boolean isMatchPattern(Pattern pattern, String str) {
public boolean isKeyMatch(PGPPublicKey key) {
return keyInfo.isKeyMatch(key);
}

public boolean isNoKey() {
return keyInfo.isNoKey();
}
}
8 changes: 8 additions & 0 deletions src/main/java/org/simplify4u/plugins/KeyInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ public KeyInfo(String strKeys) {
throw new IllegalArgumentException("null key not allowed");
}

if (strKeys.trim().isEmpty()) {
return;
}

for (String key : strKeys.split(",")) {
key = key.trim();
if (key.startsWith("0x")) {
Expand Down Expand Up @@ -97,4 +101,8 @@ private boolean compareArrays(byte[] keyBytes, byte[] fingerprint) {
}
return true;
}

public boolean isNoKey() {
return keysID.isEmpty();
}
}
13 changes: 11 additions & 2 deletions src/main/java/org/simplify4u/plugins/KeysMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ public void load(String locale) throws ResourceNotFoundException, IOException {
}
}

public boolean isNoKey(Artifact artifact) {
for (ArtifactInfo artifactInfo : keysMapList) {
if (artifactInfo.isMatch(artifact)) {
return artifactInfo.isNoKey();
}
}
return false;
}

public boolean isValidKey(Artifact artifact, PGPPublicKey key) {
if (keysMapList.isEmpty()) {
return true;
Expand All @@ -80,12 +89,12 @@ private Map<String, String> loadKeysMap(final InputStream inputStream)
if (!currentLine.isEmpty() && !isCommentLine(currentLine)) {
final String[] parts = currentLine.split("=");

if (parts.length != 2) {
if (parts.length > 2) {
throw new IllegalArgumentException(
"Property line is malformed: " + currentLine);
}

keysMaps.put(parts[0], parts[1]);
keysMaps.put(parts[0], parts.length == 1 ? "" : parts[1]);
}
}

Expand Down
31 changes: 31 additions & 0 deletions src/main/java/org/simplify4u/plugins/PGPSignatures.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.simplify4u.plugins;
cobratbq marked this conversation as resolved.
Show resolved Hide resolved

import org.bouncycastle.openpgp.PGPSignature;

import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;

/**
* Utilities for PGP Signature class.
*/
final class PGPSignatures {

/**
* Read the content of a file into the PGP signature instance (for verification).
*
* @param signature the PGP signature instance. The instance is expected to be initialized.
* @param file the file to read
* @throws IOException In case of failure to open the file or failure while reading its content.
*/
static void readFileContentInto(final PGPSignature signature, final File file) throws IOException {
try (InputStream inArtifact = new BufferedInputStream(new FileInputStream(file))) {
int t;
while ((t = inArtifact.read()) >= 0) {
signature.update((byte) t);
}
}
}
}
91 changes: 65 additions & 26 deletions src/main/java/org/simplify4u/plugins/PGPVerifyMojo.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.simplify4u.plugins;

import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -79,6 +78,9 @@
@Mojo(name = "check", requiresProject = true, requiresDependencyResolution = ResolutionScope.TEST,
defaultPhase = LifecyclePhase.VALIDATE)
public class PGPVerifyMojo extends AbstractMojo {

private static final String PGP_VERIFICATION_RESULT_FORMAT = "%s PGP Signature %s\n KeyId: 0x%X UserIds: %s";

@Parameter(property = "project", readonly = true, required = true)
private MavenProject project;

Expand Down Expand Up @@ -132,6 +134,23 @@ public class PGPVerifyMojo extends AbstractMojo {
@Parameter(property = "pgpverify.failNoSignature", defaultValue = "false")
private boolean failNoSignature;

/**
* Fail the build if any artifact without key is not present in the keys map.
* <p>
* When enabled, PGPVerify will look up all artifacts in the <code>keys
* map</code>. Unsigned artifacts will need to be present in the keys map
* but are expected to have no public key, i.e. an empty string.
* <p>
* When <code>strictNoSignature</code> is enabled, PGPVerify will no longer
* output warnings when unsigned artifacts are encountered. Instead, it will
* check if the unsigned artifact is listed in the <code>keys map</code>. If
* so it will proceed, if not it will fail the build.
*
* @since 1.5.0
*/
@Parameter(property = "pgpverify.strictNoSignature", defaultValue = "false")
private boolean strictNoSignature;
cobratbq marked this conversation as resolved.
Show resolved Hide resolved

/**
* Fail the build if any dependency has a weak signature.
*
Expand Down Expand Up @@ -205,7 +224,10 @@ public class PGPVerifyMojo extends AbstractMojo {
* wildcard.</p>
*
* <p><code>pgpKey</code> must be written as hex number starting with 0x.
* You can use <code>*</code> or <code>any</code> for match any pgp key.</p>
* You can use <code>*</code> or <code>any</code> for match any pgp key.
* If the pgpKey is an empty string, pgp-verify will expect the package to
* be unsigned. Please refer to <code>strictNoSignature</code> configuration
* parameter for its use.</p>
*
* <p>You can also omit <code>version</code> and <code>artifactId</code> which means any value
* for those fields.</p>
Expand Down Expand Up @@ -356,7 +378,7 @@ private void verifyArtifacts(Set<Artifact> artifacts)
for (Artifact artifact : artifacts) {
final Artifact ascArtifact = resolveAscArtifact(artifact);

if (ascArtifact != null) {
if (ascArtifact != null || strictNoSignature) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this logic to resolveAscArtifact method - so we don't need null to map

artifactToAsc.put(artifact, ascArtifact);
}
}
Expand Down Expand Up @@ -391,7 +413,7 @@ private Artifact resolveAscArtifact(Artifact artifact) throws MojoExecutionExcep
if (failNoSignature) {
getLog().error("No signature for " + artifact.getId());
throw new MojoExecutionException("No signature for " + artifact.getId());
} else {
} else if (!strictNoSignature) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call verifySignatureUnavailable in this place and throw exception - so only this place will be changed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this will result in build failing at the first unlisted, unsigned artifact. So you lose the ability to list all unsigned artifacts, so the user can solve them in one go. Your thoughts?

getLog().warn("No signature for " + artifact.getId());
}
}
Expand Down Expand Up @@ -481,6 +503,9 @@ private void verifyArtifactSignatures(Map<Artifact, Artifact> artifactToAsc)

private boolean verifyPGPSignature(Artifact artifact, Artifact ascArtifact)
throws MojoFailureException {
if (ascArtifact == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be not necessary if we don't put nulls

return verifySignatureUnavailable(artifact);
}
final File artifactFile = artifact.getFile();
final File signatureFile = ascArtifact.getFile();
final Map<Integer, String> weakSignatures = ImmutableMap.<Integer, String>builder()
Expand All @@ -504,6 +529,17 @@ private boolean verifyPGPSignature(Artifact artifact, Artifact ascArtifact)
}
PGPSignature pgpSignature = sigList.get(0);

if (weakSignatures.containsKey(pgpSignature.getHashAlgorithm())) {
final String logMessageWeakSignature = "Weak signature algorithm used: "
+ weakSignatures.get(pgpSignature.getHashAlgorithm());
if (failWeakSignature) {
getLog().error(logMessageWeakSignature);
throw new MojoFailureException(logMessageWeakSignature);
} else {
getLog().warn(logMessageWeakSignature);
}
}

PGPPublicKey publicKey = pgpKeysCache.getKey(pgpSignature.getKeyID());

if (!keysMap.isValidKey(artifact, publicKey)) {
Expand All @@ -514,37 +550,18 @@ private boolean verifyPGPSignature(Artifact artifact, Artifact ascArtifact)
}

pgpSignature.init(new BcPGPContentVerifierBuilderProvider(), publicKey);

try (InputStream inArtifact = new BufferedInputStream(new FileInputStream(artifactFile))) {

int t;
while ((t = inArtifact.read()) >= 0) {
pgpSignature.update((byte) t);
}
}

String msgFormat = "%s PGP Signature %s\n KeyId: 0x%X UserIds: %s";
PGPSignatures.readFileContentInto(pgpSignature, artifactFile);
if (pgpSignature.verify()) {
final String logMessageOK = String.format(msgFormat, artifact.getId(),
final String logMessageOK = String.format(PGP_VERIFICATION_RESULT_FORMAT, artifact.getId(),
"OK", publicKey.getKeyID(), Lists.newArrayList(publicKey.getUserIDs()));
if (quiet) {
getLog().debug(logMessageOK);
} else {
getLog().info(logMessageOK);
}
if (weakSignatures.containsKey(pgpSignature.getHashAlgorithm())) {
final String logMessageWeakSignature = "Weak signature algorithm used: "
+ weakSignatures.get(pgpSignature.getHashAlgorithm());
if (failWeakSignature) {
getLog().error(logMessageWeakSignature);
throw new MojoFailureException(logMessageWeakSignature);
} else {
getLog().warn(logMessageWeakSignature);
}
}
return true;
} else {
getLog().warn(String.format(msgFormat, artifact.getId(),
getLog().warn(String.format(PGP_VERIFICATION_RESULT_FORMAT, artifact.getId(),
"ERROR", publicKey.getKeyID(), Lists.newArrayList(publicKey.getUserIDs())));
getLog().warn(artifactFile.toString());
getLog().warn(signatureFile.toString());
Expand All @@ -556,6 +573,28 @@ private boolean verifyPGPSignature(Artifact artifact, Artifact ascArtifact)
}
}

/**
* Verify if unsigned artifact is correctly listed in keys map.
*
* @param artifact the artifact which is supposedly unsigned
* @return Returns <code>true</code> if correctly missing according to keys map,
* or <code>false</code> if verification fails.
*/
private boolean verifySignatureUnavailable(final Artifact artifact) {
if (keysMap.isNoKey(artifact)) {
final String logMessage = String.format("%s PGP Signature unavailable, consistent with keys map.",
artifact.getId());
if (quiet) {
getLog().debug(logMessage);
} else {
getLog().info(logMessage);
}
return true;
}
getLog().error("Unsigned artifact not listed in keys map: " + artifact.getId());
return false;
}

/**
* Indicates whether or not an artifact should be skipped, based on the configuration of this
* mojo.
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/org/simplify4u/plugins/KeyInfoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

import static org.simplify4u.plugins.TestUtils.getPGPgpPublicKey;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
import static org.testng.AssertJUnit.assertFalse;

/**
* @author Slawomir Jaranowski.
Expand Down Expand Up @@ -47,4 +49,18 @@ public void testIsKeyMatch(String strKeys, long key, boolean match) throws Excep
KeyInfo keyInfo = new KeyInfo(strKeys);
assertEquals(keyInfo.isKeyMatch(getPGPgpPublicKey(key)), match);
}

@Test
public void testIsNoKey() {

KeyInfo keyInfo = new KeyInfo("");
assertTrue(keyInfo.isNoKey());
}

@Test
public void testIsNoKeyIncorrect() {

KeyInfo keyInfo = new KeyInfo("0x123456789abcdef0");
assertFalse(keyInfo.isNoKey());
}
}
16 changes: 16 additions & 0 deletions src/test/java/org/simplify4u/plugins/KeysMapTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,20 @@ public void keysProcessedInEncounterOrder() throws Exception {
getArtifact("test", "test-package", "1.0.0"),
getPGPgpPublicKey(0xA6ADFC93EF34893EL)));
}

@Test
public void artifactsWithoutKeysProcessed() throws Exception {
keysMap.load("/keysMap3.list");

assertTrue(
keysMap.isNoKey(
getArtifact("test", "test-package", "1.0.0")));
assertFalse(
keysMap.isValidKey(
getArtifact("test", "test-package", "1.0.0"),
getPGPgpPublicKey(0xA6ADFC93EF34893EL)));
assertFalse(
keysMap.isNoKey(
getArtifact("test", "test-package-2", "1.0.0")));
}
}
17 changes: 17 additions & 0 deletions src/test/resources/keysMap3.list
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#
# Copyright 2015 Slawomir Jaranowski
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
test:test-package:*=
test:test-package-2:*=0xA6ADFC93EF34893E