Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
*/
public class KeyAliasesCache extends CommonCacheBase<PublicKey> {

long lastUpdate = 0L;

@Override
public PublicKey getEntry(final Integer id) {
if (id == null) {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,8 @@ public WorkerType getWorkerType() {
}
return type;
}

public boolean requiresTransaction(final IServices services) {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,11 @@ public interface IWorker {
* @return a WorkerStatus object.
*/
WorkerStatusInfo getStatus(final List<String> additionalFatalErrors, final IServices services);

/**
* If worker requires a database transaction when using this crypto token.
*
* @return True or false
*/
boolean requiresTransaction(final IServices services);
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@ public boolean isSingleton() {
return false;
}

@Override
public boolean requiresTransaction(final IServices services) {
return false;
}

/**
* @return No log types
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@ public boolean isNoCertificatesRequired() {
return false;
}

@Override
public boolean requiresTransactionForSigning() {
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,19 @@ public WorkerStatusInfo getStatus(final List<String> 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<String> getFatalErrors(IServices services) {
final LinkedList<String> errors = new LinkedList<>(super.getFatalErrors(services));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> additionalFatalErrors, final IServices services) {
final List<String> fatalErrorsIncludingAdditionalErrors = new LinkedList<>(additionalFatalErrors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 {
Expand Down Expand Up @@ -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()) {
Expand All @@ -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);
Expand All @@ -240,6 +251,11 @@ public void ejbTimeout(Timer timer) {
timerInfo.toString(),
Collections.<String, Object>singletonMap("Message", e.getMessage()));
}
try {
ut.rollback();
} catch (SystemException ex) {
LOG.error("Rollback failed.", e);
}
} catch (RuntimeException e) {
/*
* DSS-377:
Expand All @@ -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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand Down