Skip to content

Commit

Permalink
backport duo secuity bypass fixes from 5.3
Browse files Browse the repository at this point in the history
  • Loading branch information
mmoayyed committed Aug 29, 2018
1 parent 8eba5ce commit 7e5f5e7
Show file tree
Hide file tree
Showing 12 changed files with 287 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,15 @@ boolean supports(Event event,
Authentication authentication,
RegisteredService registeredService,
HttpServletRequest request);


/**
* This method will inspect global and service properties and determine which
* FailureMode applies to the current authentication transaction.
*
* @param service the service
* @return the FailureMode
*/
default RegisteredServiceMultifactorPolicy.FailureModes determineFailureMode(final RegisteredService service) {
return RegisteredServiceMultifactorPolicy.FailureModes.NONE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,26 @@ public interface CasWebflowConstants {
*/
String TRANSITION_ID_INTERRUPT_REQUIRED = "interruptRequired";

/**
* Provider service is unavailable.
*/
String TRANSITION_ID_UNAVAILABLE = "unavailable";

/**
* User allowed to bypass auth by provider.
*/
String TRANSITION_ID_BYPASS = "bypass";

/**
* User was denied access by provider.
*/
String TRANSITION_ID_DENY = "deny";

/**
* The transition state 'success'.
*/
String TRANSITION_ID_SUCCESS = "success";

/**
* Transition id 'redirect' .
*/
Expand All @@ -60,6 +76,7 @@ public interface CasWebflowConstants {
* Front transition id.
*/
String TRANSITION_ID_FRONT = "front";

/**
* Proceed transition id.
*/
Expand Down Expand Up @@ -89,6 +106,7 @@ public interface CasWebflowConstants {
* The state id 'stopWebflow'.
*/
String STATE_ID_STOP_WEBFLOW = "stopWebflow";

/**
* The state id 'clientAction'.
*/
Expand All @@ -104,7 +122,6 @@ public interface CasWebflowConstants {
*/
String VIEW_ID_REGISTER_DEVICE = "registerDeviceView";


/**
* The state id 'registerTrustedDevice'.
*/
Expand Down Expand Up @@ -154,6 +171,7 @@ public interface CasWebflowConstants {
* The transition state 'submit'.
*/
String TRANSITION_ID_SUBMIT = "submit";

/**
* The transition state 'error'.
*/
Expand Down Expand Up @@ -183,12 +201,12 @@ public interface CasWebflowConstants {
* 'gateway' state id.
*/
String STATE_ID_GATEWAY = "gateway";

/**
* 'finishMfaTrustedAuth' state id.
*/
String STATE_ID_FINISH_MFA_TRUSTED_AUTH = "finishMfaTrustedAuth";


/**
* The transition state 'warn'.
*/
Expand Down Expand Up @@ -244,7 +262,6 @@ public interface CasWebflowConstants {
*/
String STATE_ID_GENERATE_SERVICE_TICKET = "generateServiceTicket";


/**
* The state 'gatewayServicesManagementCheck'.
*/
Expand Down Expand Up @@ -285,7 +302,6 @@ public interface CasWebflowConstants {
*/
String STATE_ID_SERVICE_UNAUTHZ_CHECK = "serviceUnauthorizedCheck";


/**
* The state 'viewGenericLoginSuccess'.
*/
Expand Down Expand Up @@ -321,6 +337,26 @@ public interface CasWebflowConstants {
*/
String STATE_ID_REDIRECT = "redirect";

/**
* State id when MFA provider has been detected as unavailable and failureMode is closed.
*/
String STATE_ID_MFA_UNAVAILABLE = "mfaUnavailable";

/**
* State id when MFA provider has denied access to a user because of account lockout.
*/
String STATE_ID_MFA_DENIED = "mfaDenied";

/**
* View id when MFA provider has been detected as unavailable and failureMode is closed.
*/
String VIEW_ID_MFA_UNAVAILABLE = "casMfaUnavailableView";

/**
* View id when MFA provider has denied access to a user because of account lockout.
*/
String VIEW_ID_MFA_DENIED = "casMfaDeniedView";

/**
* The view id 'casPostResponseView'.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.apereo.cas.services.MultifactorAuthenticationProvider;
import org.apereo.cas.services.RegisteredService;
import org.apereo.cas.services.RegisteredServiceMultifactorPolicy;
import org.apereo.cas.services.RegisteredServiceMultifactorPolicy.FailureModes;

import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand All @@ -16,9 +17,6 @@

import javax.servlet.http.HttpServletRequest;

import static org.apereo.cas.services.RegisteredServiceMultifactorPolicy.FailureModes.CLOSED;
import static org.apereo.cas.services.RegisteredServiceMultifactorPolicy.FailureModes.UNDEFINED;

/**
* The {@link AbstractMultifactorAuthenticationProvider} is responsible for
* as the parent of all providers.
Expand Down Expand Up @@ -77,19 +75,7 @@ protected boolean supportsInternal(final Event e, final Authentication authentic

@Override
public boolean isAvailable(final RegisteredService service) throws AuthenticationException {
var failureMode = CLOSED;
if (StringUtils.isNotBlank(this.globalFailureMode)) {
failureMode = RegisteredServiceMultifactorPolicy.FailureModes.valueOf(this.globalFailureMode);
LOGGER.debug("Using global multi-factor failure mode for [{}] defined as [{}]", service, failureMode);
}
if (service != null) {
LOGGER.debug("Evaluating multifactor authentication policy for service [{}}", service);
val policy = service.getMultifactorPolicy();
if (policy != null && policy.getFailureMode() != UNDEFINED) {
failureMode = policy.getFailureMode();
LOGGER.debug("Multi-factor failure mode for [{}] is defined as [{}]", service.getServiceId(), failureMode);
}
}
val failureMode = determineFailureMode(service);
if (failureMode != RegisteredServiceMultifactorPolicy.FailureModes.NONE) {
if (isAvailable()) {
return true;
Expand Down Expand Up @@ -120,4 +106,24 @@ protected boolean isAvailable() {
public boolean matches(final String identifier) {
return StringUtils.isNotBlank(getId()) ? getId().matches(identifier) : false;
}

@Override
public RegisteredServiceMultifactorPolicy.FailureModes determineFailureMode(final RegisteredService service) {
var failureMode = FailureModes.CLOSED;
if (StringUtils.isNotBlank(this.globalFailureMode)) {
failureMode = RegisteredServiceMultifactorPolicy.FailureModes.valueOf(this.globalFailureMode);
LOGGER.debug("Using global multifactor failure mode for [{}] defined as [{}]", service, failureMode);
}
if (service != null) {
LOGGER.debug("Evaluating multifactor authentication policy for service [{}]", service.getServiceId());
val policy = service.getMultifactorPolicy();
if (policy != null && policy.getFailureMode() != null && policy.getFailureMode() != FailureModes.NONE) {
failureMode = policy.getFailureMode();
LOGGER.debug("Multifactor failure mode for [{}] is defined as [{}]", service.getServiceId(), failureMode);
}
}
LOGGER.debug("Final failure mode has been determined to be [{}]", failureMode);
return failureMode;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ protected void augmentMultifactorProviderFlowRegistry(final FlowDefinitionRegist
states.forEach(s -> {
val state = getState(flow, s);
ensureEndStateTransitionExists(state, flow, CasWebflowConstants.TRANSITION_ID_SUCCESS, CasWebflowConstants.STATE_ID_SUCCESS);
ensureEndStateTransitionExists(state, flow, CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS,
CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS);
ensureEndStateTransitionExists(state, flow, CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS, CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS);
ensureEndStateTransitionExists(state, flow, CasWebflowConstants.TRANSITION_ID_UNAVAILABLE, CasWebflowConstants.STATE_ID_MFA_UNAVAILABLE);
ensureEndStateTransitionExists(state, flow, CasWebflowConstants.TRANSITION_ID_DENY, CasWebflowConstants.STATE_ID_MFA_DENIED);
});
}
});
Expand All @@ -89,6 +90,14 @@ protected void augmentMultifactorProviderFlowRegistry(final FlowDefinitionRegist
* @param mfaProviderFlowRegistry the registry
*/
protected void registerMultifactorProviderAuthenticationWebflow(final Flow flow, final String subflowId, final FlowDefinitionRegistry mfaProviderFlowRegistry) {
LOGGER.debug("Adding end state [{}] with transition to [{}] to flow [{}] for MFA",
CasWebflowConstants.STATE_ID_MFA_UNAVAILABLE, CasWebflowConstants.VIEW_ID_MFA_UNAVAILABLE, flow.getId());
createEndState(flow, CasWebflowConstants.STATE_ID_MFA_UNAVAILABLE, CasWebflowConstants.VIEW_ID_MFA_UNAVAILABLE);

LOGGER.debug("Adding end state [{}] with transition to [{}] to flow [{}] for MFA",
CasWebflowConstants.STATE_ID_MFA_DENIED, CasWebflowConstants.VIEW_ID_MFA_DENIED, flow.getId());
createEndState(flow, CasWebflowConstants.STATE_ID_MFA_DENIED, CasWebflowConstants.VIEW_ID_MFA_DENIED);

val subflowState = createSubflowState(flow, subflowId, subflowId);
val states = getCandidateStatesForMultifactorAuthentication();
LOGGER.debug("Candidate states for multifactor authentication are [{}]", states);
Expand All @@ -97,11 +106,24 @@ protected void registerMultifactorProviderAuthenticationWebflow(final Flow flow,
LOGGER.debug("Locating state [{}] to process for multifactor authentication", s);
val actionState = getState(flow, s);

LOGGER.debug("Locating transition id [{}] to process multifactor authentication for state [{}", CasWebflowConstants.TRANSITION_ID_SUCCESS, s);
LOGGER.debug("Adding transition [{}] to [{}] for [{}]", CasWebflowConstants.TRANSITION_ID_DENY, CasWebflowConstants.STATE_ID_MFA_DENIED, s);
createTransitionForState(actionState, CasWebflowConstants.TRANSITION_ID_DENY, CasWebflowConstants.STATE_ID_MFA_DENIED);

LOGGER.debug("Adding transition [{}] to [{}] for [{}]", CasWebflowConstants.TRANSITION_ID_UNAVAILABLE, CasWebflowConstants.STATE_ID_MFA_UNAVAILABLE, s);
createTransitionForState(actionState, CasWebflowConstants.TRANSITION_ID_UNAVAILABLE, CasWebflowConstants.STATE_ID_MFA_UNAVAILABLE);

LOGGER.debug("Locating transition id [{}] to process multifactor authentication for state [{}]", CasWebflowConstants.TRANSITION_ID_SUCCESS, s);
val targetSuccessId = actionState.getTransition(CasWebflowConstants.TRANSITION_ID_SUCCESS).getTargetStateId();

LOGGER.debug("Locating transition id [{}] to process multifactor authentication for state [{}", CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS, s);
LOGGER.debug("Locating transition id [{}] to process multifactor authentication for state [{}]", CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS, s);
val targetWarningsId = actionState.getTransition(CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS).getTargetStateId();

LOGGER.debug("Locating transition id [{}] to process multifactor authentication for state [{}]", CasWebflowConstants.TRANSITION_ID_DENY, s);
val targetDenied = actionState.getTransition(CasWebflowConstants.TRANSITION_ID_DENY).getTargetStateId();

LOGGER.debug("Location transition id [{}] to process multifactor authentication for stat [{}]", CasWebflowConstants.TRANSITION_ID_UNAVAILABLE, s);
val targetUnavailable = actionState.getTransition(CasWebflowConstants.TRANSITION_ID_UNAVAILABLE).getTargetStateId();

val mappings = new ArrayList<DefaultMapping>();
val inputMapper = createMapperToSubflowState(mappings);
val subflowMapper = createSubflowAttributeMapper(inputMapper, null);
Expand All @@ -111,6 +133,8 @@ protected void registerMultifactorProviderAuthenticationWebflow(final Flow flow,
val transitionSet = subflowState.getTransitionSet();
transitionSet.add(createTransition(CasWebflowConstants.TRANSITION_ID_SUCCESS, targetSuccessId));
transitionSet.add(createTransition(CasWebflowConstants.TRANSITION_ID_SUCCESS_WITH_WARNINGS, targetWarningsId));
transitionSet.add(createTransition(CasWebflowConstants.TRANSITION_ID_DENY, targetDenied));
transitionSet.add(createTransition(CasWebflowConstants.TRANSITION_ID_UNAVAILABLE, targetUnavailable));

LOGGER.debug("Creating transition [{}] for state [{}]", subflowId, actionState.getId());
createTransitionForState(actionState, subflowId, subflowId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,9 @@ public enum DuoUserAccountAuthStatus {
/**
* The user is not known to Duo and needs to enroll.
*/
ENROLL
ENROLL,
/**
* Duo service was unavailable.
*/
UNAVAILABLE
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package org.apereo.cas.adaptors.duo.authn;

import org.apereo.cas.adaptors.duo.DuoUserAccountAuthStatus;
import org.apereo.cas.authentication.AbstractMultifactorAuthenticationProvider;
import org.apereo.cas.authentication.Authentication;
import org.apereo.cas.configuration.model.support.mfa.DuoSecurityMultifactorProperties;
import org.apereo.cas.services.RegisteredService;

import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
Expand All @@ -13,9 +10,7 @@
import lombok.NonNull;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;
import lombok.val;
import org.apache.commons.lang3.StringUtils;
import org.springframework.webflow.execution.Event;

/**
* This is {@link DefaultDuoMultifactorAuthenticationProvider}.
Expand Down Expand Up @@ -48,24 +43,6 @@ public String getId() {
return StringUtils.defaultIfBlank(super.getId(), DuoSecurityMultifactorProperties.DEFAULT_IDENTIFIER);
}

@Override
protected boolean supportsInternal(final Event e, final Authentication authentication, final RegisteredService registeredService) {
if (!super.supportsInternal(e, authentication, registeredService)) {
return false;
}
val principal = authentication.getPrincipal();
val acct = this.duoAuthenticationService.getDuoUserAccount(principal.getId());
LOGGER.debug("Found duo user account status [{}] for [{}]", acct, principal);
if (acct.getStatus() == DuoUserAccountAuthStatus.ALLOW) {
LOGGER.debug("Account status is set for allow/bypass for [{}]", principal);
return false;
}
if (acct.getStatus() == DuoUserAccountAuthStatus.DENY) {
LOGGER.warn("Account status is set to deny access to [{}]", principal);
}
return true;
}

@Override
public String getFriendlyName() {
return "Duo Security";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.apereo.cas.util.http.HttpClient;

import com.duosecurity.client.Http;
import com.duosecurity.duoweb.DuoWebException;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.EqualsAndHashCode;
import lombok.RequiredArgsConstructor;
Expand All @@ -31,11 +32,16 @@ public abstract class BaseDuoSecurityAuthenticationService implements DuoSecurit
private static final long serialVersionUID = -8044100706027708789L;

private static final int AUTH_API_VERSION = 2;
private static final int RESULT_CODE_ERROR_THRESHOLD = 49999;

private static final String RESULT_KEY_RESPONSE = "response";
private static final String RESULT_KEY_STAT = "stat";
private static final String RESULT_KEY_RESULT = "result";
private static final String RESULT_KEY_ENROLL_PORTAL_URL = "enroll_portal_url";
private static final String RESULT_KEY_STATUS_MESSAGE = "status_msg";
private static final String RESULT_KEY_CODE = "code";
private static final String RESULT_KEY_MESSAGE = "message";
private static final String RESULT_KEY_MESSAGE_DETAIL = "message_detail";

private static final ObjectMapper MAPPER = new ObjectMapper().findAndRegisterModules();

Expand Down Expand Up @@ -97,8 +103,12 @@ public DuoUserAccount getDuoUserAccount(final String username) {
LOGGER.debug("Received Duo admin response [{}]", jsonResponse);

val result = MAPPER.readTree(jsonResponse);
if (result.has(RESULT_KEY_RESPONSE) && result.has(RESULT_KEY_STAT)
&& result.get(RESULT_KEY_STAT).asText().equalsIgnoreCase("OK")) {
if (!result.has(RESULT_KEY_STAT)) {
LOGGER.warn("Duo admin response was received in unknown format: [{}]", jsonResponse);
throw new DuoWebException("Invalid response format received from Duo");
}

if (result.get(RESULT_KEY_STAT).asText().equalsIgnoreCase("OK")) {

val response = result.get(RESULT_KEY_RESPONSE);
val authResult = response.get(RESULT_KEY_RESULT).asText().toUpperCase();
Expand All @@ -110,9 +120,22 @@ public DuoUserAccount getDuoUserAccount(final String username) {
val enrollUrl = response.get(RESULT_KEY_ENROLL_PORTAL_URL).asText();
account.setEnrollPortalUrl(enrollUrl);
}
} else {
val code = result.get(RESULT_KEY_CODE).asInt();
if (code > RESULT_CODE_ERROR_THRESHOLD) {
LOGGER.warn("Duo returned a FAIL response with a code indicating a server error: [{}], Duo will be considered unavailable",
result.get(RESULT_KEY_MESSAGE));
throw new DuoWebException("Duo returned code 500: " + result.get(RESULT_KEY_MESSAGE));
}
LOGGER.warn("Duo returned an Invalid request response with message [{}] and detail [{}] "
+ "when determining user account. This maybe a configuration error in the admin request and Duo will "
+ "still be considered available",
result.get(RESULT_KEY_MESSAGE).asText(),
result.get(RESULT_KEY_MESSAGE_DETAIL).asText());
}
} catch (final Exception e) {
LOGGER.warn("Reaching Duo has failed with error: [{}]", e.getMessage(), e);
account.setStatus(DuoUserAccountAuthStatus.UNAVAILABLE);
}
return account;
}
Expand Down
Loading

0 comments on commit 7e5f5e7

Please sign in to comment.