diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiErrorCode.java b/api/src/main/java/org/apache/cloudstack/api/ApiErrorCode.java index 03dc37325d4b..616c37484d87 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiErrorCode.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiErrorCode.java @@ -22,6 +22,7 @@ */ public enum ApiErrorCode { + BAD_REQUEST(400), UNAUTHORIZED(401), UNAUTHORIZED2FA(511), METHOD_NOT_ALLOWED(405), diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java index cbbcdc3bda42..cb75939d6bc5 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java @@ -48,4 +48,6 @@ public ResponseObject loginUser(HttpSession session, String username, String pas boolean forgotPassword(UserAccount userAccount, Domain domain); boolean resetPassword(UserAccount userAccount, String token, String password); + + boolean isPostRequestsAndTimestampsEnforced(); } diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index b8227ef9d589..347520009070 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -201,6 +201,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer private static final String SANITIZATION_REGEX = "[\n\r]"; private static boolean encodeApiResponse = false; + private boolean isPostRequestsAndTimestampsEnforced = false; /** * Non-printable ASCII characters - numbers 0 to 31 and 127 decimal @@ -284,6 +285,13 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer , "Session cookie is marked as secure if this is enabled. Secure cookies only work when HTTPS is used." , false , ConfigKey.Scope.Global); + static final ConfigKey EnforcePostRequestsAndTimestamps = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED + , Boolean.class + , "enforce.post.requests.and.timestamps" + , "false" + , "Enable/Disable whether the ApiServer should only accept POST requests for state-changing APIs and requests with timestamps." + , false + , ConfigKey.Scope.Global); private static final ConfigKey JSONDefaultContentType = new ConfigKey<> (ConfigKey.CATEGORY_ADVANCED , String.class , "json.content.type" @@ -441,6 +449,7 @@ protected void setupIntegrationPortListener(Integer apiPort) { public boolean start() { Security.addProvider(new BouncyCastleProvider()); Integer apiPort = IntegrationAPIPort.value(); // api port, null by default + isPostRequestsAndTimestampsEnforced = EnforcePostRequestsAndTimestamps.value(); final Long snapshotLimit = ConcurrentSnapshotsThresholdPerHost.value(); if (snapshotLimit == null || snapshotLimit <= 0) { @@ -720,6 +729,11 @@ public String handleRequest(final Map params, final String responseType, final S return response; } + @Override + public boolean isPostRequestsAndTimestampsEnforced() { + return isPostRequestsAndTimestampsEnforced; + } + private String getBaseAsyncResponse(final long jobId, final BaseAsyncCmd cmd) { final AsyncJobResponse response = new AsyncJobResponse(); @@ -967,7 +981,6 @@ public boolean verifyRequest(final Map requestParameters, fina // put the name in a list that we'll sort later final List parameterNames = new ArrayList<>(requestParameters.keySet()); - Collections.sort(parameterNames); String signatureVersion = null; @@ -1019,12 +1032,22 @@ public boolean verifyRequest(final Map requestParameters, fina } final Date now = new Date(System.currentTimeMillis()); + final Date thresholdTime = new Date(now.getTime() + 15 * 60 * 1000); if (expiresTS.before(now)) { signature = signature.replaceAll(SANITIZATION_REGEX, "_"); apiKey = apiKey.replaceAll(SANITIZATION_REGEX, "_"); logger.debug("Request expired -- ignoring ...sig [{}], apiKey [{}].", signature, apiKey); return false; + } else if (isPostRequestsAndTimestampsEnforced && expiresTS.after(thresholdTime)) { + signature = signature.replaceAll(SANITIZATION_REGEX, "_"); + apiKey = apiKey.replaceAll(SANITIZATION_REGEX, "_"); + logger.debug(String.format("Expiration parameter is set for too long -- ignoring ...sig [%s], apiKey [%s].", signature, apiKey)); + return false; } + } else if (isPostRequestsAndTimestampsEnforced) { + // Force expiration parameter + logger.debug("Signature Version must be 3, and should be along with the Expires parameter -- ignoring request."); + return false; } final TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB); @@ -1648,6 +1671,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] { + EnforcePostRequestsAndTimestamps, IntegrationAPIPort, ConcurrentSnapshotsThresholdPerHost, EncodeApiResponse, diff --git a/server/src/main/java/com/cloud/api/ApiServlet.java b/server/src/main/java/com/cloud/api/ApiServlet.java index 4994c42bb4dc..41229670c38a 100644 --- a/server/src/main/java/com/cloud/api/ApiServlet.java +++ b/server/src/main/java/com/cloud/api/ApiServlet.java @@ -22,8 +22,11 @@ import java.net.UnknownHostException; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; +import java.util.Set; import javax.inject.Inject; import javax.servlet.ServletConfig; @@ -46,6 +49,7 @@ import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.managed.context.ManagedContext; import org.apache.cloudstack.utils.consoleproxy.ConsoleAccessUtils; +import org.apache.commons.collections.MapUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -78,6 +82,39 @@ public class ApiServlet extends HttpServlet { private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServlet.class.getName()); private static final String REPLACEMENT = "_"; private static final String LOGGER_REPLACEMENTS = "[\n\r\t]"; + private static final Pattern GET_REQUEST_COMMANDS = Pattern.compile("^(get|list|query|find)(\\w+)+$"); + private static final HashSet GET_REQUEST_COMMANDS_LIST = new HashSet<>(Set.of("isaccountallowedtocreateofferingswithtags", + "readyforshutdown", "cloudianisenabled", "quotabalance", "quotasummary", "quotatarifflist", "quotaisenabled", "quotastatement", "verifyoauthcodeandgetuser")); + private static final HashSet POST_REQUESTS_TO_DISABLE_LOGGING = new HashSet<>(Set.of( + "login", + "oauthlogin", + "createaccount", + "createuser", + "updateuser", + "forgotpassword", + "resetpassword", + "importrole", + "updaterolepermission", + "updateprojectrolepermission", + "createstoragepool", + "addhost", + "updatehostpassword", + "addcluster", + "addvmwaredc", + "configureoutofbandmanagement", + "uploadcustomcertificate", + "addciscovnmcresource", + "addnetscalerloadbalancer", + "createtungstenfabricprovider", + "addnsxcontroller", + "configtungstenfabricservice", + "createnetworkacl", + "updatenetworkaclitem", + "quotavalidateactivationrule", + "quotatariffupdate", + "listandswitchsamlaccount", + "uploadresourceicon" + )); @Inject ApiServerService apiServer; @@ -193,11 +230,24 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp utf8Fixup(req, params); + final Object[] commandObj = params.get(ApiConstants.COMMAND); + final String command = commandObj == null ? null : (String) commandObj[0]; + // logging the request start and end in management log for easy debugging String reqStr = ""; String cleanQueryString = StringUtils.cleanString(req.getQueryString()); if (LOGGER.isDebugEnabled()) { reqStr = auditTrailSb.toString() + " " + cleanQueryString; + if (req.getMethod().equalsIgnoreCase("POST") && org.apache.commons.lang3.StringUtils.isNotBlank(command)) { + if (!POST_REQUESTS_TO_DISABLE_LOGGING.contains(command.toLowerCase()) && !reqParams.containsKey(ApiConstants.USER_DATA)) { + String cleanParamsString = getCleanParamsString(reqParams); + if (org.apache.commons.lang3.StringUtils.isNotBlank(cleanParamsString)) { + reqStr += "\n" + cleanParamsString; + } + } else { + reqStr += " " + command; + } + } LOGGER.debug("===START=== " + reqStr); } @@ -213,8 +263,6 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp responseType = (String)responseTypeParam[0]; } - final Object[] commandObj = params.get(ApiConstants.COMMAND); - final String command = commandObj == null ? null : (String) commandObj[0]; final Object[] userObj = params.get(ApiConstants.USERNAME); String username = userObj == null ? null : (String)userObj[0]; if (LOGGER.isTraceEnabled()) { @@ -317,6 +365,19 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp } } + if (apiServer.isPostRequestsAndTimestampsEnforced() && !isStateChangingCommandUsingPOST(command, req.getMethod(), params)) { + String errorText = String.format("State changing command %s needs to be sent using POST request", command); + if (command.equalsIgnoreCase("updateConfiguration") && params.containsKey("name")) { + errorText = String.format("Changes for configuration %s needs to be sent using POST request", params.get("name")[0]); + } + auditTrailSb.append(" " + HttpServletResponse.SC_BAD_REQUEST + " " + errorText); + final String serializedResponse = + apiServer.getSerializedApiError(new ServerApiException(ApiErrorCode.BAD_REQUEST, errorText), params, + responseType); + HttpUtils.writeHttpResponse(resp, serializedResponse, HttpServletResponse.SC_BAD_REQUEST, responseType, ApiServer.JSONcontentType.value()); + return; + } + Long userId = null; if (!isNew) { userId = (Long)session.getAttribute("userid"); @@ -407,6 +468,15 @@ private boolean checkIfAuthenticatorIsOf2FA(String command) { return verify2FA; } + private boolean isStateChangingCommandUsingPOST(String command, String method, Map params) { + if (command == null || (!GET_REQUEST_COMMANDS.matcher(command.toLowerCase()).matches() && !GET_REQUEST_COMMANDS_LIST.contains(command.toLowerCase()) + && !command.equalsIgnoreCase("updateConfiguration") && !method.equals("POST"))) { + return false; + } + return !command.equalsIgnoreCase("updateConfiguration") || method.equals("POST") || (params.containsKey("name") + && params.get("name")[0].toString().equalsIgnoreCase(ApiServer.EnforcePostRequestsAndTimestamps.key())); + } + protected boolean skip2FAcheckForAPIs(String command) { boolean skip2FAcheck = false; @@ -644,4 +714,45 @@ private static String getCorrectIPAddress(String ip) { } return null; } + + private String getCleanParamsString(Map reqParams) { + if (MapUtils.isEmpty(reqParams)) { + return ""; + } + + StringBuilder cleanParamsString = new StringBuilder(); + for (Map.Entry reqParam : reqParams.entrySet()) { + if (org.apache.commons.lang3.StringUtils.isBlank(reqParam.getKey())) { + continue; + } + + cleanParamsString.append(reqParam.getKey()); + cleanParamsString.append("="); + + if (reqParam.getKey().toLowerCase().contains("password") + || reqParam.getKey().toLowerCase().contains("privatekey") + || reqParam.getKey().toLowerCase().contains("accesskey") + || reqParam.getKey().toLowerCase().contains("secretkey")) { + cleanParamsString.append("\n"); + continue; + } + + if (reqParam.getValue() == null || reqParam.getValue().length == 0) { + cleanParamsString.append("\n"); + continue; + } + + for (String param : reqParam.getValue()) { + if (org.apache.commons.lang3.StringUtils.isBlank(param)) { + continue; + } + String cleanParamString = StringUtils.cleanString(param.trim()); + cleanParamsString.append(cleanParamString); + cleanParamsString.append(" "); + } + cleanParamsString.append("\n"); + } + + return cleanParamsString.toString(); + } } diff --git a/ui/src/api/index.js b/ui/src/api/index.js index 7ab87780a9d4..85c46c483b24 100644 --- a/ui/src/api/index.js +++ b/ui/src/api/index.js @@ -23,18 +23,10 @@ import { ACCESS_TOKEN } from '@/store/mutation-types' -export function api (command, args = {}, method = 'GET', data = {}) { - let params = {} +export function getAPI (command, args = {}) { args.command = command args.response = 'json' - if (data) { - params = new URLSearchParams() - Object.entries(data).forEach(([key, value]) => { - params.append(key, value) - }) - } - const sessionkey = vueProps.$localStorage.get(ACCESS_TOKEN) || Cookies.get('sessionkey') if (sessionkey) { args.sessionkey = sessionkey @@ -45,8 +37,30 @@ export function api (command, args = {}, method = 'GET', data = {}) { ...args }, url: '/', - method, - data: params || {} + method: 'GET' + }) +} + +export function postAPI (command, data = {}) { + const params = new URLSearchParams() + params.append('command', command) + params.append('response', 'json') + if (data) { + Object.entries(data).forEach(([key, value]) => { + if (value !== undefined && value !== null && value !== '') { + params.append(key, value) + } + }) + } + + const sessionkey = vueProps.$localStorage.get(ACCESS_TOKEN) || Cookies.get('sessionkey') + if (sessionkey) { + params.append('sessionkey', sessionkey) + } + return axios({ + url: '/', + method: 'POST', + data: params }) } @@ -56,7 +70,7 @@ export function login (arg) { } // Logout before login is called to purge any duplicate sessionkey cookies - api('logout') + postAPI('logout') const params = new URLSearchParams() params.append('command', 'login') @@ -66,7 +80,7 @@ export function login (arg) { params.append('response', 'json') return axios({ url: '/', - method: 'post', + method: 'POST', data: params, headers: { 'content-type': 'application/x-www-form-urlencoded' @@ -77,7 +91,7 @@ export function login (arg) { export function logout () { message.destroy() notification.destroy() - return api('logout') + return postAPI('logout') } export function oauthlogin (arg) { @@ -86,7 +100,7 @@ export function oauthlogin (arg) { } // Logout before login is called to purge any duplicate sessionkey cookies - api('logout') + postAPI('logout') const params = new URLSearchParams() params.append('command', 'oauthlogin') diff --git a/ui/src/components/header/SamlDomainSwitcher.vue b/ui/src/components/header/SamlDomainSwitcher.vue index 082bab7bf13c..e0799bef4d83 100644 --- a/ui/src/components/header/SamlDomainSwitcher.vue +++ b/ui/src/components/header/SamlDomainSwitcher.vue @@ -51,7 +51,7 @@