You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by bh...@apache.org on 2015/01/14 16:13:55 UTC

[1/2] git commit: updated refs/heads/4.4 to 7cb19f5

Repository: cloudstack
Updated Branches:
  refs/heads/4.4 701925312 -> 7cb19f5aa


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 <ro...@shapeblue.com>

Conflicts:
	server/src/com/cloud/api/ApiServer.java
	server/src/com/cloud/user/AccountManagerImpl.java

(cherry picked from commit 9b4e39e837af498599859c4a6687eb8bf9f8ad89)
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/7f45df35
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/7f45df35
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/7f45df35

Branch: refs/heads/4.4
Commit: 7f45df359395758b3070fbe2c65e90c3d5d88516
Parents: 7019253
Author: Pierre-Yves Ritschard <py...@spootnik.org>
Authored: Wed Jan 14 11:27:35 2015 +0100
Committer: Rohit Yadav <ro...@shapeblue.com>
Committed: Wed Jan 14 20:40:30 2015 +0530

----------------------------------------------------------------------
 server/src/com/cloud/api/ApiServer.java         |  4 ++-
 .../com/cloud/api/ConstantTimeComparator.java   | 36 ++++++++++++++++++++
 .../com/cloud/servlet/ConsoleProxyServlet.java  |  3 +-
 .../src/com/cloud/user/AccountManagerImpl.java  |  4 ++-
 4 files changed, 44 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7f45df35/server/src/com/cloud/api/ApiServer.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java
index 19a183e..35ff809 100755
--- a/server/src/com/cloud/api/ApiServer.java
+++ b/server/src/com/cloud/api/ApiServer.java
@@ -891,9 +891,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 {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7f45df35/server/src/com/cloud/api/ConstantTimeComparator.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ConstantTimeComparator.java b/server/src/com/cloud/api/ConstantTimeComparator.java
new file mode 100644
index 0000000..4612eee
--- /dev/null
+++ b/server/src/com/cloud/api/ConstantTimeComparator.java
@@ -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());
+    }
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7f45df35/server/src/com/cloud/servlet/ConsoleProxyServlet.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/com/cloud/servlet/ConsoleProxyServlet.java
index 60f32cf..61fc704 100644
--- a/server/src/com/cloud/servlet/ConsoleProxyServlet.java
+++ b/server/src/com/cloud/servlet/ConsoleProxyServlet.java
@@ -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;
@@ -657,7 +658,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);
             }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7f45df35/server/src/com/cloud/user/AccountManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
index decbedf..e5b8ab4 100755
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -61,6 +61,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;
@@ -488,6 +489,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;
@@ -2074,7 +2076,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 {


[2/2] git commit: updated refs/heads/4.4 to 7cb19f5

Posted by bh...@apache.org.
move ConstantTimeComparator to utils

(cherry picked from commit b2393c31ed8f689e45227f12371fc042c9dbd0e4)
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>

Conflicts:
	server/src/com/cloud/api/ApiServer.java


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/7cb19f5a
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/7cb19f5a
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/7cb19f5a

Branch: refs/heads/4.4
Commit: 7cb19f5aa4c3bf10bccd086cb54ce7b089b93663
Parents: 7f45df3
Author: Pierre-Yves Ritschard <py...@spootnik.org>
Authored: Wed Jan 14 12:14:00 2015 +0100
Committer: Rohit Yadav <ro...@shapeblue.com>
Committed: Wed Jan 14 20:42:17 2015 +0530

----------------------------------------------------------------------
 server/src/com/cloud/api/ApiServer.java         |  1 +
 .../com/cloud/api/ConstantTimeComparator.java   | 36 ------------------
 .../com/cloud/servlet/ConsoleProxyServlet.java  |  2 +-
 .../src/com/cloud/user/AccountManagerImpl.java  |  2 +-
 .../com/cloud/utils/ConstantTimeComparator.java | 39 ++++++++++++++++++++
 5 files changed, 42 insertions(+), 38 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cb19f5a/server/src/com/cloud/api/ApiServer.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java
index 35ff809..b62745d 100755
--- a/server/src/com/cloud/api/ApiServer.java
+++ b/server/src/com/cloud/api/ApiServer.java
@@ -158,6 +158,7 @@ import com.cloud.user.DomainManager;
 import com.cloud.user.User;
 import com.cloud.user.UserAccount;
 import com.cloud.user.UserVO;
+import com.cloud.utils.ConstantTimeComparator;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
 import com.cloud.utils.StringUtils;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cb19f5a/server/src/com/cloud/api/ConstantTimeComparator.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ConstantTimeComparator.java b/server/src/com/cloud/api/ConstantTimeComparator.java
deleted file mode 100644
index 4612eee..0000000
--- a/server/src/com/cloud/api/ConstantTimeComparator.java
+++ /dev/null
@@ -1,36 +0,0 @@
-// 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());
-    }
-}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cb19f5a/server/src/com/cloud/servlet/ConsoleProxyServlet.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/servlet/ConsoleProxyServlet.java b/server/src/com/cloud/servlet/ConsoleProxyServlet.java
index 61fc704..1053aa2 100644
--- a/server/src/com/cloud/servlet/ConsoleProxyServlet.java
+++ b/server/src/com/cloud/servlet/ConsoleProxyServlet.java
@@ -45,7 +45,6 @@ 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;
@@ -54,6 +53,7 @@ import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
 import com.cloud.user.User;
 import com.cloud.uservm.UserVm;
+import com.cloud.utils.ConstantTimeComparator;
 import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.db.EntityManager;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cb19f5a/server/src/com/cloud/user/AccountManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
index e5b8ab4..66064e7 100755
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -61,7 +61,6 @@ 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;
@@ -132,6 +131,7 @@ import com.cloud.user.Account.State;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.user.dao.UserAccountDao;
 import com.cloud.user.dao.UserDao;
+import com.cloud.utils.ConstantTimeComparator;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7cb19f5a/utils/src/com/cloud/utils/ConstantTimeComparator.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/ConstantTimeComparator.java b/utils/src/com/cloud/utils/ConstantTimeComparator.java
new file mode 100644
index 0000000..4d4a595
--- /dev/null
+++ b/utils/src/com/cloud/utils/ConstantTimeComparator.java
@@ -0,0 +1,39 @@
+//
+// 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.utils;
+
+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());
+    }
+}