[java-identity-provider] 06/06: IDP-1595 Add signature checking
Rod Widdowson
rdw at steadingsoftware.com
Wed May 20 16:05:02 UTC 2020
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch dev/IDP-1595
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=f096dbb025f63a416f4c8dbf6c6827faf7120b59
commit f096dbb025f63a416f4c8dbf6c6827faf7120b59
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Wed May 20 17:03:13 2020 +0100
IDP-1595 Add signature checking
https://issues.shibboleth.net/jira/browse/IDP-1595
This involved a complete rewrite of the load/save code because
of "weirdness" in the BC key ring code. TL;DR do each public
key one at at time. Also BC's names are weird.
---
.../idp/installer/plugin/impl/TrustStore.java | 118 +++++++++++++++++----
.../idp/installer/plugin/impl/TrustStoreTest.java | 52 ++++++++-
.../net/shibboleth/idp/installer/plugin/shib.ico | Bin 0 -> 3638 bytes
.../idp/installer/plugin/shib.ico.asc.bad | 17 +++
4 files changed, 165 insertions(+), 22 deletions(-)
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 4ca82cbfc..228ca94ac 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
@@ -23,11 +23,15 @@ import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
+import java.util.ArrayList;
import java.util.Collections;
+import java.util.Iterator;
import javax.annotation.Nonnull;
+import org.bouncycastle.bcpg.ArmoredOutputStream;
import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPObjectFactory;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPPublicKeyRingCollection;
@@ -36,12 +40,14 @@ import org.bouncycastle.openpgp.PGPSignatureList;
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.slf4j.Logger;
import org.slf4j.LoggerFactory;
import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
/**
* Code to handle (load, update, check) the trust store for an individual plugin.
@@ -50,7 +56,7 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
public final class TrustStore extends AbstractInitializableComponent {
/** logger. */
- @NonnullAfterInit private final Logger log = LoggerFactory.getLogger(TrustStore.class);
+ @Nonnull private final Logger log = LoggerFactory.getLogger(TrustStore.class);
/** Where the IdP is installed. */
@NonnullAfterInit private String idpHome;
@@ -71,6 +77,7 @@ public final class TrustStore extends AbstractInitializableComponent {
* @param what The id to set.
*/
public void setPluginId(final String what) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
pluginId = what;
}
@@ -78,6 +85,7 @@ public final class TrustStore extends AbstractInitializableComponent {
* @param what The idpHome to set.
*/
public void setIdpHome(final String what) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
idpHome = what;
}
@@ -89,11 +97,26 @@ public final class TrustStore extends AbstractInitializableComponent {
*/
protected void loadStore() throws IOException {
try (final InputStream in = Files.newInputStream(store);
- final InputStream in2 = PGPUtil.getDecoderStream(in)) {
- keyRings = new PGPPublicKeyRingCollection(in2, new JcaKeyFingerprintCalculator());
- } catch (final PGPException e) {
- throw new IOException("Bad keystore", e);
+ 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);
+ }
}
@@ -109,20 +132,44 @@ public final class TrustStore extends AbstractInitializableComponent {
} catch (final PGPException e) {
throw new IOException("Bad keystore", e);
}
- saveStore();
+ saveStoreInternal();
}
-
+
/** Save the store to its designated location.
- *
+ *
* @throws IOException from {@link Files#newOutputStream(Path, java.nio.file.OpenOption...)} and
* from {@link PGPPublicKeyRingCollection#encode(OutputStream)}
*/
public void saveStore() throws IOException {
+ ComponentSupport.ifNotInitializedThrowUninitializedComponentException(this);
+ saveStoreInternal();
+ }
+
+ /** Save the store to its designated location.
+ *
+ * @throws IOException from {@link Files#newOutputStream(Path, java.nio.file.OpenOption...)} and
+ * from {@link PGPPublicKeyRingCollection#encode(OutputStream)}
+ */
+ public void saveStoreInternal() throws IOException {
if (Files.exists(store)) {
Files.copy(store, backup, StandardCopyOption.REPLACE_EXISTING);
}
- try (final OutputStream out = Files.newOutputStream(store)) {
- keyRings.encode(out);
+ try (final OutputStream outStream = Files.newOutputStream(store)) {
+ final Iterator<PGPPublicKeyRing> kit = keyRings.getKeyRings();
+ while (kit.hasNext()) {
+ final PGPPublicKey kr = kit.next().getPublicKey();
+
+ final StringBuffer comment = new StringBuffer().append("\n\r");
+ final Iterator<String> sit = kr.getUserIDs();
+ if (sit.hasNext()) {
+ comment .append(sit.next()).append('\t');
+ }
+ comment.append("id\t").append(String.format("%X", (int) kr.getKeyID())).append("\n\r");
+ outStream.write(comment.toString().getBytes());
+ try (OutputStream armed = new ArmoredOutputStream(outStream)) {
+ kr.encode(armed);
+ }
+ }
}
}
@@ -140,17 +187,42 @@ public final class TrustStore extends AbstractInitializableComponent {
* @return whether it is there
*/
public boolean contains(final Signature signature) {
-
+
final PGPSignature sig = signature.getSignature();
-
- for (final PGPPublicKeyRing keyRing : keyRings) {
- for (final PGPPublicKey key : keyRing) {
- if (sig.getKeyID() == key.getKeyID()) {
- return true;
- }
+
+ log.debug("Looking for key with Id {}", sig.toString());
+
+ try {
+ return keyRings.getPublicKey(sig.getKeyID()) != null;
+ } catch (final PGPException e) {
+ log.warn("Error looking for key {}", signature.toString(), e);
+ return false;
+ }
+ }
+
+ /** Run a signature check over the streams.
+ * @param input what to check
+ * @param signature what to check with
+ * @return whether it passed or not
+ * @throws IOException if we get an error reading the stream
+ */
+ public boolean checkSignature(final InputStream input, final Signature signature) throws IOException {
+ try {
+ final PGPSignature pgpSignature = signature.getSignature();
+ final PGPPublicKey pubKey = keyRings.getPublicKey(pgpSignature.getKeyID());
+ pgpSignature.init(new JcaPGPContentVerifierBuilderProvider().setProvider("BC"), pubKey);
+
+ final byte[] buffer = new byte[1024];
+ int count = input.read(buffer);
+ while (count > 0) {
+ pgpSignature.update(buffer, 0, count);
+ count = input.read(buffer);
}
+ return pgpSignature.verify();
+ } catch (final PGPException e) {
+ log.warn("Error thrown during signature check", e);
+ return false;
}
- return false;
}
/** {@inheritDoc} */
@@ -197,6 +269,10 @@ public final class TrustStore extends AbstractInitializableComponent {
/** What we are hiding. */
@Nonnull private PGPSignature signature;
+
+ /** printable key. */
+ @Nonnull private String keyId;
+
protected Signature(final @Nonnull InputStream input) throws IOException {
try (final InputStream sigStream = PGPUtil.getDecoderStream(input)) {
@@ -209,10 +285,16 @@ public final class TrustStore extends AbstractInitializableComponent {
throw new IOException("Provided file was not a signature");
}
}
+ keyId = String.format("%X", (int)signature.getKeyID());
}
protected PGPSignature getSignature() {
return signature;
}
+
+ /** {@inheritDoc} */
+ public String toString() {
+ return keyId;
+ }
}
}
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 88252ce7e..68b1f7982 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
@@ -28,7 +28,9 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.SimpleFileVisitor;
import java.nio.file.attribute.BasicFileAttributes;
+import java.security.Security;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
import org.testng.annotations.AfterClass;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
@@ -47,6 +49,8 @@ public class TrustStoreTest {
@BeforeClass public void setup() throws IOException {
dir = Files.createTempDirectory("TrustStoreTest");
+ Files.createDirectories(dir.resolve("credentials").resolve(pluginId));
+ Security.addProvider(new BouncyCastleProvider());
}
@AfterClass public void teardown() throws IOException {
@@ -81,12 +85,15 @@ public class TrustStoreTest {
}
}
- @Test public void signaturePresentTest() throws ComponentInitializationException, IOException {
+ private void populateKeyStore() throws IOException {
try (final InputStream trustStream = TrustStoreTest.class.getResourceAsStream("/net/shibboleth/idp/installer/plugin/keys.txt");
- final OutputStream outStream = Files.newOutputStream(dir.resolve("credentials").resolve(pluginId).resolve("truststore.asc"))) {
- trustStream.transferTo(outStream);
+ final OutputStream outStream = Files.newOutputStream(dir.resolve("credentials").resolve(pluginId).resolve("truststore.asc"))) {
+ trustStream.transferTo(outStream);
}
-
+ }
+
+ @Test public void signaturePresentTest() throws ComponentInitializationException, IOException {
+ populateKeyStore();
final TrustStore ts = new TrustStore();
ts.setIdpHome(dir.toString());
ts.setPluginId(pluginId);
@@ -97,4 +104,41 @@ public class TrustStoreTest {
}
}
+ @Test public void signingTest() throws ComponentInitializationException, IOException {
+ populateKeyStore();
+ final TrustStore ts = new TrustStore();
+ ts.setIdpHome(dir.toString());
+ ts.setPluginId(pluginId);
+ ts.initialize();
+ try( final InputStream sigStream = TrustStoreTest.class.getResourceAsStream("/net/shibboleth/idp/installer/plugin/shib.ico.asc");
+ final InputStream badSigStream = TrustStoreTest.class.getResourceAsStream("/net/shibboleth/idp/installer/plugin/shib.ico.asc.bad");
+ final InputStream dataStream = TrustStoreTest.class.getResourceAsStream("/net/shibboleth/idp/installer/plugin/shib.ico");
+ final InputStream dataStream2 = TrustStoreTest.class.getResourceAsStream("/net/shibboleth/idp/installer/plugin/shib.ico")) {
+
+ Signature badSig = TrustStore.signatureOf(badSigStream);
+ assertTrue(ts.contains(badSig));
+ assertFalse(ts.checkSignature(dataStream, badSig));
+ assertTrue(ts.checkSignature(dataStream2, TrustStore.signatureOf(sigStream)));
+ }
+ }
+
+ @Test public void loadSave() throws IOException, ComponentInitializationException {
+ populateKeyStore();
+ final Signature signature;
+ try( final InputStream sigStream = TrustStoreTest.class.getResourceAsStream("/net/shibboleth/idp/installer/plugin/shib.ico.asc")) {
+ signature = TrustStore.signatureOf(sigStream);
+ }
+ TrustStore ts = new TrustStore();
+ ts.setIdpHome(dir.toString());
+ ts.setPluginId(pluginId);
+ ts.initialize();
+ assertTrue(ts.contains(signature));
+ ts.saveStore();
+
+ ts = new TrustStore();
+ ts.setIdpHome(dir.toString());
+ ts.setPluginId(pluginId);
+ ts.initialize();
+ assertTrue(ts.contains(signature));
+ }
}
diff --git a/idp-installer/src/test/resources/net/shibboleth/idp/installer/plugin/shib.ico b/idp-installer/src/test/resources/net/shibboleth/idp/installer/plugin/shib.ico
new file mode 100644
index 000000000..e60a6dfd2
Binary files /dev/null and b/idp-installer/src/test/resources/net/shibboleth/idp/installer/plugin/shib.ico differ
diff --git a/idp-installer/src/test/resources/net/shibboleth/idp/installer/plugin/shib.ico.asc.bad b/idp-installer/src/test/resources/net/shibboleth/idp/installer/plugin/shib.ico.asc.bad
new file mode 100644
index 000000000..536338486
--- /dev/null
+++ b/idp-installer/src/test/resources/net/shibboleth/idp/installer/plugin/shib.ico.asc.bad
@@ -0,0 +1,17 @@
+-----BEGIN PGP SIGNATURE-----
+Version: GnuPG v2.0.22 (GNU/Linux)
+
+iQIcBAABCgAGBQJeZ/5yAAoJELd8Uu7CF3HdDvEQAJLIhGQc7swpgdZaeMNlWhw8
+z5uGVC7ST5WlS9L3ThERgHxrD0IBBLPVzGcvXmo4xMw6VVjr5p88tcXJvmnrj50P
+lJt+wu04QLsA7y0GGmRR+pX3hzv0A42LDpSSbkLzUU5Joa7KZLICRQPgbkYnEiew
+vLxjSCiWyde77LCnSyfSfJ08NZ4YhTVPlH9NpWrXQKdH1XFwxmFXfo/SdQ9xbTZa
+x6GF1Jr/5cbwJB4RlmEda3tMgmQbYblNXYrVtow/Se1bBK63WtEUFx7DokBYFMvz
+lA+LxB7FilUZVfi0x18DWhBs0Uz2tCnMx07PXW4Z/+V6eKtNIsO79aiauiUbZOMg
+Rvbnb45H7PfgJZQ5rtVJmJTrAarXRZ9SqIc7oapwWsT7XayVBShtawMMI2Mo9OOg
+EvhivUXZlfoHY8S26QWNlm0XWHCpbZXBG8k0vmcNRj4ujhb8HyncbZDxR+tPFz9l
+JbxVIddxbflyPnN+TAJ+bMX8BiVdweRbLdz6YKnRxu1gS7ftryD/94CEw+z30EZ7
+MO8J+9Tzf9zOwE6tQk1nQovHTRiUS7forsSWzuhAnKREvsV89HNKv2Wns77C0bRT
+OsBHC71DNNQECgVVCi33iezbAg4BMvffTmAlQJnS9VL0KsjYtamlKiZFIP30rXMS
+LBryjOwDL1XXVtRH2/2e
+=QnMa
+-----END PGP SIGNATURE-----
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list