From 3f7e41b245c5eba4ea9409d396bbc84b546837bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Le=C3=9Fmann=20=28=40MarkL4YG=29?= Date: Sat, 23 Nov 2024 13:41:30 +0100 Subject: [PATCH] chore: Update termination/exception handling --- .../openssl/OpenSSLCertificateCreator.java | 102 ++++++++++-------- .../service/CertificateCreationService.java | 5 - 2 files changed, 57 insertions(+), 50 deletions(-) diff --git a/src/main/java/de/mlessmann/certassist/openssl/OpenSSLCertificateCreator.java b/src/main/java/de/mlessmann/certassist/openssl/OpenSSLCertificateCreator.java index ea02474..add5193 100644 --- a/src/main/java/de/mlessmann/certassist/openssl/OpenSSLCertificateCreator.java +++ b/src/main/java/de/mlessmann/certassist/openssl/OpenSSLCertificateCreator.java @@ -1,6 +1,7 @@ package de.mlessmann.certassist.openssl; import static de.mlessmann.certassist.Constants.CERTASSIST_TMP_PREFIX; +import static java.util.concurrent.TimeUnit.*; import static org.slf4j.LoggerFactory.getLogger; import de.mlessmann.certassist.DeleteRecursiveFileVisitor; @@ -18,7 +19,6 @@ import java.util.List; import java.util.Optional; import java.util.UUID; import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -95,7 +95,7 @@ public class OpenSSLCertificateCreator { try { log.debug("Process is still alive. Asking politely for it to destroy itself and waiting on exit for 2s"); sysProc.destroy(); - sysProc.waitFor(2_000, TimeUnit.MILLISECONDS); + sysProc.waitFor(2_000, MILLISECONDS); } catch (InterruptedException e) { log.debug("Interrupted while waiting for process to terminate. Registering forceful termination onExit", e); Runtime.getRuntime().addShutdownHook(new Thread(sysProc::destroyForcibly)); @@ -103,8 +103,7 @@ public class OpenSSLCertificateCreator { } @NonNull - public OpenSSLCertificateResult createCertificate(CertificateRequest request) - throws CommandLineOperationException, InterruptedException { + public OpenSSLCertificateResult createCertificate(CertificateRequest request) throws CommandLineOperationException { Path tmpDir; try { tmpDir = Files.createTempDirectory(CERTASSIST_TMP_PREFIX); @@ -151,12 +150,14 @@ public class OpenSSLCertificateCreator { } private Path createKeyfile(CertificateRequest request, Path outFile, String filePassword) - throws CommandLineOperationException, InterruptedException { + throws CommandLineOperationException { Path keyFile = outFile.toAbsolutePath(); log.debug("Writing new certificate key to {}", keyFile); + StartedProcess keygenProc = null; try { - StartedProcess keygenProc = new ProcessExecutor() + keygenProc = + new ProcessExecutor() .command( resolveOpenSSL(), "genrsa", @@ -171,17 +172,17 @@ public class OpenSSLCertificateCreator { .redirectOutput(Slf4jStream.of(openSSLLogger).asDebug()) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - keygenProc.getFuture().get(); - } catch (IOException e) { + keygenProc.getFuture().get(3, MINUTES); + } catch (IOException | ExecutionException | InterruptedException | TimeoutException e) { throw new CommandLineOperationException("Failure running OpenSSL keygen command.", e); - } catch (ExecutionException e) { - throw new RuntimeException(e); + } finally { + killIfActive(keygenProc); } return keyFile; } private Path createCertificate(CertificateRequest request, Path keyFile, Path outFile, String keyPassphrase) - throws CommandLineOperationException, InterruptedException { + throws CommandLineOperationException { log.debug("Writing new certificate file {}", outFile); String certSubject = buildSubjectArg(request); @@ -211,11 +212,9 @@ public class OpenSSLCertificateCreator { .redirectOutput(Slf4jStream.of(openSSLLogger).asDebug()) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - certGenProc.getFuture().get(); - } catch (IOException e) { + certGenProc.getFuture().get(30, SECONDS); + } catch (IOException | ExecutionException | InterruptedException | TimeoutException e) { throw new CommandLineOperationException("Failure running OpenSSL req command.", e); - } catch (ExecutionException e) { - throw new RuntimeException(e); } finally { killIfActive(certGenProc); } @@ -223,12 +222,14 @@ public class OpenSSLCertificateCreator { } private Path createSigningRequest(CertificateRequest request, Path keyFile, Path outFile, String certPassword) - throws CommandLineOperationException, InterruptedException { + throws CommandLineOperationException { log.atDebug().log("Writing new certificate signing request file {}", outFile); String certSubject = buildSubjectArg(request); + StartedProcess certGenProc = null; try { - StartedProcess certGenProc = new ProcessExecutor() + certGenProc = + new ProcessExecutor() .command( resolveOpenSSL(), "req", @@ -248,11 +249,11 @@ public class OpenSSLCertificateCreator { .redirectOutput(Slf4jStream.of(openSSLLogger).asDebug()) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - certGenProc.getFuture().get(); - } catch (IOException e) { + certGenProc.getFuture().get(30, SECONDS); + } catch (IOException | ExecutionException | InterruptedException | TimeoutException e) { throw new CommandLineOperationException("Failure running OpenSSL req command.", e); - } catch (ExecutionException e) { - throw new RuntimeException(e); + } finally { + killIfActive(certGenProc); } return outFile; } @@ -271,6 +272,7 @@ public class OpenSSLCertificateCreator { } Path tmpDir = null; + StartedProcess verifyCommand = null; try { Path tempTrustedBundle; if (trustedCAs.size() == 1) { @@ -289,16 +291,18 @@ public class OpenSSLCertificateCreator { } } - StartedProcess verifyCommand = new ProcessExecutor() + verifyCommand = + new ProcessExecutor() .command(resolveOpenSSL(), "verify", "-CAfile", tempTrustedBundle.toString(), fullChainFile.toString()) .redirectOutput(Slf4jStream.of(openSSLLogger).asError()) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - var verifyResult = verifyCommand.getFuture().get(); + var verifyResult = verifyCommand.getFuture().get(30, SECONDS); return verifyResult.getExitValue() == 0; - } catch (IOException | InterruptedException | ExecutionException e) { + } catch (IOException | InterruptedException | ExecutionException | TimeoutException e) { throw new RuntimeException(e); } finally { + killIfActive(verifyCommand); if (tmpDir != null) { try { Files.walkFileTree(tmpDir, new DeleteRecursiveFileVisitor()); @@ -313,7 +317,7 @@ public class OpenSSLCertificateCreator { /** * Checks whether the provided key file is encrypted using a passphrase */ - public boolean isKeyEncrypted(@NonNull Path keyFile) throws InterruptedException, CommandLineOperationException { + public boolean isKeyEncrypted(@NonNull Path keyFile) throws CommandLineOperationException { // If the key is not encrypted, any passphrase will work -> so generate a random one to check. String passphrase = UUID.randomUUID().toString(); boolean firstPass = verifyKeyPassphrase(keyFile, passphrase); @@ -330,9 +334,11 @@ public class OpenSSLCertificateCreator { * @implNote Due to the implementation of the OpenSSL cli, any password will be valid for unencrypted keys. (Check with {@link #isKeyEncrypted(Path).) */ public boolean verifyKeyPassphrase(@NonNull Path keyFile, @NonNull String passphrase) - throws CommandLineOperationException, InterruptedException { + throws CommandLineOperationException { + StartedProcess verifyCommand = null; try { - StartedProcess verifyCommand = new ProcessExecutor() + verifyCommand = + new ProcessExecutor() .command( resolveOpenSSL(), "rsa", @@ -346,10 +352,12 @@ public class OpenSSLCertificateCreator { .redirectOutput(Slf4jStream.of(openSSLLogger).asError()) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - var verifyResult = verifyCommand.getFuture().get(); + var verifyResult = verifyCommand.getFuture().get(30, SECONDS); return verifyResult.getExitValue() == 0; - } catch (IOException | InterruptedException | ExecutionException e) { + } catch (IOException | InterruptedException | ExecutionException | TimeoutException e) { throw new CommandLineOperationException("Failed to verify key encryption", e); + } finally { + killIfActive(verifyCommand); } } @@ -359,7 +367,7 @@ public class OpenSSLCertificateCreator { Path caKey, String caKeyPassphrase, Path csrFile - ) throws CommandLineOperationException, InterruptedException { + ) throws CommandLineOperationException { Path outFile = csrFile.resolveSibling(csrFile.getFileName().toString().replace(".csr", ".crt")); log.debug("Writing new signed certificate file {}", outFile); Path extFile = csrFile.resolveSibling(csrFile.getFileName().toString().replace(".csr", ".ext")); @@ -422,31 +430,31 @@ public class OpenSSLCertificateCreator { .redirectOutput(Slf4jStream.of(openSSLLogger).asDebug()) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - ProcessResult result = certGenProc.getFuture().get(30, TimeUnit.SECONDS); + ProcessResult result = certGenProc.getFuture().get(30, SECONDS); // Check exit code if (result.getExitValue() != 0) { - throw new CommandLineOperationException("Failed to sign certificate. Exit code: " + result.getExitValue()); + throw new CommandLineOperationException( + "Failed to sign certificate. Exit code: " + result.getExitValue() + ); } - - } catch (IOException | TimeoutException e) { + } catch (IOException | TimeoutException | ExecutionException | InterruptedException e) { throw new CommandLineOperationException("Failure running OpenSSL x509 command.", e); - } catch (ExecutionException e) { - throw new RuntimeException(e); } finally { killIfActive(certGenProc); } return outFile; } - public String getCertificateFingerprint(Path certificate) - throws CommandLineOperationException, InterruptedException { + public String getCertificateFingerprint(Path certificate) throws CommandLineOperationException { + StartedProcess fingerprintProc = null; try { - StartedProcess fingerprintProc = new ProcessExecutor() + fingerprintProc = + new ProcessExecutor() .command(resolveOpenSSL(), "x509", "-in", certificate.toString(), "-noout", "-fingerprint") .readOutput(true) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - var fingerprintResult = fingerprintProc.getFuture().get(); + var fingerprintResult = fingerprintProc.getFuture().get(30, SECONDS); String output = fingerprintResult.getOutput().getUTF8(); if (fingerprintResult.getExitValue() != 0) { @@ -473,14 +481,18 @@ public class OpenSSLCertificateCreator { ); } return "%s;%s".formatted(algorithm, fingerprint); - } catch (IOException | ExecutionException e) { + } catch (IOException | ExecutionException | TimeoutException | InterruptedException e) { throw new RuntimeException(e); + } finally { + killIfActive(fingerprintProc); } } - public CertificateRequest getCertificateInfo(Path path) throws CommandLineOperationException, InterruptedException { + public CertificateRequest getCertificateInfo(Path path) throws CommandLineOperationException { + StartedProcess infoProc = null; try { - StartedProcess infoProc = new ProcessExecutor() + infoProc = + new ProcessExecutor() .command( resolveOpenSSL(), "x509", @@ -507,7 +519,7 @@ public class OpenSSLCertificateCreator { .readOutput(true) .redirectError(Slf4jStream.of(openSSLLogger).asError()) .start(); - var infoResult = infoProc.getFuture().get(); + var infoResult = infoProc.getFuture().get(30, SECONDS); String output = infoResult.getOutput().getUTF8(); if (infoResult.getExitValue() != 0) { log.debug("Certificate info command output:\n{}", output); @@ -516,7 +528,7 @@ public class OpenSSLCertificateCreator { ); } return getCertificateInfo(output.lines().toArray(String[]::new)); - } catch (IOException | ExecutionException e) { + } catch (IOException | ExecutionException | InterruptedException | TimeoutException e) { throw new RuntimeException(e); } } diff --git a/src/main/java/de/mlessmann/certassist/service/CertificateCreationService.java b/src/main/java/de/mlessmann/certassist/service/CertificateCreationService.java index bc1265f..429efe3 100644 --- a/src/main/java/de/mlessmann/certassist/service/CertificateCreationService.java +++ b/src/main/java/de/mlessmann/certassist/service/CertificateCreationService.java @@ -32,9 +32,6 @@ public class CertificateCreationService { ) { certificate.setPrivateKey(Files.readAllBytes(certificateCreatorResult.certificateKeyPath())); certificate.setCert(Files.readAllBytes(certificateCreatorResult.certificatePath())); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IllegalStateException("Interrupted exception", e); } catch (CommandLineOperationException | IOException e) { throw new IllegalStateException("Failed to create certificate!", e); } @@ -81,8 +78,6 @@ public class CertificateCreationService { return certificateRepository.save(entity); } catch (CommandLineOperationException | IOException e) { throw new RuntimeException("Unable to import certificate", e); - } catch (InterruptedException e) { - throw new RuntimeException(e); } }