From 4ce9f468466080d54c21e40c84d1b45f96d68920 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 11:26:12 +0200 Subject: [PATCH 1/8] Increase coverage of PGPCertificateDirectory --- .../InMemoryCertificateDirectoryBackend.java | 4 +++ .../cert_d/PGPCertificateDirectoryTest.java | 36 ++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/InMemoryCertificateDirectoryBackend.java b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/InMemoryCertificateDirectoryBackend.java index b3c3bd6..90797af 100644 --- a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/InMemoryCertificateDirectoryBackend.java +++ b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/InMemoryCertificateDirectoryBackend.java @@ -21,6 +21,10 @@ import java.util.Iterator; import java.util.Map; import java.util.concurrent.atomic.AtomicLong; +/** + * Implementation of the {@link PGPCertificateDirectory.Backend} which stores key material in-memory. + * It uses object locking with {@link #wait()} and {@link #notify()} to synchronize write-access. + */ public class InMemoryCertificateDirectoryBackend implements PGPCertificateDirectory.Backend { protected static class ObjectLockingMechanism implements PGPCertificateDirectory.LockingMechanism { diff --git a/pgp-cert-d-java/src/test/java/pgp/cert_d/PGPCertificateDirectoryTest.java b/pgp-cert-d-java/src/test/java/pgp/cert_d/PGPCertificateDirectoryTest.java index 8605f0e..4ffb865 100644 --- a/pgp-cert-d-java/src/test/java/pgp/cert_d/PGPCertificateDirectoryTest.java +++ b/pgp-cert-d-java/src/test/java/pgp/cert_d/PGPCertificateDirectoryTest.java @@ -70,7 +70,7 @@ public class PGPCertificateDirectoryTest { @ParameterizedTest @MethodSource("provideTestSubjects") - public void lockDirectoryAndInsertWillFail(PGPCertificateDirectory directory) + public void lockDirectoryAndTryInsertWillFail(PGPCertificateDirectory directory) throws IOException, InterruptedException, BadDataException { // Manually lock the dir assertFalse(directory.backend.getLock().isLocked()); @@ -86,6 +86,40 @@ public class PGPCertificateDirectoryTest { assertNotNull(inserted); } + @ParameterizedTest + @MethodSource("provideTestSubjects") + public void lockDirectoryAndTryInsertTrustRootWillFail(PGPCertificateDirectory directory) + throws IOException, InterruptedException, BadDataException { + // Manually lock the dir + assertFalse(directory.backend.getLock().isLocked()); + directory.backend.getLock().lockDirectory(); + assertTrue(directory.backend.getLock().isLocked()); + + KeyMaterial inserted = directory.tryInsertTrustRoot(TestKeys.getHarryKey(), merger); + assertNull(inserted); + + directory.backend.getLock().releaseDirectory(); + inserted = directory.tryInsertTrustRoot(TestKeys.getHarryKey(), merger); + assertNotNull(inserted); + } + + @ParameterizedTest + @MethodSource("provideTestSubjects") + public void lockDirectoryAndTryInsertWithSpecialNameWillFail(PGPCertificateDirectory directory) + throws IOException, InterruptedException, BadDataException, BadNameException { + // Manually lock the dir + assertFalse(directory.backend.getLock().isLocked()); + directory.backend.getLock().lockDirectory(); + assertTrue(directory.backend.getLock().isLocked()); + + Certificate inserted = directory.tryInsertWithSpecialName(SpecialNames.TRUST_ROOT, TestKeys.getHarryKey(), merger); + assertNull(inserted); + + directory.backend.getLock().releaseDirectory(); + inserted = directory.tryInsertWithSpecialName(SpecialNames.TRUST_ROOT, TestKeys.getHarryKey(), merger); + assertNotNull(inserted); + } + @ParameterizedTest @MethodSource("provideTestSubjects") public void getByInvalidNameFails(PGPCertificateDirectory directory) { From 2758c3efb87a5d0fcf385f818a3cc7286e491454 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 11:40:22 +0200 Subject: [PATCH 2/8] Add javadoc --- .../backend/FileBasedCertificateDirectoryBackend.java | 11 +++++++++++ .../subkey_lookup/InMemorySubkeyLookupFactory.java | 3 +++ .../pgp/cert_d/subkey_lookup/SubkeyLookupFactory.java | 3 +++ 3 files changed, 17 insertions(+) diff --git a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java index e5750e8..def57e6 100644 --- a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java +++ b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java @@ -35,12 +35,20 @@ import java.util.Iterator; import java.util.List; import java.util.regex.Pattern; +/** + * Implementation of {@link PGPCertificateDirectory.Backend} which stores certificates in a directory structure. + * + * @see Shared PGP Certificate Directory + */ public class FileBasedCertificateDirectoryBackend implements PGPCertificateDirectory.Backend { private abstract static class Lazy { abstract E get() throws BadDataException; } + /** + * Locking mechanism which uses a lock file to synchronize write-access to the store. + */ private static class FileLockingMechanism implements PGPCertificateDirectory.LockingMechanism { private final File lockFile; @@ -364,6 +372,9 @@ public class FileBasedCertificateDirectoryBackend implements PGPCertificateDirec return getTag(certFile); } + /** + * Class to resolve file names from certificate fingerprints / special names. + */ public static class FilenameResolver { private final File baseDirectory; diff --git a/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/InMemorySubkeyLookupFactory.java b/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/InMemorySubkeyLookupFactory.java index a224c64..98e006c 100644 --- a/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/InMemorySubkeyLookupFactory.java +++ b/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/InMemorySubkeyLookupFactory.java @@ -6,6 +6,9 @@ package pgp.cert_d.subkey_lookup; import java.io.File; +/** + * Factory class to instantiate {@link InMemorySubkeyLookup} objects. + */ public class InMemorySubkeyLookupFactory implements SubkeyLookupFactory { @Override public SubkeyLookup createFileBasedInstance(File baseDirectory) { diff --git a/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/SubkeyLookupFactory.java b/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/SubkeyLookupFactory.java index 442e9ef..a958b84 100644 --- a/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/SubkeyLookupFactory.java +++ b/pgp-cert-d-java/src/main/java/pgp/cert_d/subkey_lookup/SubkeyLookupFactory.java @@ -6,6 +6,9 @@ package pgp.cert_d.subkey_lookup; import java.io.File; +/** + * Factory class to instantiate different {@link SubkeyLookup} implementations. + */ public interface SubkeyLookupFactory { /** From 3ec7b082be76aaedb91e4a99915a00cd0c50231c Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 11:41:05 +0200 Subject: [PATCH 3/8] Prevent NPEs in file walker --- .../backend/FileBasedCertificateDirectoryBackend.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java index def57e6..1ffc550 100644 --- a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java +++ b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java @@ -217,6 +217,10 @@ public class FileBasedCertificateDirectoryBackend implements PGPCertificateDirec } }); + if (subdirectories == null) { + subdirectories = new File[0]; + } + for (File subdirectory : subdirectories) { File[] files = subdirectory.listFiles(new FileFilter() { @Override @@ -225,6 +229,10 @@ public class FileBasedCertificateDirectoryBackend implements PGPCertificateDirec } }); + if (files == null) { + files = new File[0]; + } + for (File certFile : files) { certificateQueue.add(new Lazy() { @Override From da39d41df329c5f1d5022903d3a7b7a0bd35d962 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 11:41:41 +0200 Subject: [PATCH 4/8] Remove unused method --- .../FileBasedCertificateDirectoryBackend.java | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java index 1ffc550..4d37b0d 100644 --- a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java +++ b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java @@ -434,24 +434,6 @@ public class FileBasedCertificateDirectoryBackend implements PGPCertificateDirec return new File(getBaseDirectory(), specialName); } - /** - * Calculate the file location for the key addressed using the given special name. - * For known special names, see {@link SpecialNames}. - * - * @param specialName special name (e.g. "trust-root") - * @return absolute key file location - * - * @throws BadNameException in case the given special name is not known - */ - public File getKeyFileBySpecialName(String specialName) - throws BadNameException { - if (!isSpecialName(specialName)) { - throw new BadNameException(String.format("%s is not a known special name", specialName)); - } - - return new File(getBaseDirectory(), specialName + ".key"); - } - private boolean isFingerprint(String fingerprint) { return openPgpV4FingerprintPattern.matcher(fingerprint).matches(); } From 142d1f6b16779b0ccb48e1d5d5d64723a8e5a701 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 11:41:52 +0200 Subject: [PATCH 5/8] Add support for v5 fingerprints --- .../cert_d/backend/FileBasedCertificateDirectoryBackend.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java index 4d37b0d..89e52ba 100644 --- a/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java +++ b/pgp-cert-d-java/src/main/java/pgp/cert_d/backend/FileBasedCertificateDirectoryBackend.java @@ -386,7 +386,8 @@ public class FileBasedCertificateDirectoryBackend implements PGPCertificateDirec public static class FilenameResolver { private final File baseDirectory; - private final Pattern openPgpV4FingerprintPattern = Pattern.compile("^[a-f0-9]{40}$"); + // matches v4 and v5 fingerprints (v4 = 40 hex chars, v5 = 64 hex chars) + private final Pattern openPgpFingerprint = Pattern.compile("^[a-f0-9]{40}([a-f0-9]{24})?$"); public FilenameResolver(File baseDirectory) { this.baseDirectory = baseDirectory; @@ -435,7 +436,7 @@ public class FileBasedCertificateDirectoryBackend implements PGPCertificateDirec } private boolean isFingerprint(String fingerprint) { - return openPgpV4FingerprintPattern.matcher(fingerprint).matches(); + return openPgpFingerprint.matcher(fingerprint).matches(); } private boolean isSpecialName(String specialName) { From 682b3d58fa47cad5260b1cba39c6205b82bfd46a Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 11:55:15 +0200 Subject: [PATCH 6/8] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51b270f..c5c96e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,11 @@ SPDX-License-Identifier: CC0-1.0 - Rename `CertificateMerger` to `KeyMaterialMerger` - Rework `PGPCertificateStore` class - `pgp-cert-d-java`: + - Increase minimum Android API level to 26 + - Add `PGPCertificateDirectories` factory class - Rework `PGPCertificateDirectory` class by separating out backend logic - Split interface into `ReadOnlyPGPCertificateDirectory` and `WritingPGPCertificateDirectory` + - `FileBasedCertificateDirectoryBackend`: Calculate tag based on file attributes (inode) - `pgp-cert-d-java-jdbc-sqlite-lookup`: - Add `DatabaseSubkeyLookupFactory` From 65a283c80b8c23b830aa0106f1d6d1c67593001f Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 12:00:14 +0200 Subject: [PATCH 7/8] Cert-D-Java 0.2.0 --- CHANGELOG.md | 2 +- version.gradle | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5c96e0..ad2475e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ SPDX-License-Identifier: CC0-1.0 # Cert-D-Java Changelog -## 0.1.2-SNAPSHOT +## 0.2.0 - `pgp-certificate-store`: - Rework `Certificate`, `Key` to inherit from `KeyMaterial` - Rename `CertificateReaderBackend` to `KeyMaterialReaderBackend` diff --git a/version.gradle b/version.gradle index 9223018..8e3b938 100644 --- a/version.gradle +++ b/version.gradle @@ -4,8 +4,8 @@ allprojects { ext { - shortVersion = '0.1.2' - isSnapshot = true + shortVersion = '0.2.0' + isSnapshot = false minAndroidSdk = 26 animalsnifferSignatureVersion = "$minAndroidSdk:8.0.0_r2" javaSourceCompatibility = 1.8 From eab31b8c1242196d13b228c3a0f504c2ede42e5c Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 25 Aug 2022 12:02:01 +0200 Subject: [PATCH 8/8] Cert-D-Java 0.2.1-SNAPSHOT --- version.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.gradle b/version.gradle index 8e3b938..db46d9a 100644 --- a/version.gradle +++ b/version.gradle @@ -4,8 +4,8 @@ allprojects { ext { - shortVersion = '0.2.0' - isSnapshot = false + shortVersion = '0.2.1' + isSnapshot = true minAndroidSdk = 26 animalsnifferSignatureVersion = "$minAndroidSdk:8.0.0_r2" javaSourceCompatibility = 1.8