Skip to content

Commit 1928fe7

Browse files
sureshanapartiKevin Li
andcommitted
Support ApiServer to enforce POST requests with timestamps for state changing APIs
Co-authored-by: Kevin Li <[email protected]>
1 parent 99863c2 commit 1928fe7

File tree

4 files changed

+56
-1
lines changed

4 files changed

+56
-1
lines changed

api/src/main/java/org/apache/cloudstack/api/ApiErrorCode.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
*/
2323
public enum ApiErrorCode {
2424

25+
BAD_REQUEST(400),
2526
UNAUTHORIZED(401),
2627
UNAUTHORIZED2FA(511),
2728
METHOD_NOT_ALLOWED(405),

api/src/main/java/org/apache/cloudstack/api/ApiServerService.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,6 @@ public ResponseObject loginUser(HttpSession session, String username, String pas
4848
boolean forgotPassword(UserAccount userAccount, Domain domain);
4949

5050
boolean resetPassword(UserAccount userAccount, String token, String password);
51+
52+
boolean isEnforcePostRequestsAndTimestamps();
5153
}

server/src/main/java/com/cloud/api/ApiServer.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
201201
private static final String SANITIZATION_REGEX = "[\n\r]";
202202

203203
private static boolean encodeApiResponse = false;
204+
private boolean isEnforcePostRequestsAndTimestamps = false;
204205

205206
/**
206207
* Non-printable ASCII characters - numbers 0 to 31 and 127 decimal
@@ -284,6 +285,13 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
284285
, "Session cookie is marked as secure if this is enabled. Secure cookies only work when HTTPS is used."
285286
, false
286287
, ConfigKey.Scope.Global);
288+
static final ConfigKey<Boolean> EnforcePostRequestsAndTimestamps = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED
289+
, Boolean.class
290+
, "enable.enforce.post.requests.and.timestamps"
291+
, "false"
292+
, "enables/disables whether the ApiServer only accepts state-changing POST requests and requests with timestamps."
293+
, false
294+
, ConfigKey.Scope.Global);
287295
private static final ConfigKey<String> JSONDefaultContentType = new ConfigKey<> (ConfigKey.CATEGORY_ADVANCED
288296
, String.class
289297
, "json.content.type"
@@ -441,6 +449,7 @@ protected void setupIntegrationPortListener(Integer apiPort) {
441449
public boolean start() {
442450
Security.addProvider(new BouncyCastleProvider());
443451
Integer apiPort = IntegrationAPIPort.value(); // api port, null by default
452+
isEnforcePostRequestsAndTimestamps = EnforcePostRequestsAndTimestamps.value();
444453

445454
final Long snapshotLimit = ConcurrentSnapshotsThresholdPerHost.value();
446455
if (snapshotLimit == null || snapshotLimit <= 0) {
@@ -720,6 +729,11 @@ public String handleRequest(final Map params, final String responseType, final S
720729
return response;
721730
}
722731

732+
@Override
733+
public boolean isEnforcePostRequestsAndTimestamps() {
734+
return isEnforcePostRequestsAndTimestamps;
735+
}
736+
723737
private String getBaseAsyncResponse(final long jobId, final BaseAsyncCmd cmd) {
724738
final AsyncJobResponse response = new AsyncJobResponse();
725739

@@ -967,7 +981,6 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
967981

968982
// put the name in a list that we'll sort later
969983
final List<String> parameterNames = new ArrayList<>(requestParameters.keySet());
970-
971984
Collections.sort(parameterNames);
972985

973986
String signatureVersion = null;
@@ -1019,12 +1032,22 @@ public boolean verifyRequest(final Map<String, Object[]> requestParameters, fina
10191032
}
10201033

10211034
final Date now = new Date(System.currentTimeMillis());
1035+
final Date thresholdTime = new Date(now.getTime() + 15 * 60 * 1000);
10221036
if (expiresTS.before(now)) {
10231037
signature = signature.replaceAll(SANITIZATION_REGEX, "_");
10241038
apiKey = apiKey.replaceAll(SANITIZATION_REGEX, "_");
10251039
logger.debug("Request expired -- ignoring ...sig [{}], apiKey [{}].", signature, apiKey);
10261040
return false;
1041+
} else if (isEnforcePostRequestsAndTimestamps && expiresTS.after(thresholdTime)) {
1042+
signature = signature.replaceAll(SANITIZATION_REGEX, "_");
1043+
apiKey = apiKey.replaceAll(SANITIZATION_REGEX, "_");
1044+
logger.debug(String.format("Expiration parameter is set for too long -- ignoring ...sig [%s], apiKey [%s].", signature, apiKey));
1045+
return false;
10271046
}
1047+
} else if (isEnforcePostRequestsAndTimestamps) {
1048+
// Force expiration parameter
1049+
logger.debug("Signature Version must be 3, and should be along with the Expires parameter -- ignoring request.");
1050+
return false;
10281051
}
10291052

10301053
final TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB);
@@ -1648,6 +1671,7 @@ public String getConfigComponentName() {
16481671
@Override
16491672
public ConfigKey<?>[] getConfigKeys() {
16501673
return new ConfigKey<?>[] {
1674+
EnforcePostRequestsAndTimestamps,
16511675
IntegrationAPIPort,
16521676
ConcurrentSnapshotsThresholdPerHost,
16531677
EncodeApiResponse,

server/src/main/java/com/cloud/api/ApiServlet.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,11 @@
2222
import java.net.UnknownHostException;
2323
import java.util.Arrays;
2424
import java.util.HashMap;
25+
import java.util.HashSet;
2526
import java.util.List;
2627
import java.util.Map;
28+
import java.util.regex.Pattern;
29+
import java.util.Set;
2730

2831
import javax.inject.Inject;
2932
import javax.servlet.ServletConfig;
@@ -78,6 +81,9 @@ public class ApiServlet extends HttpServlet {
7881
private static final Logger ACCESSLOGGER = LogManager.getLogger("apiserver." + ApiServlet.class.getName());
7982
private static final String REPLACEMENT = "_";
8083
private static final String LOGGER_REPLACEMENTS = "[\n\r\t]";
84+
private static final Pattern GET_REQUEST_COMMANDS = Pattern.compile("^(get|list|query|find)(\\w+)+$");
85+
private static final HashSet<String> GET_REQUEST_COMMANDS_LIST = new HashSet<String>(Set.of("isaccountallowedtocreateofferingswithtags",
86+
"readyforshutdown", "cloudianisenabled", "quotabalance", "quotasummary", "quotatarifflist", "quotaisenabled", "quotastatement"));
8187

8288
@Inject
8389
ApiServerService apiServer;
@@ -317,6 +323,19 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp
317323
}
318324
}
319325

326+
if (apiServer.isEnforcePostRequestsAndTimestamps() && !isStateChangingCommandUsingPOST(command, req.getMethod(), params)) {
327+
String errorText = String.format("State changing command %s needs to be sent using POST request", command);
328+
if (command.equalsIgnoreCase("updateConfiguration") && params.containsKey("name")) {
329+
errorText = String.format("Changes for configuration %s needs to be sent using POST request", params.get("name")[0]);
330+
}
331+
auditTrailSb.append(" " + HttpServletResponse.SC_BAD_REQUEST + " " + errorText);
332+
final String serializedResponse =
333+
apiServer.getSerializedApiError(new ServerApiException(ApiErrorCode.BAD_REQUEST, errorText), params,
334+
responseType);
335+
HttpUtils.writeHttpResponse(resp, serializedResponse, HttpServletResponse.SC_BAD_REQUEST, responseType, ApiServer.JSONcontentType.value());
336+
return;
337+
}
338+
320339
Long userId = null;
321340
if (!isNew) {
322341
userId = (Long)session.getAttribute("userid");
@@ -407,6 +426,15 @@ private boolean checkIfAuthenticatorIsOf2FA(String command) {
407426
return verify2FA;
408427
}
409428

429+
private boolean isStateChangingCommandUsingPOST(String command, String method, Map<String, Object[]> params) {
430+
if (command == null || (!GET_REQUEST_COMMANDS.matcher(command.toLowerCase()).matches() && !GET_REQUEST_COMMANDS_LIST.contains(command.toLowerCase())
431+
&& !command.equalsIgnoreCase("updateConfiguration") && !method.equals("POST"))) {
432+
return false;
433+
}
434+
return !command.equalsIgnoreCase("updateConfiguration") || method.equals("POST") || (params.containsKey("name")
435+
&& params.get("name")[0].toString().equalsIgnoreCase(ApiServer.EnforcePostRequestsAndTimestamps.key()));
436+
}
437+
410438
protected boolean skip2FAcheckForAPIs(String command) {
411439
boolean skip2FAcheck = false;
412440

0 commit comments

Comments
 (0)