[java-identity-provider] 01/02: IDP-1595 Code check the signature on the files
Rod Widdowson
rdw at steadingsoftware.com
Fri Jul 3 12:57:16 UTC 2020
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch master
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=9a7e2bb58dcc9dd9532ef7c44330af17db30cc99
commit 9a7e2bb58dcc9dd9532ef7c44330af17db30cc99
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon Jun 29 16:40:57 2020 +0100
IDP-1595 Code check the signature on the files
https://issues.shibboleth.net/jira/browse/IDP-1595
---
.../idp/installer/plugin/impl/PluginInstaller.java | 129 ++++++++++++++++++--
.../idp/installer/plugin/impl/TrustStore.java | 131 +++++++++++++++------
.../installer/plugin/impl/PluginInstallerTest.java | 12 ++
.../idp/installer/plugin/impl/TrustStoreTest.java | 4 +-
4 files changed, 232 insertions(+), 44 deletions(-)
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
index 36634e3ff..ad2074557 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
@@ -31,8 +31,11 @@ import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
+import java.util.Iterator;
import java.util.List;
+import java.util.Properties;
import java.util.ServiceLoader;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
@@ -48,9 +51,13 @@ import org.apache.tools.ant.BuildException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.base.Predicates;
+
+import net.shibboleth.idp.installer.plugin.impl.TrustStore.Signature;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.plugin.PluginDescription;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
@@ -75,9 +82,12 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
/** Where we have downloaded. */
private Path downloadDirectory;
- /** Our TrustStore. */
- private TrustStore trustStore;
-
+ /** The callback before we install a certificate into the TrustStore. */
+ @Nonnull private Predicate<String> acceptCert = Predicates.alwaysFalse();
+
+ /** The actual distribution. */
+ private Path distribution;
+
/** set IdP Home.
* @param home Where we are working from
*/
@@ -90,7 +100,14 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
*/
public void setPluginId( @Nonnull @NotEmpty final String id) {
pluginId = Constraint.isNotNull(StringSupport.trimOrNull(id), "Plugin id should be be non-null");
- }
+ }
+
+ /** Set the acceptCert predicate.
+ * @param what what to set.
+ */
+ public void setAcceptCert(final Predicate<String> what) {
+ acceptCert = Constraint.isNotNull(what, "Accept Cert Preducate should be non-null");
+ }
/** Install the plugin from the provided URL. Involves downloading
* the file and then doing a {@link #installPlugin(Path, String)}.
@@ -109,18 +126,31 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
* <li>Install from the folder</li></ul>
* @param base the directory where the files are
* @param fileName the name
+ * @throws BuildException if badness is detected.
*/
- public void installPlugin(@Nonnull final Path base,
- @Nonnull @NotEmpty final String fileName) {
- unpack(base, fileName);
+ public void installPlugin(@Nonnull final Path base,
+ @Nonnull @NotEmpty final String fileName) throws BuildException {
+ if (!Files.exists(base.resolve(fileName))) {
+ log.error("Could not find distribution {}", base.resolve(fileName));
+ throw new BuildException("Could not find distribution");
+ }
+ if (!Files.exists(base.resolve(fileName + ".asc"))) {
+ log.error("Could not find distribution {}", base.resolve(fileName + ".asc"));
+ throw new BuildException("Could not find signature for distribution");
+ }
+ unpack(base, fileName);
+ setupPluginId();
+ checkSignature(base, fileName);
+ //doInstall();
}
-
- /** Method to unpack a zip or tgz file into out {{@link #unpackDirectory}.
+
+ /** Method to unpack a zip or tgz file into out {{@link #unpackDirectory}.
* @param base Where the zip/tgz file is
* @param fileName the name.
* @throws BuildException if badness is detected.
*/
+ // CheckStyle: CyclomaticComplexity OFF
private void unpack(final Path base, final String fileName) throws BuildException {
Constraint.isNull(unpackDirectory, "cannot unpack multiple times");
try {
@@ -136,7 +166,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
continue;
}
final File output = unpackDirectory.resolve(entry.getName()).toFile();
- log.debug("Unpacking {} to {}", entry.getName(), output);
+ log.trace("Unpacking {} to {}", entry.getName(), output);
if (entry.isDirectory()) {
if (!output.isDirectory() && !output.mkdirs()) {
log.error("Failed to create directory {}", output);
@@ -154,10 +184,21 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
}
}
}
+ final Iterator<Path> contents = Files.newDirectoryStream(unpackDirectory).iterator();
+ if (!contents.hasNext()) {
+ log.error("No contents unpacked from {}", fullName);
+ throw new BuildException("Distro was empty");
+ }
+ distribution = contents.next();
+ if (contents.hasNext()) {
+ log.error("Too many packages in distributions {}", fullName);
+ throw new BuildException("Too many packages in distributions");
+ }
} catch (final IOException e) {
throw new BuildException(e);
}
}
+ // CheckStyle: CyclomaticComplexity OFF
/** does the file name end in .zip?
* @param fileName the name to consider
@@ -192,6 +233,74 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
return new TarArchiveInputStream(new GzipCompressorInputStream(inStream));
}
+ /** Look into the distribution and suck out the plugin id.
+ * @throws BuildException if badness is detected.
+ */
+ private void setupPluginId() throws BuildException {
+ final File propertyFile = distribution.resolve("bootstrap").resolve("id.property").toFile();
+ if (!propertyFile.exists()) {
+ log.error("Could not locate identity of plugin at {}", propertyFile);
+ throw new BuildException("Could not locate identity of plugin");
+ }
+ try (final InputStream inStream = new BufferedInputStream(new FileInputStream(propertyFile))) {
+ final Properties idProperties = new Properties();
+ idProperties.load(inStream);
+ final String id = StringSupport.trimOrNull(idProperties.getProperty("pluginid"));
+ if (id == null) {
+ log.error("identity property file {} did not contain 'pluginid' property", propertyFile);
+ throw new BuildException("No property in ID file");
+ }
+ setPluginId(id);
+ } catch (final IOException e) {
+ log.error("Could not load plugin identity at {}", propertyFile, e);
+ throw new BuildException(e);
+ }
+ }
+
+ /** Check the signature of the plugin.
+ * @param base Where the zip/tgz file is
+ * @param fileName the name.
+ * @throws BuildException if badness is detected.
+ */
+ private void checkSignature(final Path base, final String fileName) throws BuildException {
+ try (final InputStream sigStream = new BufferedInputStream(
+ new FileInputStream(base.resolve(fileName + ".asc").toFile()))) {
+ final TrustStore trust = new TrustStore();
+ trust.setIdpHome(idpHome);
+ trust.setPluginId(pluginId);
+ trust.initialize();
+ final Signature sig = TrustStore.signatureOf(sigStream);
+ if (!trust.contains(sig)) {
+ log.info("TrustStore does not contain signature {}", sig);
+ final File certs = distribution.resolve("bootstrap").resolve("keys.txt").toFile();
+ if (!certs.exists()) {
+ log.info("No embedded keys file, signature check fails");
+ throw new BuildException("No Certificate found to check signiture o distribution");
+ }
+ try (final InputStream keysStream = new BufferedInputStream(
+ new FileInputStream(certs))) {
+ trust.importCertificateFromStream(sig, keysStream, acceptCert);
+ }
+ if (!trust.contains(sig)) {
+ log.info("Certificate not added to Trust Store");
+ throw new BuildException("Could not check signature of distribution");
+ }
+ }
+
+ try (final InputStream distroStream = new BufferedInputStream(
+ new FileInputStream(base.resolve(fileName).toFile()))) {
+ if (!trust.checkSignature(distroStream, sig)) {
+ log.info("Signature checked for {} failed", fileName);
+ throw new BuildException("Signature check failed");
+ }
+ }
+
+ } catch (final ComponentInitializationException | IOException e) {
+ log.error("Could not manage truststore for [{}, {}] ", idpHome, pluginId, e);
+ throw new BuildException(e);
+ }
+ }
+
/**
* Return a list of the installed plugins.
* @return All the plugins.
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java
index b5fe1a5c2..bd58f3887 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/TrustStore.java
@@ -26,8 +26,10 @@ import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
+import java.util.function.Predicate;
import javax.annotation.Nonnull;
+import javax.annotation.concurrent.NotThreadSafe;
import org.bouncycastle.bcpg.ArmoredOutputStream;
import org.bouncycastle.openpgp.PGPException;
@@ -41,6 +43,7 @@ import org.bouncycastle.openpgp.PGPUtil;
import org.bouncycastle.openpgp.jcajce.JcaPGPObjectFactory;
import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator;
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentVerifierBuilderProvider;
+import org.bouncycastle.util.encoders.Hex;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -53,7 +56,7 @@ import net.shibboleth.utilities.java.support.component.ComponentSupport;
* Code to handle (load, update, check) the trust store for an individual plugin.
* a thin shim on BC.
*/
-public final class TrustStore extends AbstractInitializableComponent {
+ at NotThreadSafe public final class TrustStore extends AbstractInitializableComponent {
/** logger. */
@Nonnull private final Logger log = LoggerFactory.getLogger(TrustStore.class);
@@ -63,7 +66,7 @@ public final class TrustStore extends AbstractInitializableComponent {
/** The plugin this is the trust store for. */
@NonnullAfterInit private String pluginId;
-
+
/** The key store. */
@NonnullAfterInit private Path store;
@@ -73,15 +76,17 @@ public final class TrustStore extends AbstractInitializableComponent {
/** KeyRing. */
@NonnullAfterInit private PGPPublicKeyRingCollection keyRings;
- /** Set the pluginId.
- * @param what The id to set.
+ /** Set the pluginId.
+ *
+ * @param what to set.
*/
public void setPluginId(final String what) {
ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
pluginId = what;
}
-
- /** Set the IdPHome.
+
+ /** Set IdPHome.
+ *
* @param what The idpHome to set.
*/
public void setIdpHome(final Path what) {
@@ -89,6 +94,37 @@ public final class TrustStore extends AbstractInitializableComponent {
idpHome = what;
}
+ /** Return a store loaded from the supplied stream.
+ *
+ * @param in the stream
+ * @return a suitable store
+ * @throws IOException from {@link Files#newInputStream(Path, java.nio.file.OpenOption...)} and from
+ * {@link PGPPublicKeyRingCollection#PGPPublicKeyRingCollection(InputStream,
+ * org.bouncycastle.openpgp.operator.KeyFingerPrintCalculator)}
+ */
+ private static PGPPublicKeyRingCollection loadStoreFrom(final InputStream in) throws IOException {
+ try (final InputStream decoded = PGPUtil.getDecoderStream(in)) {
+ final ArrayList<PGPPublicKeyRing> listr = new ArrayList<>();
+
+ PGPObjectFactory pgpFact = new PGPObjectFactory(decoded, new JcaKeyFingerprintCalculator());
+ Object obj;
+ while ((obj = pgpFact.nextObject()) != null) {
+ // Inner loop - when new factories return nothing we are done
+ do {
+ if (!(obj instanceof PGPPublicKeyRing)) {
+ throw new IOException(obj.getClass().getName() + " found where PGPPublicKeyRing expected");
+ }
+ listr.add((PGPPublicKeyRing) obj);
+ obj = pgpFact.nextObject();
+ } while (obj != null);
+ pgpFact = new PGPObjectFactory(decoded, new JcaKeyFingerprintCalculator());
+ }
+ return new PGPPublicKeyRingCollection(listr);
+ } catch (final PGPException e) {
+ throw new IOException("Error reading key ring", e);
+ }
+ }
+
/** Load the store from its designated location.
*
* @throws IOException from {@link Files#newInputStream(Path, java.nio.file.OpenOption...)} and from
@@ -96,30 +132,11 @@ public final class TrustStore extends AbstractInitializableComponent {
* org.bouncycastle.openpgp.operator.KeyFingerPrintCalculator)}
*/
protected void loadStore() throws IOException {
- try (final InputStream in = Files.newInputStream(store);
- final InputStream decoded = PGPUtil.getDecoderStream(in)) {
- final ArrayList<PGPPublicKeyRing> listr = new ArrayList<>();
-
- PGPObjectFactory pgpFact = new PGPObjectFactory(decoded, new JcaKeyFingerprintCalculator());
- Object obj;
- while ((obj = pgpFact.nextObject()) != null) {
- // Inner loop - when new factories return nothing we are done
- do {
- if (!(obj instanceof PGPPublicKeyRing)) {
- throw new IOException(obj.getClass().getName() + " found where PGPPublicKeyRing expected");
- }
- listr.add((PGPPublicKeyRing) obj);
- obj = pgpFact.nextObject();
- } while (obj != null);
- pgpFact = new PGPObjectFactory(decoded, new JcaKeyFingerprintCalculator());
- }
- keyRings = new PGPPublicKeyRingCollection(listr);
- } catch (final PGPException e) {
- throw new IOException("Error reading key ring", e);
+ try (final InputStream in = Files.newInputStream(store)) {
+ keyRings = loadStoreFrom(in);
}
}
-
/** Create an empty store and save to new location.
*
* @throws IOException from {@link #saveStore()} and in the unlikely event that
@@ -173,6 +190,49 @@ public final class TrustStore extends AbstractInitializableComponent {
}
}
+ /** Load up the provided store and if the certificate is found and the
+ * Predicate allows it add it to the store which we will then save.
+ *
+ * @param sigForCert the signature we are looking for a cert for.
+ * @param certStream where to load the cert from
+ * @param accept whether we actually want to install this certificate
+ * @throws IOException if the load or save fails
+ */
+ public void importCertificateFromStream(final Signature sigForCert,
+ final InputStream certStream,
+ final Predicate<String> accept) throws IOException {
+ final PGPPublicKeyRingCollection providedStore = loadStoreFrom(certStream);
+
+ try {
+ final PGPPublicKey cert = providedStore.getPublicKey(sigForCert.getSignature().getKeyID());
+ if (cert == null) {
+ log.info("Provided certificate stream did not contain a certificate for {}", sigForCert);
+ return;
+ }
+ final StringBuilder builder = new StringBuilder("Certificate:\t").
+ append(sigForCert.toString()).
+ append("\nFingerPrint:\t").
+ append(new String(Hex.encode(cert.getFingerprint())));
+ final Iterator<String> namesIterator = cert.getUserIDs();
+ while (namesIterator.hasNext()) {
+ builder.append("\nUsername:\t").append(namesIterator.next());
+ }
+ builder.append('\n');
+ final String certInfo = builder.toString();
+ log.debug("Asking to import certificate\n {}", certInfo);
+ if (!accept.test(certInfo)) {
+ log.info("Certificate import barred by user");
+ return;
+ }
+ keyRings = PGPPublicKeyRingCollection.addPublicKeyRing(
+ keyRings,
+ new PGPPublicKeyRing(Collections.singletonList(cert)));
+ saveStoreInternal();
+ } catch (final PGPException e) {
+ log.warn("Couldn't locate certificate", e);
+ }
+ }
+
/** Provide an opaque signature object from an input stream.
* @param stream what to read.
* @return the Signature.
@@ -190,12 +250,12 @@ public final class TrustStore extends AbstractInitializableComponent {
final PGPSignature sig = signature.getSignature();
- log.debug("Looking for key with Id {}", sig.toString());
+ log.debug("Looking for key with Id {}", signature);
try {
return keyRings.getPublicKey(sig.getKeyID()) != null;
} catch (final PGPException e) {
- log.warn("Error looking for key {}", signature.toString(), e);
+ log.warn("Error looking for key {}", signature, e);
return false;
}
}
@@ -218,7 +278,13 @@ public final class TrustStore extends AbstractInitializableComponent {
pgpSignature.update(buffer, 0, count);
count = input.read(buffer);
}
- return pgpSignature.verify();
+ final boolean result = pgpSignature.verify();
+ if (result) {
+ log.debug("Signature Check Succeeded");
+ } else {
+ log.debug("Signature Check Failed");
+ }
+ return result;
} catch (final PGPException e) {
log.warn("Error thrown during signature check", e);
return false;
@@ -272,7 +338,6 @@ public final class TrustStore extends AbstractInitializableComponent {
/** printable key. */
@Nonnull private String keyId;
-
protected Signature(final @Nonnull InputStream input) throws IOException {
try (final InputStream sigStream = PGPUtil.getDecoderStream(input)) {
final JcaPGPObjectFactory factory = new JcaPGPObjectFactory(sigStream);
@@ -284,7 +349,7 @@ public final class TrustStore extends AbstractInitializableComponent {
throw new IOException("Provided file was not a signature");
}
}
- keyId = String.format("%X", (int)signature.getKeyID());
+ keyId = String.format("0X%X", signature.getKeyID());
}
protected PGPSignature getSignature() {
@@ -294,6 +359,6 @@ public final class TrustStore extends AbstractInitializableComponent {
/** {@inheritDoc} */
public String toString() {
return keyId;
- }
+ }
}
}
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
index 507cf1d2f..97c0db111 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/PluginInstallerTest.java
@@ -21,11 +21,16 @@ import static org.testng.Assert.assertEquals;
import java.io.File;
import java.io.IOException;
+import java.security.Security;
import java.util.List;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.springframework.core.io.ClassPathResource;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import com.google.common.base.Predicates;
+
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import net.shibboleth.utilities.java.support.plugin.PluginDescription;
import net.shibboleth.utilities.java.support.resource.Resource;
@@ -33,6 +38,12 @@ import net.shibboleth.utilities.java.support.resource.Resource;
@SuppressWarnings("javadoc")
public class PluginInstallerTest {
+ @BeforeClass public void setup() throws IOException {
+ if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
+ Security.addProvider(new BouncyCastleProvider());
+ }
+ }
+
@Test public void testListing() throws ComponentInitializationException, IOException {
try (final PluginInstaller inst = new PluginInstaller()) {
@@ -55,6 +66,7 @@ public class PluginInstallerTest {
@Test(enabled = false) public void testUnpackTgz() throws ComponentInitializationException, IOException {
try (final PluginInstaller inst = new PluginInstaller()) {
inst.setIdpHome(new ClassPathResource("idphome-test").getFile().toPath());
+ inst.setAcceptCert(Predicates.alwaysTrue());
inst.initialize();
final File f = new File("H:\\Perforce\\Juno\\New\\plugins\\java-idp-plugin-scripting\\nashorn-dist\\target");
inst.installPlugin(f.toPath(),"shibboleth-idp-plugin-nashorn-0.0.1-SNAPSHOT.tar.gz");
diff --git a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TrustStoreTest.java b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TrustStoreTest.java
index ff54cb1be..3cc021c31 100644
--- a/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TrustStoreTest.java
+++ b/idp-installer/src/test/java/net/shibboleth/idp/installer/plugin/impl/TrustStoreTest.java
@@ -50,7 +50,9 @@ public class TrustStoreTest {
@BeforeClass public void setup() throws IOException {
dir = Files.createTempDirectory("TrustStoreTest");
Files.createDirectories(dir.resolve("credentials").resolve(pluginId));
- Security.addProvider(new BouncyCastleProvider());
+ if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
+ Security.addProvider(new BouncyCastleProvider());
+ }
}
@AfterClass public void teardown() throws IOException {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list