fix review findings and code smells

This commit is contained in:
Konstantin Schaper
2020-08-12 12:00:57 +02:00
parent a4d0ff2b24
commit 82f752a7fc
16 changed files with 134 additions and 108 deletions

View File

@@ -77,7 +77,7 @@ public class DefaultGPG implements GPG {
public Optional<PublicKey> findPublicKey(String id) {
Optional<RawGpgKey> key = publicKeyStore.findById(id);
return key.map(rawGpgKey -> new DefaultPublicKey(rawGpgKey.getId(), rawGpgKey.getOwner(), rawGpgKey.getRaw(), rawGpgKey.getContacts()));
return key.map(RawGpgKeyToDefaultPublicKeyMapper::map);
}
@Override
@@ -109,12 +109,12 @@ public class DefaultGPG implements GPG {
privateKeyStore.setForUserId(userId, rawPrivateKey);
publicKeyStore.add("Default SCM-Manager Signing Key", userId, rawPublicKey, true);
return new DefaultPrivateKey(rawPrivateKey);
return DefaultPrivateKey.parseRaw(rawPrivateKey);
} catch (PGPException | NoSuchAlgorithmException | NoSuchProviderException | IOException e) {
throw new GPGException("Private key could not be generated", e);
}
} else {
return new DefaultPrivateKey(privateRawKey.get());
return DefaultPrivateKey.parseRaw(privateRawKey.get());
}
}

View File

@@ -37,22 +37,26 @@ import org.bouncycastle.openpgp.PGPSignatureGenerator;
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder;
import sonia.scm.security.PrivateKey;
import javax.validation.constraints.NotNull;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.Optional;
class DefaultPrivateKey implements PrivateKey {
final Optional<PGPPrivateKey> privateKey;
static DefaultPrivateKey parseRaw(String rawPrivateKey) {
return new DefaultPrivateKey(KeysExtractor.extractPrivateKey(rawPrivateKey));
}
DefaultPrivateKey(String rawPrivateKey) {
privateKey = KeysExtractor.extractPrivateKey(rawPrivateKey);
private final PGPPrivateKey privateKey;
private DefaultPrivateKey(@NotNull PGPPrivateKey privateKey) {
this.privateKey = privateKey;
}
@Override
public String getId() {
return privateKey.map(Keys::createId).orElse(null);
return Keys.createId(privateKey);
}
@Override
@@ -64,14 +68,10 @@ class DefaultPrivateKey implements PrivateKey {
HashAlgorithmTags.SHA1).setProvider(BouncyCastleProvider.PROVIDER_NAME)
);
if (privateKey.isPresent()) {
try {
signatureGenerator.init(PGPSignature.BINARY_DOCUMENT, privateKey.get());
} catch (PGPException e) {
throw new GPGException("Could not initialize signature generator", e);
}
} else {
throw new GPGException("Missing private key");
try {
signatureGenerator.init(PGPSignature.BINARY_DOCUMENT, privateKey);
} catch (PGPException e) {
throw new GPGException("Could not initialize signature generator", e);
}
ByteArrayOutputStream buffer = new ByteArrayOutputStream();

View File

@@ -45,7 +45,9 @@ import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.util.Date;
class GPGKeyPairGenerator {
final class GPGKeyPairGenerator {
private GPGKeyPairGenerator() {}
static PGPKeyRingGenerator generateKeyPair() throws PGPException, NoSuchProviderException, NoSuchAlgorithmException {
KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA", "BC");
keyPairGenerator.initialize(2048);

View File

@@ -33,41 +33,31 @@ import org.bouncycastle.openpgp.PGPUtil;
import org.bouncycastle.openpgp.jcajce.JcaPGPSecretKeyRingCollection;
import org.bouncycastle.openpgp.operator.jcajce.JcaKeyFingerprintCalculator;
import org.bouncycastle.openpgp.operator.jcajce.JcePBESecretKeyDecryptorBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.Optional;
public class KeysExtractor {
final class KeysExtractor {
private KeysExtractor() {}
private static final Logger LOG = LoggerFactory.getLogger(KeysExtractor.class);
static Optional<PGPPrivateKey> extractPrivateKey(String rawKey) {
static PGPPrivateKey extractPrivateKey(String rawKey) {
try (final InputStream decoderStream = PGPUtil.getDecoderStream(new ByteArrayInputStream(rawKey.getBytes()))) {
JcaPGPSecretKeyRingCollection secretKeyRingCollection = new JcaPGPSecretKeyRingCollection(decoderStream);
final PGPPrivateKey privateKey = secretKeyRingCollection.getKeyRings().next().getSecretKey().extractPrivateKey(new JcePBESecretKeyDecryptorBuilder().build(new char[]{}));
return Optional.of(privateKey);
return secretKeyRingCollection.getKeyRings().next().getSecretKey().extractPrivateKey(new JcePBESecretKeyDecryptorBuilder().build(new char[]{}));
} catch (Exception e) {
LOG.error("Invalid PGP key", e);
return Optional.empty();
throw new GPGException("Invalid PGP key", e);
}
}
static Optional<PGPPublicKey> extractPublicKey(String rawKey) {
static PGPPublicKey extractPublicKey(String rawKey) {
try {
ArmoredInputStream armoredInputStream = new ArmoredInputStream(new ByteArrayInputStream(rawKey.getBytes()));
PGPObjectFactory pgpObjectFactory = new PGPObjectFactory(armoredInputStream, new JcaKeyFingerprintCalculator());
PGPPublicKey publicKey = ((PGPPublicKeyRing) pgpObjectFactory.nextObject()).getPublicKey();
return Optional.of(publicKey);
return ((PGPPublicKeyRing) pgpObjectFactory.nextObject()).getPublicKey();
} catch (IOException e) {
LOG.error("Invalid PGP key", e);
throw new GPGException("Invalid PGP key", e);
}
return Optional.empty();
}
}

View File

@@ -84,7 +84,7 @@ public class PublicKeyStore {
RawGpgKey key = new RawGpgKey(master, displayName, username, rawKey, getContactsFromPublicKey(rawKey), Instant.now(), readonly);
store.put(master, key);
eventBus.post(new PublicKeyCreatedEvent(new DefaultPublicKey(key.getId(), key.getOwner(), key.getRaw(), key.getContacts())));
eventBus.post(new PublicKeyCreatedEvent(RawGpgKeyToDefaultPublicKeyMapper.map(key)));
return key;
@@ -92,8 +92,8 @@ public class PublicKeyStore {
private Set<Person> getContactsFromPublicKey(String rawKey) {
List<String> userIds = new ArrayList<>();
Optional<PGPPublicKey> publicKeyFromRawKey = extractPublicKey(rawKey);
publicKeyFromRawKey.ifPresent(pgpPublicKey -> pgpPublicKey.getUserIDs().forEachRemaining(userIds::add));
PGPPublicKey publicKeyFromRawKey = extractPublicKey(rawKey);
publicKeyFromRawKey.getUserIDs().forEachRemaining(userIds::add);
return userIds.stream().map(Person::toPerson).collect(Collectors.toSet());
}
@@ -104,7 +104,7 @@ public class PublicKeyStore {
if (!rawGpgKey.isReadonly()) {
UserPermissions.changePublicKeys(rawGpgKey.getOwner()).check();
store.remove(id);
eventBus.post(new PublicKeyDeletedEvent(new DefaultPublicKey(rawGpgKey.getId(), rawGpgKey.getOwner(), rawGpgKey.getRaw(), rawGpgKey.getContacts())));
eventBus.post(new PublicKeyDeletedEvent(RawGpgKeyToDefaultPublicKeyMapper.map(rawGpgKey)));
} else {
throw new DeletingReadonlyKeyNotAllowedException(id);
}

View File

@@ -27,7 +27,6 @@ package sonia.scm.security.gpg;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import sonia.scm.repository.Person;
import sonia.scm.xml.XmlInstantAdapter;

View File

@@ -0,0 +1,37 @@
/*
* 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 sonia.scm.security.PublicKey;
final class RawGpgKeyToDefaultPublicKeyMapper {
private RawGpgKeyToDefaultPublicKeyMapper() {}
static PublicKey map(RawGpgKey key) {
return new DefaultPublicKey(key.getId(), key.getOwner(), key.getRaw(), key.getContacts());
}
}