diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 1866168aa..a0437fa59 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -13,4 +13,4 @@ - [ ] I have kept the patch limited to only change the parts related to the patch - [ ] This change requires a documentation update -See also [Contributing Guidelines](../../CONTRIBUTING.md). +See also [Contributing Guidelines](../CONTRIBUTING.md). diff --git a/signserver/modules/SignServer-Common/src/main/java/org/signserver/common/SignServerConstants.java b/signserver/modules/SignServer-Common/src/main/java/org/signserver/common/SignServerConstants.java index d4cce59ff..008542813 100644 --- a/signserver/modules/SignServer-Common/src/main/java/org/signserver/common/SignServerConstants.java +++ b/signserver/modules/SignServer-Common/src/main/java/org/signserver/common/SignServerConstants.java @@ -89,6 +89,9 @@ public class SignServerConstants { */ public static final String KEYUSAGELIMIT = "KEYUSAGELIMIT"; public static String DISABLEKEYUSAGECOUNTER = "DISABLEKEYUSAGECOUNTER"; + + public static String PROCESSINTRANSACTION = "PROCESSINTRANSACTION"; + /** * Constant used to set the default value of configuration property to NULL if not setting property means property value is NULL. * diff --git a/signserver/modules/SignServer-Server/src/main/java/org/cesecore/keys/token/KeyAliasesCache.java b/signserver/modules/SignServer-Server/src/main/java/org/cesecore/keys/token/KeyAliasesCache.java index 9b397589b..ca8008203 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/cesecore/keys/token/KeyAliasesCache.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/cesecore/keys/token/KeyAliasesCache.java @@ -23,6 +23,8 @@ */ public class KeyAliasesCache extends CommonCacheBase { + long lastUpdate = 0L; + @Override public PublicKey getEntry(final Integer id) { if (id == null) { @@ -31,6 +33,10 @@ public PublicKey getEntry(final Integer id) { return super.getEntry(id); } + public void updateCacheTimeStamp() { + this.lastUpdate = System.currentTimeMillis(); + } + @Override protected long getCacheTime() { return 30000; // Cache key aliases for 30 seconds diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/BaseWorker.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/BaseWorker.java index 699ba3a53..bc7e1c0e2 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/BaseWorker.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/BaseWorker.java @@ -209,4 +209,8 @@ public WorkerType getWorkerType() { } return type; } + + public boolean requiresTransaction(final IServices services) { + return false; + } } diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/IWorker.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/IWorker.java index c7f832004..33fdbd604 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/IWorker.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/IWorker.java @@ -68,4 +68,11 @@ public interface IWorker { * @return a WorkerStatus object. */ WorkerStatusInfo getStatus(final List additionalFatalErrors, final IServices services); + + /** + * If worker requires a database transaction when using this crypto token. + * + * @return True or false + */ + boolean requiresTransaction(final IServices services); } diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/UnloadableWorker.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/UnloadableWorker.java index 0953362ef..0d604accb 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/UnloadableWorker.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/UnloadableWorker.java @@ -129,6 +129,11 @@ public boolean isSingleton() { return false; } + @Override + public boolean requiresTransaction(final IServices services) { + return false; + } + /** * @return No log types */ diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/BaseCryptoToken.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/BaseCryptoToken.java index 28362046c..0e6da60c1 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/BaseCryptoToken.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/BaseCryptoToken.java @@ -27,4 +27,9 @@ public boolean isNoCertificatesRequired() { return false; } + @Override + public boolean requiresTransactionForSigning() { + return false; + } + } diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/ICryptoTokenV4.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/ICryptoTokenV4.java index 6feb0be05..eaa5588d9 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/ICryptoTokenV4.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/cryptotokens/ICryptoTokenV4.java @@ -279,4 +279,11 @@ boolean removeKey(String alias, IServices services) throws CryptoTokenOfflineExc * @return True or false */ boolean isNoCertificatesRequired(); + + /** + * If worker requires a database transaction for signing operation. + * + * @return True or false + */ + boolean requiresTransactionForSigning(); } diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/signers/BaseSigner.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/signers/BaseSigner.java index 7d64f3fb0..4209029a4 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/signers/BaseSigner.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/signers/BaseSigner.java @@ -234,6 +234,19 @@ public WorkerStatusInfo getStatus(final List additionalFatalErrors, fina completeEntries, config); } + public boolean requiresTransaction(final IServices services) { + try { + ICryptoTokenV4 cryptoToken = super.getCryptoToken(services); + if (cryptoToken == null) { + return false; + } + return cryptoToken.requiresTransactionForSigning(); + } catch (Exception e) { + LOG.warn("Unable to determine whether a worker requires a transaction. Defaulting to False.", e); + return false; + } + } + @Override protected List getFatalErrors(IServices services) { final LinkedList errors = new LinkedList<>(super.getFatalErrors(services)); diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/BaseTimedService.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/BaseTimedService.java index b73255dff..01c11a378 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/BaseTimedService.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/BaseTimedService.java @@ -142,6 +142,11 @@ public boolean isSingleton() { return active.trim().equalsIgnoreCase("TRUE"); } + @Override + public boolean requiresTransaction(final IServices services) { + return false; + } + @Override public WorkerStatusInfo getStatus(final List additionalFatalErrors, final IServices services) { final List fatalErrorsIncludingAdditionalErrors = new LinkedList<>(additionalFatalErrors); diff --git a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/ITimedService.java b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/ITimedService.java index bbebb5724..9ea20c6ae 100644 --- a/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/ITimedService.java +++ b/signserver/modules/SignServer-Server/src/main/java/org/signserver/server/timedservices/ITimedService.java @@ -14,6 +14,7 @@ import java.util.Set; import org.signserver.common.ServiceContext; +import org.signserver.server.IServices; import org.signserver.server.IWorker; import org.signserver.server.ServiceExecutionFailedException; @@ -62,7 +63,12 @@ public interface ITimedService extends IWorker { * the time, of false if it should be run on all nodes simultaneously. */ boolean isSingleton(); - + + /** + * @return true if the service requires a transaction to be executed successfully + */ + boolean requiresTransaction(final IServices services); + /** * Get log types for logging work invocations. * diff --git a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/DispatcherProcessSessionBean.java b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/DispatcherProcessSessionBean.java index d25bf8c33..6cd85975a 100644 --- a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/DispatcherProcessSessionBean.java +++ b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/DispatcherProcessSessionBean.java @@ -139,7 +139,7 @@ public Response process(final AdminInfo adminInfo, final WorkerIdentifier wi, throws IllegalRequestException, CryptoTokenOfflineException, SignServerException { requestContext.setServices(servicesImpl); - if (SessionUtils.needsTransaction(workerManagerSession, wi)) { + if (SessionUtils.needsTransaction(workerManagerSession, wi, servicesImpl)) { // use separate transaction bean to avoid deadlock return dispatcherProcessTransSession.processWithTransaction(adminInfo, wi, request, requestContext); } else { diff --git a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/InternalProcessSessionBean.java b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/InternalProcessSessionBean.java index 3b4a0e11b..8c463b323 100644 --- a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/InternalProcessSessionBean.java +++ b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/InternalProcessSessionBean.java @@ -137,7 +137,7 @@ public Response process(final AdminInfo adminInfo, final WorkerIdentifier wi, throws IllegalRequestException, CryptoTokenOfflineException, SignServerException { requestContext.setServices(servicesImpl); - if (SessionUtils.needsTransaction(workerManagerSession, wi)) { + if (SessionUtils.needsTransaction(workerManagerSession, wi, servicesImpl)) { // use separate transaction bean to avoid deadlock return internalProcessTransSession.processWithTransaction(adminInfo, wi, request, requestContext); } else { diff --git a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ProcessSessionBean.java b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ProcessSessionBean.java index c859c9b1c..3b7dbf9c3 100644 --- a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ProcessSessionBean.java +++ b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ProcessSessionBean.java @@ -336,7 +336,7 @@ public Response process(final AdminInfo adminInfo, final WorkerIdentifier wi, LOG.debug(">process: " + wi); } - if (SessionUtils.needsTransaction(workerManagerSession, wi)) { + if (SessionUtils.needsTransaction(workerManagerSession, wi, servicesImpl)) { // use separate transaction bean to avoid deadlock return processTransSession.processWithTransaction(adminInfo, wi, request, requestContext); } else { diff --git a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ServiceTimerSessionBean.java b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ServiceTimerSessionBean.java index 6c4125307..c996e3000 100644 --- a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ServiceTimerSessionBean.java +++ b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/ServiceTimerSessionBean.java @@ -160,6 +160,7 @@ public void ejbTimeout(Timer timer) { ITimedService timedService = null; boolean run = false; boolean isSingleton = false; + boolean requiresTransaction = false; UserTransaction ut = sessionCtx.getUserTransaction(); try { ut.begin(); @@ -168,6 +169,7 @@ public void ejbTimeout(Timer timer) { timedService = (ITimedService) worker; sessionCtx.getTimerService().createSingleActionTimer(timedService.getNextInterval(), new TimerConfig(timerInfo, false)); isSingleton = timedService.isSingleton(); + requiresTransaction = timedService.requiresTransaction(servicesImpl); if (!isSingleton) { run = true; } else { @@ -195,11 +197,19 @@ public void ejbTimeout(Timer timer) { } } + ut = sessionCtx.getUserTransaction(); if (run) { if (serviceConfig != null && timedService != null) { try { if (timedService.isActive() && timedService.getNextInterval() != ITimedService.DONT_EXECUTE) { - timedService.work(new ServiceContext(servicesImpl)); + if(requiresTransaction) { + ut.begin(); + timedService.work(new ServiceContext(servicesImpl)); + ut.commit(); + } else { + timedService.work(new ServiceContext(servicesImpl)); + } + serviceConfig.setLastRunTimestamp(new Date()); for (final ITimedService.LogType logType : timedService.getLogTypes()) { @@ -225,7 +235,8 @@ public void ejbTimeout(Timer timer) { } } - } catch (ServiceExecutionFailedException e) { + } catch (ServiceExecutionFailedException | SystemException | HeuristicRollbackException | NotSupportedException | + HeuristicMixedException | RollbackException e) { // always log to error log, regardless of log types // setup for service run logging LOG.error("Service" + timerInfo + " execution failed. ", e); @@ -240,6 +251,11 @@ public void ejbTimeout(Timer timer) { timerInfo.toString(), Collections.singletonMap("Message", e.getMessage())); } + try { + ut.rollback(); + } catch (SystemException ex) { + LOG.error("Rollback failed.", e); + } } catch (RuntimeException e) { /* * DSS-377: @@ -251,6 +267,11 @@ public void ejbTimeout(Timer timer) { * since it is some kind of catastrophic failure.. */ LOG.error("Service worker execution failed.", e); + try { + ut.rollback(); + } catch (SystemException ex) { + throw new RuntimeException("Rollback failed", ex); + } } } else { LOG.error("Service with ID " + timerInfo + " not found."); diff --git a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/SessionUtils.java b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/SessionUtils.java index e3f41ed54..d2ef31cda 100644 --- a/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/SessionUtils.java +++ b/signserver/modules/SignServer-ejb/src/main/java/org/signserver/ejb/SessionUtils.java @@ -17,6 +17,8 @@ import org.signserver.ejb.worker.impl.PreloadedWorkerConfig; import org.signserver.ejb.worker.impl.WorkerManagerSingletonBean; import org.signserver.ejb.worker.impl.WorkerWithComponents; +import org.signserver.server.IServices; +import org.signserver.server.IWorker; /** * Utility functions for session beans. @@ -33,13 +35,14 @@ public class SessionUtils { * @return true if the request needs a transaction */ public static boolean needsTransaction(final WorkerManagerSingletonBean session, - final WorkerIdentifier wi) { + final WorkerIdentifier wi, IServices services) { try { - final WorkerWithComponents worker = session.getWorkerWithComponents(wi); - final PreloadedWorkerConfig pwc = worker.getPreloadedConfig(); + final WorkerWithComponents workerWithComponents = session.getWorkerWithComponents(wi); + final IWorker worker = workerWithComponents.getWorker(); + final PreloadedWorkerConfig pwc = workerWithComponents.getPreloadedConfig(); - return !worker.getArchivers().isEmpty() || - !pwc.isDisableKeyUsageCounter() || pwc.isKeyUsageLimitSpecified(); + return !workerWithComponents.getArchivers().isEmpty() || + !pwc.isDisableKeyUsageCounter() || pwc.isKeyUsageLimitSpecified() || worker.requiresTransaction(services); } catch (NoSuchWorkerException e) { return false; }