From 8025e82b1b3a462bdf83311a7710d122f62f5133 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Mon, 8 May 2023 15:28:53 +0200 Subject: [PATCH] Use pgpainless for key generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The library pgpainless (https://gh.pgpainless.org/) makes it much more easy to create gpg keys for new users. As a benefit, these keys can be verified by GitHub. Committed-by: Konstantin Schaper Co-authored-by: René Pfeuffer --- gradle/changelog/github_gpg_verification.yaml | 2 + gradle/dependencies.gradle | 8 +- scm-webapp/build.gradle | 2 + .../sonia/scm/security/gpg/DefaultGPG.java | 15 ++-- .../scm/security/gpg/GPGKeyExporter.java | 43 ----------- .../scm/security/gpg/GPGKeyPairGenerator.java | 37 ++-------- .../scm/security/gpg/DefaultGPGTest.java | 16 ++-- .../scm/security/gpg/GPGKeyExporterTest.java | 14 ++-- .../security/gpg/GPGKeyPairGeneratorTest.java | 73 ------------------- 9 files changed, 36 insertions(+), 174 deletions(-) create mode 100644 gradle/changelog/github_gpg_verification.yaml delete mode 100644 scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyExporter.java delete mode 100644 scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyPairGeneratorTest.java diff --git a/gradle/changelog/github_gpg_verification.yaml b/gradle/changelog/github_gpg_verification.yaml new file mode 100644 index 0000000000..1a11f5dbe4 --- /dev/null +++ b/gradle/changelog/github_gpg_verification.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Automatically created gpg keys can now be verified by Github diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 3114e02a1a..67811e3e73 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -12,7 +12,7 @@ ext { shiroVersion = '1.10.0' sspVersion = '1.3.0' jjwtVersion = '0.11.5' - bouncycastleVersion = '1.67' + bouncycastleVersion = '1.73' jettyVersion = '9.4.44.v20210927' luceneVersion = '8.9.0' @@ -112,9 +112,9 @@ ext { jjwtJackson: "io.jsonwebtoken:jjwt-jackson:${jjwtVersion}", // gpg - bouncycastlePg: "org.bouncycastle:bcpg-jdk15on:${bouncycastleVersion}", - bouncycastleProv: "org.bouncycastle:bcprov-jdk15on:${bouncycastleVersion}", - bouncycastlePkix: "org.bouncycastle:bcpkix-jdk15on:${bouncycastleVersion}", + bouncycastlePg: "org.bouncycastle:bcpg-jdk15to18:${bouncycastleVersion}", + bouncycastleProv: "org.bouncycastle:bcprov-jdk15to18:${bouncycastleVersion}", + bouncycastlePkix: "org.bouncycastle:bcpkix-jdk15to18:${bouncycastleVersion}", // service registration metainfServices: 'org.kohsuke.metainf-services:metainf-services:1.8', diff --git a/scm-webapp/build.gradle b/scm-webapp/build.gradle index 20fd75ddf4..877aeb065d 100644 --- a/scm-webapp/build.gradle +++ b/scm-webapp/build.gradle @@ -73,6 +73,8 @@ dependencies { implementation libraries.bouncycastleProv implementation libraries.bouncycastlePkix + implementation 'org.pgpainless:pgpainless-core:1.5.0' + // json implementation libraries.jacksonJaxbAnnotations implementation libraries.jacksonJaxRsBase diff --git a/scm-webapp/src/main/java/sonia/scm/security/gpg/DefaultGPG.java b/scm-webapp/src/main/java/sonia/scm/security/gpg/DefaultGPG.java index 9799eac57e..2e3c4411ba 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/gpg/DefaultGPG.java +++ b/scm-webapp/src/main/java/sonia/scm/security/gpg/DefaultGPG.java @@ -27,10 +27,11 @@ package sonia.scm.security.gpg; import org.apache.shiro.SecurityUtils; import org.bouncycastle.bcpg.ArmoredInputStream; import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPKeyRingGenerator; import org.bouncycastle.openpgp.PGPObjectFactory; +import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.openpgp.PGPSignatureList; import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator; +import org.pgpainless.PGPainless; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.security.GPG; @@ -40,8 +41,8 @@ import sonia.scm.security.PublicKey; import javax.inject.Inject; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -99,18 +100,18 @@ public class DefaultGPG implements GPG { final String userId = SecurityUtils.getSubject().getPrincipal().toString(); final Optional privateRawKey = privateKeyStore.getForUserId(userId); - if (!privateRawKey.isPresent()) { + if (privateRawKey.isEmpty()) { try { - final PGPKeyRingGenerator keyPair = GPGKeyPairGenerator.generateKeyPair(); + PGPSecretKeyRing secretKeys = GPGKeyPairGenerator.generateKeyPair(); - final String rawPublicKey = GPGKeyExporter.exportKeyRing(keyPair.generatePublicKeyRing()); - final String rawPrivateKey = GPGKeyExporter.exportKeyRing(keyPair.generateSecretKeyRing()); + final String rawPublicKey = PGPainless.asciiArmor(PGPainless.extractCertificate(secretKeys)); + final String rawPrivateKey = PGPainless.asciiArmor(secretKeys); privateKeyStore.setForUserId(userId, rawPrivateKey); publicKeyStore.add("Default SCM-Manager Signing Key", userId, rawPublicKey, true); return DefaultPrivateKey.parseRaw(rawPrivateKey); - } catch (PGPException | NoSuchAlgorithmException | NoSuchProviderException | IOException e) { + } catch (PGPException | NoSuchAlgorithmException | InvalidAlgorithmParameterException | IOException e) { throw new GPGException("Private key could not be generated", e); } } else { diff --git a/scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyExporter.java b/scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyExporter.java deleted file mode 100644 index 15a21daf89..0000000000 --- a/scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyExporter.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2020-present Cloudogu GmbH and Contributors - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -package sonia.scm.security.gpg; - -import org.bouncycastle.bcpg.ArmoredOutputStream; -import org.bouncycastle.openpgp.PGPKeyRing; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; - -class GPGKeyExporter { - private GPGKeyExporter() { } - - static String exportKeyRing(PGPKeyRing keyRing) throws IOException { - final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); - final ArmoredOutputStream armoredOutputStream = new ArmoredOutputStream(byteArrayOutputStream); - keyRing.encode(armoredOutputStream); - armoredOutputStream.close(); - return new String(byteArrayOutputStream.toByteArray()); - } -} diff --git a/scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyPairGenerator.java b/scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyPairGenerator.java index 2863305485..2456eadfa8 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyPairGenerator.java +++ b/scm-webapp/src/main/java/sonia/scm/security/gpg/GPGKeyPairGenerator.java @@ -25,27 +25,17 @@ package sonia.scm.security.gpg; import org.apache.shiro.SecurityUtils; -import org.bouncycastle.bcpg.HashAlgorithmTags; -import org.bouncycastle.bcpg.PublicKeyAlgorithmTags; -import org.bouncycastle.bcpg.SymmetricKeyAlgorithmTags; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPKeyPair; -import org.bouncycastle.openpgp.PGPKeyRingGenerator; -import org.bouncycastle.openpgp.PGPSignature; -import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder; -import org.bouncycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBuilder; -import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyPair; -import org.bouncycastle.openpgp.operator.jcajce.JcePBESecretKeyEncryptorBuilder; +import org.bouncycastle.openpgp.PGPSecretKeyRing; +import org.pgpainless.PGPainless; +import org.pgpainless.key.generation.type.rsa.RsaLength; import sonia.scm.repository.Person; import sonia.scm.user.User; -import java.security.KeyPair; -import java.security.KeyPairGenerator; +import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; import java.security.Security; -import java.util.Date; final class GPGKeyPairGenerator { @@ -57,25 +47,10 @@ final class GPGKeyPairGenerator { private GPGKeyPairGenerator() {} - static PGPKeyRingGenerator generateKeyPair() throws PGPException, NoSuchProviderException, NoSuchAlgorithmException { - KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA", "BC"); - keyPairGenerator.initialize(2048); - - KeyPair pair = keyPairGenerator.generateKeyPair(); - - PGPKeyPair keyPair = new JcaPGPKeyPair(PublicKeyAlgorithmTags.RSA_GENERAL, pair, new Date()); + static PGPSecretKeyRing generateKeyPair() throws PGPException, NoSuchAlgorithmException, InvalidAlgorithmParameterException { final User user = SecurityUtils.getSubject().getPrincipals().oneByType(User.class); final Person person = new Person(user.getDisplayName(), user.getMail()); - return new PGPKeyRingGenerator( - PGPSignature.POSITIVE_CERTIFICATION, - keyPair, - person.toString(), - new JcaPGPDigestCalculatorProviderBuilder().build().get(HashAlgorithmTags.SHA1), - null, - null, - new JcaPGPContentSignerBuilder(PublicKeyAlgorithmTags.RSA_GENERAL, HashAlgorithmTags.SHA1), - new JcePBESecretKeyEncryptorBuilder(SymmetricKeyAlgorithmTags.AES_256).build(new char[]{}) - ); + return PGPainless.generateKeyRing().simpleRsaKeyRing(person.toString(), RsaLength._4096); } } diff --git a/scm-webapp/src/test/java/sonia/scm/security/gpg/DefaultGPGTest.java b/scm-webapp/src/test/java/sonia/scm/security/gpg/DefaultGPGTest.java index c8817a4d81..5a205eba91 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/gpg/DefaultGPGTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/gpg/DefaultGPGTest.java @@ -32,8 +32,8 @@ import org.apache.shiro.subject.Subject; import org.apache.shiro.util.ThreadContext; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPKeyRingGenerator; import org.bouncycastle.openpgp.PGPPrivateKey; +import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -41,14 +41,15 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.pgpainless.PGPainless; import sonia.scm.repository.Person; import sonia.scm.security.PrivateKey; import sonia.scm.security.PublicKey; import sonia.scm.util.MockUtil; import java.io.IOException; +import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; import java.security.Security; import java.time.Instant; import java.util.Collections; @@ -58,7 +59,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -81,8 +81,6 @@ class DefaultGPGTest { @InjectMocks private DefaultGPG gpg; - private Subject subjectUnderTest; - @AfterEach void unbindThreadContext() { ThreadContext.unbindSubject(); @@ -94,7 +92,7 @@ class DefaultGPGTest { registerBouncyCastleProviderIfNecessary(); SecurityUtils.setSecurityManager(new DefaultSecurityManager()); - subjectUnderTest = MockUtil.createUserSubject(SecurityUtils.getSecurityManager()); + Subject subjectUnderTest = MockUtil.createUserSubject(SecurityUtils.getSecurityManager()); ThreadContext.bind(subjectUnderTest); } @@ -141,9 +139,9 @@ class DefaultGPGTest { } @Test - void shouldImportExportedGeneratedPrivateKey() throws NoSuchProviderException, NoSuchAlgorithmException, PGPException, IOException { - final PGPKeyRingGenerator keyRingGenerator = GPGKeyPairGenerator.generateKeyPair(); - final String exportedPrivateKey = GPGKeyExporter.exportKeyRing(keyRingGenerator.generateSecretKeyRing()); + void shouldImportExportedGeneratedPrivateKey() throws NoSuchAlgorithmException, PGPException, IOException, InvalidAlgorithmParameterException { + final PGPSecretKeyRing secretKeys = GPGKeyPairGenerator.generateKeyPair(); + final String exportedPrivateKey = PGPainless.asciiArmor(secretKeys); final PGPPrivateKey privateKey = KeysExtractor.extractPrivateKey(exportedPrivateKey); assertThat(privateKey).isNotNull(); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyExporterTest.java b/scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyExporterTest.java index dccf40bd07..ecbcec316f 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyExporterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyExporterTest.java @@ -26,19 +26,19 @@ package sonia.scm.security.gpg; import org.apache.shiro.SecurityUtils; import org.apache.shiro.mgt.DefaultSecurityManager; -import org.apache.shiro.subject.Subject; import org.apache.shiro.util.ThreadContext; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPKeyRingGenerator; +import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.pgpainless.PGPainless; import sonia.scm.util.MockUtil; import java.io.IOException; +import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; import java.security.Security; import static org.assertj.core.api.Assertions.assertThat; @@ -66,16 +66,16 @@ class GPGKeyExporterTest { } @Test - void shouldExportGeneratedKeyPair() throws NoSuchProviderException, NoSuchAlgorithmException, PGPException, IOException { - final PGPKeyRingGenerator keyRingGenerator = GPGKeyPairGenerator.generateKeyPair(); + void shouldExportGeneratedKeyPair() throws NoSuchAlgorithmException, PGPException, IOException, InvalidAlgorithmParameterException { + final PGPSecretKeyRing secretKeys = GPGKeyPairGenerator.generateKeyPair(); - final String exportedPublicKey = GPGKeyExporter.exportKeyRing(keyRingGenerator.generatePublicKeyRing()); + final String exportedPublicKey = PGPainless.asciiArmor(PGPainless.extractCertificate(secretKeys)); assertThat(exportedPublicKey) .isNotBlank() .startsWith("-----BEGIN PGP PUBLIC KEY BLOCK-----") .contains("-----END PGP PUBLIC KEY BLOCK-----"); - final String exportedPrivateKey = GPGKeyExporter.exportKeyRing(keyRingGenerator.generateSecretKeyRing()); + final String exportedPrivateKey = PGPainless.asciiArmor(secretKeys); assertThat(exportedPrivateKey) .isNotBlank() .startsWith("-----BEGIN PGP PRIVATE KEY BLOCK-----") diff --git a/scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyPairGeneratorTest.java b/scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyPairGeneratorTest.java deleted file mode 100644 index e535dc26e9..0000000000 --- a/scm-webapp/src/test/java/sonia/scm/security/gpg/GPGKeyPairGeneratorTest.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2020-present Cloudogu GmbH and Contributors - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -package sonia.scm.security.gpg; - -import org.apache.shiro.SecurityUtils; -import org.apache.shiro.mgt.DefaultSecurityManager; -import org.apache.shiro.subject.Subject; -import org.apache.shiro.util.ThreadContext; -import org.assertj.core.api.Assertions; -import org.bouncycastle.jce.provider.BouncyCastleProvider; -import org.bouncycastle.openpgp.PGPException; -import org.bouncycastle.openpgp.PGPKeyRingGenerator; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import sonia.scm.util.MockUtil; - -import java.security.NoSuchAlgorithmException; -import java.security.NoSuchProviderException; -import java.security.Security; - -class GPGKeyPairGeneratorTest { - - private static void registerBouncyCastleProviderIfNecessary() { - if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) { - Security.addProvider(new BouncyCastleProvider()); - } - } - - @AfterEach - void unbindThreadContext() { - ThreadContext.unbindSubject(); - ThreadContext.unbindSecurityManager(); - } - - @BeforeEach - void bindThreadContext() { - registerBouncyCastleProviderIfNecessary(); - - SecurityUtils.setSecurityManager(new DefaultSecurityManager()); - ThreadContext.bind(MockUtil.createUserSubject(SecurityUtils.getSecurityManager())); - } - - @Test - void shouldGenerateKeyPair() throws NoSuchProviderException, NoSuchAlgorithmException, PGPException { - final PGPKeyRingGenerator keyRingGenerator = GPGKeyPairGenerator.generateKeyPair(); - Assertions.assertThat(keyRingGenerator.generatePublicKeyRing().getPublicKey()).isNotNull(); - Assertions.assertThat(keyRingGenerator.generateSecretKeyRing().getSecretKey()).isNotNull(); - } - -}