mirror of https://github.com/apache/cloudstack.git
Use constant-time comparison functions when checking signatures
This limits the likeliness of timing attacks against the API.
See http://codahale.com/a-lesson-in-timing-attacks/ for the
full rationale.
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Conflicts:
server/src/com/cloud/api/ApiServer.java
server/src/com/cloud/user/AccountManagerImpl.java
(cherry picked from commit 9b4e39e837)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
This commit is contained in:
parent
9a677595fa
commit
162c5af6f8
|
|
@ -908,9 +908,11 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer
|
|||
final SecretKeySpec keySpec = new SecretKeySpec(secretKey.getBytes(), "HmacSHA1");
|
||||
mac.init(keySpec);
|
||||
mac.update(unsignedRequest.getBytes());
|
||||
|
||||
final byte[] encryptedBytes = mac.doFinal();
|
||||
final String computedSignature = Base64.encodeBase64String(encryptedBytes);
|
||||
final boolean equalSig = signature.equals(computedSignature);
|
||||
final boolean equalSig = ConstantTimeComparator.compareStrings(signature, computedSignature);
|
||||
|
||||
if (!equalSig) {
|
||||
s_logger.info("User signature: " + signature + " is not equaled to computed signature: " + computedSignature);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,36 @@
|
|||
// Licensed to the Apache Software Foundation (ASF) under one
|
||||
// or more contributor license agreements. See the NOTICE file
|
||||
// distributed with this work for additional information
|
||||
// regarding copyright ownership. The ASF licenses this file
|
||||
// to you under the Apache License, Version 2.0 (the
|
||||
// "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
package com.cloud.api;
|
||||
|
||||
public class ConstantTimeComparator {
|
||||
|
||||
public static boolean compareBytes(byte[] b1, byte[] b2) {
|
||||
if (b1.length != b2.length) {
|
||||
return false;
|
||||
}
|
||||
|
||||
int result = 0;
|
||||
for (int i = 0; i < b1.length; i++) {
|
||||
result |= b1[i] ^ b2[i];
|
||||
}
|
||||
return result == 0;
|
||||
}
|
||||
|
||||
public static boolean compareStrings(String s1, String s2) {
|
||||
return compareBytes(s1.getBytes(), s2.getBytes());
|
||||
}
|
||||
}
|
||||
|
|
@ -45,6 +45,7 @@ import com.google.gson.GsonBuilder;
|
|||
|
||||
import org.apache.cloudstack.framework.security.keys.KeysManager;
|
||||
|
||||
import com.cloud.api.ConstantTimeComparator;
|
||||
import com.cloud.exception.PermissionDeniedException;
|
||||
import com.cloud.host.HostVO;
|
||||
import com.cloud.server.ManagementServer;
|
||||
|
|
@ -653,7 +654,7 @@ public class ConsoleProxyServlet extends HttpServlet {
|
|||
mac.update(unsignedRequest.getBytes());
|
||||
byte[] encryptedBytes = mac.doFinal();
|
||||
String computedSignature = Base64.encodeBase64String(encryptedBytes);
|
||||
boolean equalSig = signature.equals(computedSignature);
|
||||
boolean equalSig = ConstantTimeComparator.compareStrings(signature, computedSignature);
|
||||
if (!equalSig) {
|
||||
s_logger.debug("User signature: " + signature + " is not equaled to computed signature: " + computedSignature);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -62,6 +62,7 @@ import org.apache.cloudstack.managed.context.ManagedContextRunnable;
|
|||
import org.apache.cloudstack.region.gslb.GlobalLoadBalancerRuleDao;
|
||||
|
||||
import com.cloud.api.ApiDBUtils;
|
||||
import com.cloud.api.ConstantTimeComparator;
|
||||
import com.cloud.api.query.vo.ControlledViewEntity;
|
||||
import com.cloud.configuration.Config;
|
||||
import com.cloud.configuration.ConfigurationManager;
|
||||
|
|
@ -485,6 +486,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
|
|||
|
||||
@Override
|
||||
public void checkAccess(Account caller, AccessType accessType, boolean sameOwner, String apiName, ControlledEntity... entities) {
|
||||
|
||||
//check for the same owner
|
||||
Long ownerId = null;
|
||||
ControlledEntity prevEntity = null;
|
||||
|
|
@ -2058,7 +2060,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
|
|||
mac.update(unsignedRequest.getBytes());
|
||||
byte[] encryptedBytes = mac.doFinal();
|
||||
String computedSignature = new String(Base64.encodeBase64(encryptedBytes));
|
||||
boolean equalSig = signature.equals(computedSignature);
|
||||
boolean equalSig = ConstantTimeComparator.compareStrings(signature, computedSignature);
|
||||
if (!equalSig) {
|
||||
s_logger.info("User signature: " + signature + " is not equaled to computed signature: " + computedSignature);
|
||||
} else {
|
||||
|
|
|
|||
Loading…
Reference in New Issue