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 2016/10/28 06:14:39 UTC

[1/3] git commit: updated refs/heads/4.9 to fcee71f

Repository: cloudstack
Updated Branches:
  refs/heads/4.9 a4b1688f7 -> fcee71f35


CLOUDSTACK-9544: Check access on account trying to generate user API keys

This fixes CVE-2016-6813

Signed-off-by: Marc-Aur�le Brothier <m...@brothier.org>
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/158497d6
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/158497d6
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/158497d6

Branch: refs/heads/4.9
Commit: 158497d68a92ab1e1f864a77371ea1de5c4dc5bb
Parents: 5a2a2f4
Author: Marc-Aur�le Brothier <m...@brothier.org>
Authored: Tue Oct 18 15:33:38 2016 +0200
Committer: Rohit Yadav <ro...@shapeblue.com>
Committed: Thu Oct 27 22:15:49 2016 +0530

----------------------------------------------------------------------
 .../src/com/cloud/user/AccountManagerImpl.java  |   4 +
 test/integration/component/test_accounts.py     | 161 +++++++++++++++++++
 2 files changed, 165 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/158497d6/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 2d9cdb0..0cbbefd 100644
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -2241,6 +2241,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
     @DB
     @ActionEvent(eventType = EventTypes.EVENT_REGISTER_FOR_SECRET_API_KEY, eventDescription = "register for the developer API keys")
     public String[] createApiKeyAndSecretKey(RegisterCmd cmd) {
+        Account caller = CallContext.current().getCallingAccount();
         final Long userId = cmd.getId();
 
         User user = getUserIncludingRemoved(userId);
@@ -2248,6 +2249,9 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             throw new InvalidParameterValueException("unable to find user by id");
         }
 
+        Account account = _accountDao.findById(user.getAccountId());
+        checkAccess(caller, null, true, account);
+
         // don't allow updating system user
         if (user.getId() == User.UID_SYSTEM) {
             throw new PermissionDeniedException("user id : " + user.getId() + " is system account, update is not allowed");

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/158497d6/test/integration/component/test_accounts.py
----------------------------------------------------------------------
diff --git a/test/integration/component/test_accounts.py b/test/integration/component/test_accounts.py
index d0341dd..60421d9 100644
--- a/test/integration/component/test_accounts.py
+++ b/test/integration/component/test_accounts.py
@@ -1531,6 +1531,167 @@ class TestUserLogin(cloudstackTestCase):
         return
 
 
+class TestUserAPIKeys(cloudstackTestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestUserAPIKeys, cls).getClsTestClient()
+        cls.api_client = cls.testClient.getApiClient()
+
+        cls.services = Services().services
+        cls.domain = get_domain(cls.api_client)
+        cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        # Create an account, domain etc
+        cls.domain = Domain.create(
+            cls.api_client,
+            cls.services["domain"],
+        )
+        cls.account = Account.create(
+            cls.api_client,
+            cls.services["account"],
+            admin=False,
+            domainid=cls.domain.id
+        )
+        cls.domain_2 = Domain.create(
+            cls.api_client,
+            cls.services["domain"],
+        )
+        cls.account_2 = Account.create(
+            cls.api_client,
+            cls.services["account"],
+            admin=False,
+            domainid=cls.domain_2.id
+        )
+        cls._cleanup = [
+            cls.account,
+            cls.domain,
+            cls.account_2,
+            cls.domain_2
+        ]
+        return
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            # Cleanup resources used
+            cleanup_resources(cls.api_client, cls._cleanup)
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+        return
+
+    def setUp(self):
+        self.apiclient = self.testClient.getApiClient()
+        self.dbclient = self.testClient.getDbConnection()
+        self.cleanup = []
+        return
+
+    def tearDown(self):
+        try:
+            # Clean up, terminate the created network offerings
+            cleanup_resources(self.apiclient, self.cleanup)
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+        return
+
+    @attr(tags=[
+        "role",
+        "accounts",
+        "simulator",
+        "advanced",
+        "advancedns",
+        "basic",
+        "eip",
+        "sg"
+    ])
+    def test_user_key_renew_same_account(self):
+        # Create an User associated with the account
+        user_1 = User.create(
+            self.apiclient,
+            self.services["user"],
+            account=self.account.name,
+            domainid=self.domain.id
+        )
+        self.cleanup.append(user_1)
+        account_response = list_accounts(
+            self.apiclient,
+            id=self.account.id
+        )[0]
+        self.assertEqual(
+            hasattr(account_response, 'user'),
+            True,
+            "Users are included in account response")
+
+        account_users = account_response.user
+        self.assertEqual(
+            isinstance(account_users, list),
+            True,
+            "Check account for valid data"
+        )
+        self.assertNotEqual(
+            len(account_users),
+            0,
+            "Check number of User in Account")
+        [user] = [u for u in account_users if u.username == user_1.username]
+        self.assertEqual(
+            user.apikey,
+            None,
+            "Check that the user don't have an API key yet")
+
+        self.debug("Register API keys for user")
+        userkeys = User.registerUserKeys(self.apiclient, user_1.id)
+        users = list_accounts(
+            self.apiclient,
+            id=self.account.id
+        )[0].user
+        [user] = [u for u in users if u.id == user_1.id]
+        self.assertEqual(
+            user.apikey,
+            userkeys.apikey,
+            "Check User api key")
+        self.assertEqual(
+            user.secretkey,
+            userkeys.secretkey,
+            "Check User having secret key")
+
+        self.debug("Get test client with user keys")
+        cs_api = self.testClient.getUserApiClient(
+            UserName=self.account.name,
+            DomainName=self.account.domain)
+        self.debug("Renew API keys for user using current keys")
+        new_keys = User.registerUserKeys(cs_api, user_1.id)
+        self.assertNotEqual(
+            userkeys.apikey,
+            new_keys.apikey,
+            "Check API key is different")
+        self.assertNotEqual(
+            userkeys.secretkey,
+            new_keys.secretkey,
+            "Check secret key is different")
+
+    @attr(tags=[
+        "role",
+        "accounts",
+        "simulator",
+        "advanced",
+        "advancedns",
+        "basic",
+        "eip",
+        "sg"
+    ])
+    def test_user_cannot_renew_other_keys(self):
+        cs_api = self.testClient.getUserApiClient(
+            UserName=self.account.name,
+            DomainName=self.account.domain)
+        self.debug("Try to change API key of an account in another domain")
+        users = list_accounts(
+            self.apiclient,
+            id=self.account_2.id
+        )[0].user
+        with self.assertRaises(CloudstackAPIException) as e:
+            User.registerUserKeys(cs_api, users[0].id)
+
+
 class TestDomainForceRemove(cloudstackTestCase):
 
     @classmethod


[3/3] git commit: updated refs/heads/4.9 to fcee71f

Posted by bh...@apache.org.
Merge branch '4.8' into 4.9


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

Branch: refs/heads/4.9
Commit: fcee71f35b71d37ba163621dece284a9c885a1dc
Parents: a4b1688 ec1d1e5
Author: Rohit Yadav <ro...@shapeblue.com>
Authored: Fri Oct 28 11:43:16 2016 +0530
Committer: Rohit Yadav <ro...@shapeblue.com>
Committed: Fri Oct 28 11:43:16 2016 +0530

----------------------------------------------------------------------
 .../src/com/cloud/user/AccountManagerImpl.java  |   4 +
 test/integration/component/test_accounts.py     | 161 +++++++++++++++++++
 2 files changed, 165 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/fcee71f3/server/src/com/cloud/user/AccountManagerImpl.java
----------------------------------------------------------------------


[2/3] git commit: updated refs/heads/4.9 to fcee71f

Posted by bh...@apache.org.
Merge pull request #1742 from shapeblue/cve-2016-6813

CLOUDSTACK-9544: Check access on account trying to generate user API keysThis is to merge Marc's fix on 4.8+ branches.

Tests run:
$ nosetests --with-xunit --xunit-file=test-results.xml --with-marvin --marvin-config=../marvin-cfgs/adv-kvm.cfg  -s -a tags=role --zone=Sandbox-simulator --hypervisor=Simulator  test/integration/component/test_accounts.py

==== Marvin Init Started ====

=== Marvin Parse Config Successful ===

=== Marvin Setting TestData Successful===

==== Log Folder Path: /tmp//MarvinLogs//Oct_27_2016_22_44_32_GVC833. All logs will be available here ====

=== Marvin Init Logging Successful===

==== Marvin Init Successful ====
=== TestName: test_user_cannot_renew_other_keys | Status : SUCCESS ===

=== TestName: test_user_key_renew_same_account | Status : SUCCESS ===

=== TestName: test_updateAdminDetails | Status : SUCCESS ===

=== TestName: test_updateDomainAdminDetails | Status : SUCCESS ===

=== TestName: test_updateUserDetails | Status : SUCCESS ===

===final results are now copied to: /tmp//MarvinLogs/test_accounts_90CDC2===

* pr/1742:
  CLOUDSTACK-9544: Check access on account trying to generate user API keys

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/ec1d1e50
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/ec1d1e50
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/ec1d1e50

Branch: refs/heads/4.9
Commit: ec1d1e50c5bdea785fd5784700b47d561830ff46
Parents: 5a2a2f4 158497d
Author: Rohit Yadav <ro...@shapeblue.com>
Authored: Fri Oct 28 11:31:25 2016 +0530
Committer: Rohit Yadav <ro...@shapeblue.com>
Committed: Fri Oct 28 11:31:25 2016 +0530

----------------------------------------------------------------------
 .../src/com/cloud/user/AccountManagerImpl.java  |   4 +
 test/integration/component/test_accounts.py     | 161 +++++++++++++++++++
 2 files changed, 165 insertions(+)
----------------------------------------------------------------------