Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding domain check to mitigate SSRF attacks. #7

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Adding domain check to mitigate SSRF attacks. #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62e6e46
Fix 404 error handle
PasinduYeshan Aug 11, 2022
643d3a0
Add domain check
PasinduYeshan Aug 20, 2022
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 components/org.wso2.carbon.identity.conditional.auth.functions.entgra/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
version="${carbon.identity.framework.imp.pkg.version.range}",
org.wso2.carbon.identity.application.authentication.framework.context;
version="${carbon.identity.framework.imp.pkg.version.range}",
org.wso2.carbon.identity.conditional.auth.functions.common.*,
org.wso2.carbon.identity.conditional.auth.functions.common.utils,
org.wso2.carbon.identity.event;
version="${carbon.identity.framework.imp.pkg.version.range}",
</Import-Package>
Expand Down
101 changes: 90 additions & 11 deletions ...o2/carbon/identity/conditional/auth/functions/entgra/GetDeviceInfoEntgraFunctionImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package org.wso2.carbon.identity.conditional.auth.functions.entgra;

import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

Expand Down Expand Up @@ -47,6 +48,7 @@

import java.io.IOException;
import java.net.SocketTimeoutException;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Map;
Expand All @@ -68,6 +70,8 @@ public class GetDeviceInfoEntgraFunctionImpl implements GetDeviceInfoEntgraFunct

private static final Log LOG = LogFactory.getLog(GetDeviceInfoEntgraFunctionImpl.class);
private CloseableHttpClient client;
private static final char DOMAIN_SEPARATOR = '.';
private final List<String> allowedDomains;

public GetDeviceInfoEntgraFunctionImpl() {

Expand All @@ -80,7 +84,7 @@ public GetDeviceInfoEntgraFunctionImpl() {
.setRelativeRedirectsAllowed(false)
.build();
client = HttpClientBuilder.create().setDefaultRequestConfig(config).build();

allowedDomains = ConfigProvider.getInstance().getAllowedDomainsForHttpFunctions();
}

@Override
Expand All @@ -107,6 +111,17 @@ public void getDeviceInfoEntgra(JsAuthenticationContext context, String platform
try {
HttpPost tokenRequest = getTokenRequest(tokenURL, clientKey, clientSecret);

if (!isValidRequestDomain(tokenRequest.getURI())) {
outcome = OUTCOME_FAIL;
response = getErrorJsonObject(Constants.AuthResponseErrorCode.ACCESS_DENIED,
"Access is denied. Please contact your administrator.");

LOG.error("Provided Url does not contain a allowed domain. Invalid Url: " +
tokenRequest.getURI().toString());
asyncReturn.accept(authenticationContext, response, outcome);
return;
}

// For catching and logging errors.
String errorURL = tokenURL;
try (CloseableHttpResponse tResponse = client.execute(tokenRequest)) {
Expand All @@ -117,10 +132,21 @@ public void getDeviceInfoEntgra(JsAuthenticationContext context, String platform
JSONParser parser = new JSONParser();
JSONObject jsonTokenResponse = (JSONObject) parser.parse(tJsonString);
String accessToken = (String) jsonTokenResponse.get(Constants.ACCESS_TOKEN);
tResponse.close();

HttpGet deviceInfoRequest = getDeviceInfoRequest(deviceInfoURL, accessToken);

tResponse.close();
if (!isValidRequestDomain(deviceInfoRequest.getURI())) {
outcome = OUTCOME_FAIL;
response = getErrorJsonObject(Constants.AuthResponseErrorCode.ACCESS_DENIED,
"Access is denied. Please contact your administrator.");

LOG.error("Provided Url does not contain a allowed domain. Invalid Url: " +
deviceInfoRequest.getURI().toString());
asyncReturn.accept(authenticationContext, response, outcome);
return;
}

try (CloseableHttpResponse dResponse = client.execute(deviceInfoRequest)) {
int dResponseCode = dResponse.getStatusLine().getStatusCode();

Expand Down Expand Up @@ -148,8 +174,11 @@ public void getDeviceInfoEntgra(JsAuthenticationContext context, String platform
Constants.AuthResponseErrorCode.DEVICE_NOT_ENROLLED_UNDER_CURRENT_USER,
"Access is denied. Please contact your administrator.");
}
} else if (dResponseCode == 404) {
outcome = OUTCOME_FAIL;
response = getErrorJsonObject(Constants.AuthResponseErrorCode.DEVICE_NOT_ENROLLED,
"Device is not recognized. Please register your device.");
} else {

LOG.error("Error while fetching device information from Entgra Server. " +
"Response code: " + dResponseCode);
outcome = OUTCOME_FAIL;
Expand All @@ -158,14 +187,6 @@ public void getDeviceInfoEntgra(JsAuthenticationContext context, String platform
errorURL = deviceInfoURL;
throw e;
}

} else if (tokenResponseCode == 404) {
LOG.error("Error while requesting access token from Entgra Server. Response code: "
+ tokenResponseCode);
outcome = OUTCOME_FAIL;
response = getErrorJsonObject(Constants.AuthResponseErrorCode.DEVICE_NOT_ENROLLED,
"Device is not recognized. Please register your device.");

} else {
LOG.error("Error while requesting access token from Entgra Server. Response code: "
+ tokenResponseCode);
Expand Down Expand Up @@ -266,4 +287,62 @@ private JSONObject getErrorJsonObject(Constants.AuthResponseErrorCode errorCode,
errorMap.put("errorMessage", errorMessage);
return errorMap;
}

/**
* Return true if the request domain is valid or return false if the request domain is invalid.
* @param url
* @return validity
*/
private boolean isValidRequestDomain(URI url) {
Copy link
Contributor

@ashendes ashendes Aug 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there any Framework util method for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got this from the http conditional auth function. It was implemented as a private method and I didn't find any util method for this.


if (url == null) {
if (LOG.isDebugEnabled()) {
LOG.debug("Provided url for domain restriction checking is null");
}
return false;
}

if (allowedDomains.isEmpty()) {
if (LOG.isDebugEnabled()) {
LOG.debug("No domains configured for domain restriction. Allowing url by default. Url: "
+ url.toString());
}
return true;
}

String domain = getParentDomainFromUrl(url);
if (StringUtils.isEmpty(domain)) {
LOG.error("Unable to determine the domain of the url: " + url.toString());
return false;
}

if (allowedDomains.contains(domain)) {
return true;
}

if (LOG.isDebugEnabled()) {
LOG.debug("Domain: " + domain + " extracted from url: " + url.toString() + " is not available in the " +
"allowed domain list: " + StringUtils.join(allowedDomains, ','));
}
return false;
}

private String getParentDomainFromUrl(URI url) {

String parentDomain = null;
String domain = url.getHost();
String[] domainArr;
if (domain != null) {
domainArr = StringUtils.split(domain, DOMAIN_SEPARATOR);
if (domainArr.length != 0) {
parentDomain = domainArr.length == 1 ? domainArr[0] : domainArr[domainArr.length - 2];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parent domain should be the joined string of the last two elements.

parentDomain = parentDomain.toLowerCase();
}
}

if (LOG.isDebugEnabled()) {
LOG.debug("Parent domain: " + parentDomain + " extracted from url: " + url.toString());
}
return parentDomain;
}
}
4 changes: 2 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,14 @@
<osgi.service.component.imp.pkg.version.range>[1.2.0, 2.0.0)</osgi.service.component.imp.pkg.version.range>
<carbon.identity.conditional.auth.functions.imp.pkg.version.range>[1.1.6, 2.0.0)
</carbon.identity.conditional.auth.functions.imp.pkg.version.range>
<carbon.identity.conditional.auth.functions.version>1.1.6</carbon.identity.conditional.auth.functions.version>
<carbon.identity.conditional.auth.functions.version>1.2.3</carbon.identity.conditional.auth.functions.version>
<carbon.user.api.imp.pkg.version.range>[1.0.1, 2.0.0)</carbon.user.api.imp.pkg.version.range>
<identity.governance.imp.pkg.version.range>[1.4.90, 2.0.0)</identity.governance.imp.pkg.version.range>
<carbon.kernel.version>4.6.1</carbon.kernel.version>
<identity.governance.version>1.4.95</identity.governance.version>
<identity.governance.imp.pkg.version.range>[1.4.90, 2.0.0)</identity.governance.imp.pkg.version.range>
<identity.governance.version>1.4.95</identity.governance.version>
<identity.conditional.auth.functions.common.version>1.1.6</identity.conditional.auth.functions.common.version>
<identity.conditional.auth.functions.common.version>1.2.3</identity.conditional.auth.functions.common.version>
<findbugs-maven-plugin.version>3.0.5</findbugs-maven-plugin.version>
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
Expand Down