[GitHub] mrunalinikankariya commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

mrunalinikankariya commented on issue #2243: 
CLOUDSTACK-10019:template.properties has hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324537219
 
 
   Test failure doesn't seem to be relevant to the change for this ticket
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] SudharmaJain commented on issue #876: CLOUDSTACK-8865: Adding SR doesn't create Storage_pool_host_ref entry?

[git]

SudharmaJain commented on issue #876: CLOUDSTACK-8865: Adding SR doesn't create 
Storage_pool_host_ref entry?
URL: https://github.com/apache/cloudstack/pull/876#issuecomment-324538684
 
 
   tag:mergeready
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] SudharmaJain commented on issue #1733: CLOUDSTACK-9563 ExtractTemplate returns malformed URL after migrating?

[git]

SudharmaJain commented on issue #1733: CLOUDSTACK-9563 ExtractTemplate returns 
malformed URL after migrating?
URL: https://github.com/apache/cloudstack/pull/1733#issuecomment-324537402
 
 
   @jburwell Do we have the ability to execute s3 specific marvin tests? 
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] mrunalinikankariya commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

mrunalinikankariya commented on issue #2243: 
CLOUDSTACK-10019:template.properties has hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324537219
 
 
   Test failure doesn't seem to be relevant to the change for this change
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] SudharmaJain commented on issue #1733: CLOUDSTACK-9563 ExtractTemplate returns malformed URL after migrating?

[git]

SudharmaJain commented on issue #1733: CLOUDSTACK-9563 ExtractTemplate returns 
malformed URL after migrating?
URL: https://github.com/apache/cloudstack/pull/1733#issuecomment-324536874
 
 
   Here are some manual test results.
   
   Before applying the fix for template download I see following url.
   
   
![image](https://user-images.githubusercontent.com/12229259/29650828-f7deb394-88ba-11e7-916d-79f7d56b498e.png)
   
   After applying the fix I see following url.
   
   
![image](https://user-images.githubusercontent.com/12229259/29650841-132721e0-88bb-11e7-8458-2d1d705bb321.png)
   
   
   
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties 
has hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324528686
 
 
   Trillian test result (tid-1413)
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 31167 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2243-t1413-kvm-centos7.zip
   Intermitten failure detected: /marvin/tests/smoke/test_iso.py
   Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Test completed. 54 look OK, 3 have error(s)
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_vpc_remote_access_vpn | `Failure` | 55.89 | test_vpc_vpn.py
   test_04_rvpc_privategw_static_routes | `Failure` | 311.09 | 
test_privategw_acl.py
   test_05_iso_permissions | `Failure` | 0.06 | test_iso.py
   test_02_edit_iso | `Failure` | 0.05 | test_iso.py
   test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | 
test_vm_snapshots.py
   test_09_copy_delete_template | Skipped | 0.02 | test_templates.py
   test_06_copy_template | Skipped | 0.00 | test_templates.py
   test_static_role_account_acls | Skipped | 0.02 | test_staticroles.py
   test_11_ss_nfs_version_on_ssvm | Skipped | 0.03 | test_ssvm.py
   test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
   test_01_primary_storage_iscsi | Skipped | 0.04 | test_primary_storage.py
   test_vm_nic_adapter_vmxnet3 | Skipped | 0.00 | test_nic_adapter_type.py
   test_nested_virtualization_vmware | Skipped | 0.00 | 
test_nested_virtualization.py
   test_06_copy_iso | Skipped | 0.00 | test_iso.py
   test_deploy_vgpu_enabled_vm | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.py
   test_3d_gpu_support | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.py
   
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] cloudmonger commented on issue #2214: Speed-up VR initialisation/configuration

[git]

cloudmonger commented on issue #2214: Speed-up VR initialisation/configuration
URL: https://github.com/apache/cloudstack/pull/2214#issuecomment-323870070
 
 
   ### ACS CI BVT Run
**Sumarry:**
Build Number 1124
Hypervisor xenserver
NetworkType Advanced
Passed=110
Failed=4
Skipped=12
   
   _Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/r2si930m8xxzavs/AAAzNrnoF1fC3auFrvsKo_8-a?dl=0
   
   **Failed tests:**
   * test_routers_network_ops.py
   
* test_01_isolate_network_FW_PF_default_routes_egress_true Failing since 
122 runs
   
* test_02_isolate_network_FW_PF_default_routes_egress_false Failing since 
122 runs
   
* test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failing since 
118 runs
   
* test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failing since 
118 runs
   
   
   **Skipped tests:**
   test_vm_nic_adapter_vmxnet3
   test_01_verify_libvirt
   test_02_verify_libvirt_after_restart
   test_03_verify_libvirt_attach_disk
   test_04_verify_guest_lspci
   test_05_change_vm_ostype_restart
   test_06_verify_guest_lspci_again
   test_static_role_account_acls
   test_11_ss_nfs_version_on_ssvm
   test_nested_virtualization_vmware
   test_3d_gpu_support
   test_deploy_vgpu_enabled_vm
   
   **Passed test suits:**
   test_deploy_vm_with_userdata.py
   test_affinity_groups_projects.py
   test_portable_publicip.py
   test_vm_snapshots.py
   test_over_provisioning.py
   test_global_settings.py
   test_router_dnsservice.py
   test_scale_vm.py
   test_service_offerings.py
   test_routers_iptables_default_policy.py
   test_loadbalance.py
   test_routers.py
   test_reset_vm_on_reboot.py
   test_deploy_vms_with_varied_deploymentplanners.py
   test_network.py
   test_router_dns.py
   test_non_contigiousvlan.py
   test_login.py
   test_deploy_vm_iso.py
   test_list_ids_parameter.py
   test_public_ip_range.py
   test_multipleips_per_nic.py
   test_metrics_api.py
   test_regions.py
   test_affinity_groups.py
   test_network_acl.py
   test_pvlan.py
   test_volumes.py
   test_nic.py
   test_deploy_vm_root_resize.py
   test_resource_detail.py
   test_secondary_storage.py
   test_vm_life_cycle.py
   test_disk_offerings.py
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] cloudmonger commented on issue #2214: Speed-up VR initialisation/configuration

[git]

cloudmonger commented on issue #2214: Speed-up VR initialisation/configuration
URL: https://github.com/apache/cloudstack/pull/2214#issuecomment-324528552
 
 
   ### ACS CI BVT Run
**Sumarry:**
Build Number 1137
Hypervisor xenserver
NetworkType Advanced
Passed=105
Failed=5
Skipped=12
   
   _Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/r2si930m8xxzavs/AAAzNrnoF1fC3auFrvsKo_8-a?dl=0
   
   **Failed tests:**
   * test_loadbalance.py
   
* ContextSuite context=TestLoadBalance>:setup Failing since 11 runs
   
   * test_non_contigiousvlan.py
   
* test_extendPhysicalNetworkVlan Failing since 4 runs
   
   * test_routers_network_ops.py
   
* test_01_isolate_network_FW_PF_default_routes_egress_true Failing since 2 
runs
   
* test_02_isolate_network_FW_PF_default_routes_egress_false Failing since 
129 runs
   
* ContextSuite context=TestRedundantIsolateNetworks>:setup Failing since 10 
runs
   
   
   **Skipped tests:**
   test_vm_nic_adapter_vmxnet3
   test_01_verify_libvirt
   test_02_verify_libvirt_after_restart
   test_03_verify_libvirt_attach_disk
   test_04_verify_guest_lspci
   test_05_change_vm_ostype_restart
   test_06_verify_guest_lspci_again
   test_static_role_account_acls
   test_11_ss_nfs_version_on_ssvm
   test_nested_virtualization_vmware
   test_3d_gpu_support
   test_deploy_vgpu_enabled_vm
   
   **Passed test suits:**
   test_deploy_vm_with_userdata.py
   test_affinity_groups_projects.py
   test_portable_publicip.py
   test_vm_snapshots.py
   test_over_provisioning.py
   test_global_settings.py
   test_router_dnsservice.py
   test_scale_vm.py
   test_service_offerings.py
   test_routers_iptables_default_policy.py
   test_routers.py
   test_reset_vm_on_reboot.py
   test_deploy_vms_with_varied_deploymentplanners.py
   test_network.py
   test_router_dns.py
   test_login.py
   test_deploy_vm_iso.py
   test_list_ids_parameter.py
   test_public_ip_range.py
   test_multipleips_per_nic.py
   test_metrics_api.py
   test_regions.py
   test_affinity_groups.py
   test_network_acl.py
   test_pvlan.py
   test_volumes.py
   test_nic.py
   test_deploy_vm_root_resize.py
   test_resource_detail.py
   test_secondary_storage.py
   test_vm_life_cycle.py
   test_disk_offerings.py
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd closed pull request #2123: CLOUDSTACK-9914: update Quota plugin to support currency values up to 5 decimal places

[git]

rhtyd closed pull request #2123: CLOUDSTACK-9914: update Quota plugin to 
support currency values up to 5 decimal places
URL: https://github.com/apache/cloudstack/pull/2123
 
 
   
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2123: CLOUDSTACK-9914: update Quota plugin to support currency values up to 5 decimal places

[git]

rhtyd commented on issue #2123: CLOUDSTACK-9914: update Quota plugin to support 
currency values up to 5 decimal places
URL: https://github.com/apache/cloudstack/pull/2123#issuecomment-324441049
 
 
   LGTM, a db change validated with Travis and BVT is okay.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[cloudstack] branch master updated: CLOUDSTACK-9914: Update Quota plugin to support currency values up to 5 decimal places (#2123)

[bhaisaab]

This is an automated email from the ASF dual-hosted git repository.

bhaisaab pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/master by this push:
 new 57255ac  CLOUDSTACK-9914: Update Quota plugin to support currency 
values up to 5 decimal places (#2123)
57255ac is described below

commit 57255ac72c99ee667a4d0ce765d67acbe4cc25ac
Author: Gabriel Beims Bräscher 
AuthorDate: Wed Aug 23 16:40:48 2017 -0300

CLOUDSTACK-9914: Update Quota plugin to support currency values up to 5 
decimal places (#2123)

Summary: this commit alters column currency_value from table
cloud_usage.quota_tariff to support values up to 5 decimal places. The
current implementation allows up to 2 decimal places.

Issue: need to use more than 2 decimal places to define resources values
in Quota tariff.

Solution: modify column currency_value from table
cloud_usage.quota_tariff to support values up to 5 decimal places.
Values with more than 5 decimal places will be displayed with scientific
notation in the user interface.

SQL command: "ALTER TABLE cloud_usage.quota_tariff MODIFY currency_value
DECIMAL(15,5) not null"
---
 setup/db/db/schema-41000to41100-cleanup.sql | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/setup/db/db/schema-41000to41100-cleanup.sql 
b/setup/db/db/schema-41000to41100-cleanup.sql
index 7fea017..60bc535 100644
--- a/setup/db/db/schema-41000to41100-cleanup.sql
+++ b/setup/db/db/schema-41000to41100-cleanup.sql
@@ -18,3 +18,6 @@
 --;
 -- Schema upgrade cleanup from 4.10.0.0 to 4.11.0.0
 --;
+
+-- CLOUDSTACK-9914: Alter quota_tariff to support currency values up to 5 
decimal places
+ALTER TABLE `cloud_usage`.`quota_tariff` MODIFY `currency_value` DECIMAL(15,5) 
not null

-- 
To stop receiving notification emails like this one, please contact
['"commits@cloudstack.apache.org" '].

[GitHub] blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties 
has hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324441005
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been 
kicked to run smoke tests
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2206: [CLOUDSTACK-10020] Changes to make marvin work with projects and VPCs

[git]

blueorangutan commented on issue #2206: [CLOUDSTACK-10020] Changes to make 
marvin work with projects and VPCs
URL: https://github.com/apache/cloudstack/pull/2206#issuecomment-324441010
 
 
   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been 
kicked to run smoke tests
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

rhtyd commented on issue #2243: CLOUDSTACK-10019:template.properties has 
hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324440907
 
 
   @blueorangutan test
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2206: [CLOUDSTACK-10020] Changes to make marvin work with projects and VPCs

[git]

rhtyd commented on issue #2206: [CLOUDSTACK-10020] Changes to make marvin work 
with projects and VPCs
URL: https://github.com/apache/cloudstack/pull/2206#issuecomment-324440857
 
 
   @blueorangutan test
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rafaelweingartner commented on issue #2123: CLOUDSTACK-9914: update Quota plugin to support currency values up to 5 decimal places

[git]

rafaelweingartner commented on issue #2123: CLOUDSTACK-9914: update Quota 
plugin to support currency values up to 5 decimal places
URL: https://github.com/apache/cloudstack/pull/2123#issuecomment-324398815
 
 
   @DaanHoogland or @swill can one of will merge this one?
   Everything seems to be ok, but I am not able to execute merges.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties 
has hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324392571
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1016
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2246: CLOUDSTACK-10046 checksum validation for any java supported Digests-type

[git]

blueorangutan commented on issue #2246: CLOUDSTACK-10046 checksum validation 
for any java supported Digests-type
URL: https://github.com/apache/cloudstack/pull/2246#issuecomment-324392344
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1015
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2206: [CLOUDSTACK-10020] Changes to make marvin work with projects and VPCs

[git]

blueorangutan commented on issue #2206: [CLOUDSTACK-10020] Changes to make 
marvin work with projects and VPCs
URL: https://github.com/apache/cloudstack/pull/2206#issuecomment-324387645
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1014
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

blueorangutan commented on issue #2239: CLOUDSTACK-9993: Securing Agents 
Communications
URL: https://github.com/apache/cloudstack/pull/2239#issuecomment-324387473
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1012
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2217: [4.9] Smoketest health checkrun

[git]

blueorangutan commented on issue #2217: [4.9] Smoketest health checkrun
URL: https://github.com/apache/cloudstack/pull/2217#issuecomment-324387480
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1013
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

blueorangutan commented on issue #2243: CLOUDSTACK-10019:template.properties 
has hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324378070
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2243: CLOUDSTACK-10019:template.properties has hardcoded id

[git]

rhtyd commented on issue #2243: CLOUDSTACK-10019:template.properties has 
hardcoded id
URL: https://github.com/apache/cloudstack/pull/2243#issuecomment-324378008
 
 
   @blueorangutan package
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #1985: CLOUDSTACK-9812:Update "updatePortForwardingRule" api to include additional parameter to update the end port in case of port range

[git]

rhtyd commented on a change in pull request #1985: CLOUDSTACK-9812:Update 
"updatePortForwardingRule" api to include additional parameter to update the 
end port in case of port range
URL: https://github.com/apache/cloudstack/pull/1985#discussion_r134791577
 
 

 ##
 File path: test/integration/component/test_portforwardingrules.py
 ##
 @@ -0,0 +1,429 @@
+# 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.
+
+# Import Local Modules
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.lib.base import (PublicIPAddress,
+ NetworkOffering,
+ Autoscale,
+ Network,
+ NetworkServiceProvider,
+ Template,
+ VirtualMachine,
+ VPC,
+ VpcOffering,
+ StaticNATRule,
+ FireWallRule,
+ NATRule,
+ Vpn,
+ VpnUser,
+ LoadBalancerRule,
+ Account,
+ ServiceOffering,
+ PhysicalNetwork,
+ User)
+from marvin.lib.common import (get_domain,
+   get_zone,
+   get_template)
+from marvin.lib.utils import validateList, cleanup_resources
+from marvin.codes import PASS
+from nose.plugins.attrib import attr
+
+class TestPortForwardingRules(cloudstackTestCase):
 
 Review comment:
   If this test is rather quick, move it to smoke please.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2246: CLOUDSTACK-10046 checksum validation for any java supported Digests-type

[git]

blueorangutan commented on issue #2246: CLOUDSTACK-10046 checksum validation 
for any java supported Digests-type
URL: https://github.com/apache/cloudstack/pull/2246#issuecomment-324377424
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2246: CLOUDSTACK-10046 checksum validation for any java supported Digests-type

[git]

rhtyd commented on issue #2246: CLOUDSTACK-10046 checksum validation for any 
java supported Digests-type
URL: https://github.com/apache/cloudstack/pull/2246#issuecomment-324377210
 
 
   @blueorangutan package
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2206: [CLOUDSTACK-10020] Changes to make marvin work with projects and VPCs

[git]

blueorangutan commented on issue #2206: [CLOUDSTACK-10020] Changes to make 
marvin work with projects and VPCs
URL: https://github.com/apache/cloudstack/pull/2206#issuecomment-324377145
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2206: [CLOUDSTACK-10020] Changes to make marvin work with projects and VPCs

[git]

rhtyd commented on issue #2206: [CLOUDSTACK-10020] Changes to make marvin work 
with projects and VPCs
URL: https://github.com/apache/cloudstack/pull/2206#issuecomment-324376904
 
 
   @blueorangutan package
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2217: [4.9] Smoketest health checkrun

[git]

blueorangutan commented on issue #2217: [4.9] Smoketest health checkrun
URL: https://github.com/apache/cloudstack/pull/2217#issuecomment-324375924
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2217: [4.9] Smoketest health checkrun

[git]

rhtyd commented on issue #2217: [4.9] Smoketest health checkrun
URL: https://github.com/apache/cloudstack/pull/2217#issuecomment-324375881
 
 
   @blueorangutan package
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2217: [4.9] Smoketest health checkrun

[git]

rhtyd commented on issue #2217: [4.9] Smoketest health checkrun
URL: https://github.com/apache/cloudstack/pull/2217#issuecomment-324375819
 
 
   @borisstoyanov I debugged, rp_filter is set by `CsAddress.py` and it's not 
the root cause. However, in all cases the backup router worked okay as soon as 
the master was rebooted and the connections/routes worked okay after that. 
Given 4.9.2.0 have the same set of limitation as with 4.9 branch, I'll proceed 
with component testing now.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

blueorangutan commented on issue #2239: CLOUDSTACK-9993: Securing Agents 
Communications
URL: https://github.com/apache/cloudstack/pull/2239#issuecomment-324374439
 
 
   @rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted 
as I make progress.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on issue #2239: CLOUDSTACK-9993: Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#issuecomment-324374398
 
 
   @blueorangutan package
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134777167
 
 

 ##
 File path: 
utils/src/main/java/org/apache/cloudstack/utils/security/KeyStoreUtils.java
 ##
 @@ -0,0 +1,70 @@
+//
+// 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 org.apache.cloudstack.utils.security;
+
+import java.io.File;
+import java.io.IOException;
+
+import com.cloud.utils.script.Script;
+import com.google.common.base.Strings;
+
+public class KeyStoreUtils {
+
+public static String defaultTmpKeyStoreFile = "/tmp/tmp.jks";
+public static String defaultKeystoreFile = "/cloud.jks";
+public static String defaultPrivateKeyFile = "/cloud.key";
 
 Review comment:
   -do-
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134777152
 
 

 ##
 File path: 
utils/src/main/java/org/apache/cloudstack/utils/security/KeyStoreUtils.java
 ##
 @@ -0,0 +1,70 @@
+//
+// 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 org.apache.cloudstack.utils.security;
+
+import java.io.File;
+import java.io.IOException;
+
+import com.cloud.utils.script.Script;
+import com.google.common.base.Strings;
+
+public class KeyStoreUtils {
+
+public static String defaultTmpKeyStoreFile = "/tmp/tmp.jks";
+public static String defaultKeystoreFile = "/cloud.jks";
 
 Review comment:
   Good comment, however, the `/` is added to add a separator in case someone 
forgets to add one in the command/exec. I can remove it, it will still work as 
these are only file names.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134776909
 
 

 ##
 File path: utils/pom.xml
 ##
 @@ -72,6 +77,10 @@
   bcprov-jdk15on
 
 
+  org.bouncycastle
 
 Review comment:
   We want dependency explicitly stated, as done for bcprov-jdk15on. The root 
pom has it as well.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134776749
 
 

 ##
 File path: setup/db/server-setup.sql
 ##
 @@ -27,3 +27,6 @@ INSERT INTO `cloud`.`configuration` (category, instance, 
component, name, value,
 
 -- Enable dynamic RBAC by default for fresh deployments
 INSERT INTO `cloud`.`configuration` (category, instance, component, name, 
value) VALUES ('Advanced', 'DEFAULT', 'RoleService', 
'dynamic.apichecker.enabled', 'true');
+
+-- Enable RootCA auth strictness for fresh deployments
+INSERT INTO `cloud`.`configuration` (category, instance, component, name, 
value) VALUES ('Advanced', 'DEFAULT', 'RootCAProvider', 
'ca.plugin.root.auth.strictness', 'true');
 
 Review comment:
   The configkey value by default is false, we want `true`
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134776690
 
 

 ##
 File path: 
services/secondary-storage/controller/src/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
 ##
 @@ -1118,7 +1119,7 @@ public boolean 
finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl
 
 StringBuilder buf = profile.getBootArgsBuilder();
 buf.append(" template=domP type=secstorage");
-buf.append(" 
host=").append(ApiServiceConfiguration.ManagementHostIPAdr.value());
+buf.append(" 
host=").append(StringUtils.shuffleCSVList(ApiServiceConfiguration.ManagementHostIPAdr.value()));
 
 Review comment:
   Yes, we want to have some randomness in case of multiple-mgmt server ips (if 
configured) that's still better than a static/configured value.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134776431
 
 

 ##
 File path: server/src/org/apache/cloudstack/ca/CAManagerImpl.java
 ##
 @@ -0,0 +1,427 @@
+// 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 org.apache.cloudstack.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.ca.IssueCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCAProvidersCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCaCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ProvisionCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.RevokeCertificateCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollManager;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.alert.AlertManager;
+import com.cloud.certificate.CrlVO;
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+
+public class CAManagerImpl extends ManagerBase implements CAManager {
+public static final Logger LOG = Logger.getLogger(CAManagerImpl.class);
+
+@Inject
+private CrlDao crlDao;
+@Inject
+private HostDao hostDao;
+@Inject
+private AgentManager agentManager;
+@Inject
+private BackgroundPollManager backgroundPollManager;
+@Inject
+private AlertManager alertManager;
+
+private static CAProvider configuredCaProvider;
+private static Map caProviderMap = new HashMap<>();
+private static Map alertMap = new ConcurrentHashMap<>();
+private static Map activeCertMap = new 
ConcurrentHashMap<>();
+
+private List caProviders;
+
+private CAProvider getConfiguredCaProvider() {
+if (configuredCaProvider == null && 
caProviderMap.containsKey(CAProviderPlugin.value())) {
+configuredCaProvider = caProviderMap.get(CAProviderPlugin.value());
+}
+if (configuredCaProvider == null) {
+throw new CloudRuntimeException("Failed to find default configured 
CA provider plugin");
+}
+return configuredCaProvider;
+}
+
+private CAProvider getCAProvider(final String provider) {
+if (Strings.isNullOrEmpty(provider)) {
+return getConfiguredCaProvider();
+}
+final String caProviderName = provider.toLowerCase();
+if (!caProviderMap.containsKey(caProviderName)) {
+

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134775466
 
 

 ##
 File path: server/src/org/apache/cloudstack/ca/CAManagerImpl.java
 ##
 @@ -0,0 +1,427 @@
+// 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 org.apache.cloudstack.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.ca.IssueCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCAProvidersCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCaCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ProvisionCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.RevokeCertificateCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollManager;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.alert.AlertManager;
+import com.cloud.certificate.CrlVO;
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+
+public class CAManagerImpl extends ManagerBase implements CAManager {
+public static final Logger LOG = Logger.getLogger(CAManagerImpl.class);
+
+@Inject
+private CrlDao crlDao;
+@Inject
+private HostDao hostDao;
+@Inject
+private AgentManager agentManager;
+@Inject
+private BackgroundPollManager backgroundPollManager;
+@Inject
+private AlertManager alertManager;
+
+private static CAProvider configuredCaProvider;
+private static Map caProviderMap = new HashMap<>();
+private static Map alertMap = new ConcurrentHashMap<>();
+private static Map activeCertMap = new 
ConcurrentHashMap<>();
+
+private List caProviders;
+
+private CAProvider getConfiguredCaProvider() {
+if (configuredCaProvider == null && 
caProviderMap.containsKey(CAProviderPlugin.value())) {
+configuredCaProvider = caProviderMap.get(CAProviderPlugin.value());
+}
+if (configuredCaProvider == null) {
+throw new CloudRuntimeException("Failed to find default configured 
CA provider plugin");
+}
+return configuredCaProvider;
+}
+
+private CAProvider getCAProvider(final String provider) {
+if (Strings.isNullOrEmpty(provider)) {
+return getConfiguredCaProvider();
+}
+final String caProviderName = provider.toLowerCase();
+if (!caProviderMap.containsKey(caProviderName)) {
+

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134775294
 
 

 ##
 File path: server/src/org/apache/cloudstack/ca/CAManagerImpl.java
 ##
 @@ -0,0 +1,427 @@
+// 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 org.apache.cloudstack.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.ca.IssueCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCAProvidersCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCaCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ProvisionCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.RevokeCertificateCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollManager;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.alert.AlertManager;
+import com.cloud.certificate.CrlVO;
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+
+public class CAManagerImpl extends ManagerBase implements CAManager {
+public static final Logger LOG = Logger.getLogger(CAManagerImpl.class);
+
+@Inject
+private CrlDao crlDao;
+@Inject
+private HostDao hostDao;
+@Inject
+private AgentManager agentManager;
+@Inject
+private BackgroundPollManager backgroundPollManager;
+@Inject
+private AlertManager alertManager;
+
+private static CAProvider configuredCaProvider;
+private static Map caProviderMap = new HashMap<>();
+private static Map alertMap = new ConcurrentHashMap<>();
+private static Map activeCertMap = new 
ConcurrentHashMap<>();
+
+private List caProviders;
+
+private CAProvider getConfiguredCaProvider() {
+if (configuredCaProvider == null && 
caProviderMap.containsKey(CAProviderPlugin.value())) {
+configuredCaProvider = caProviderMap.get(CAProviderPlugin.value());
+}
+if (configuredCaProvider == null) {
+throw new CloudRuntimeException("Failed to find default configured 
CA provider plugin");
+}
+return configuredCaProvider;
+}
+
+private CAProvider getCAProvider(final String provider) {
+if (Strings.isNullOrEmpty(provider)) {
+return getConfiguredCaProvider();
+}
+final String caProviderName = provider.toLowerCase();
+if (!caProviderMap.containsKey(caProviderName)) {
+

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134774970
 
 

 ##
 File path: server/src/org/apache/cloudstack/ca/CAManagerImpl.java
 ##
 @@ -0,0 +1,427 @@
+// 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 org.apache.cloudstack.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.ca.IssueCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCAProvidersCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCaCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ProvisionCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.RevokeCertificateCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollManager;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.alert.AlertManager;
+import com.cloud.certificate.CrlVO;
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+
+public class CAManagerImpl extends ManagerBase implements CAManager {
+public static final Logger LOG = Logger.getLogger(CAManagerImpl.class);
+
+@Inject
+private CrlDao crlDao;
+@Inject
+private HostDao hostDao;
+@Inject
+private AgentManager agentManager;
+@Inject
+private BackgroundPollManager backgroundPollManager;
+@Inject
+private AlertManager alertManager;
+
+private static CAProvider configuredCaProvider;
+private static Map caProviderMap = new HashMap<>();
+private static Map alertMap = new ConcurrentHashMap<>();
+private static Map activeCertMap = new 
ConcurrentHashMap<>();
+
+private List caProviders;
+
+private CAProvider getConfiguredCaProvider() {
+if (configuredCaProvider == null && 
caProviderMap.containsKey(CAProviderPlugin.value())) {
+configuredCaProvider = caProviderMap.get(CAProviderPlugin.value());
+}
+if (configuredCaProvider == null) {
+throw new CloudRuntimeException("Failed to find default configured 
CA provider plugin");
+}
+return configuredCaProvider;
+}
+
+private CAProvider getCAProvider(final String provider) {
+if (Strings.isNullOrEmpty(provider)) {
+return getConfiguredCaProvider();
+}
+final String caProviderName = provider.toLowerCase();
+if (!caProviderMap.containsKey(caProviderName)) {
+

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134774674
 
 

 ##
 File path: 
server/src/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java
 ##
 @@ -125,6 +137,73 @@ public boolean processTimeout(long agentId, long seq) {
 return false;
 }
 
+private void setupAgentSecurity(final Connection sshConnection, final 
String agentIp, final String agentHostname) {
 
 Review comment:
   Hmm, I would avoid, this is do-able however.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134774587
 
 

 ##
 File path: 
plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java
 ##
 @@ -0,0 +1,572 @@
+// 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 org.apache.cloudstack.ca.provider;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.KeyManagementException;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.SecureRandom;
+import java.security.Security;
+import java.security.SignatureException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.log4j.Logger;
+import org.bouncycastle.jce.PKCS10CertificationRequest;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
+
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.nio.Link;
+import com.google.common.base.Strings;
+
+public final class RootCAProvider extends AdapterBase implements CAProvider, 
Configurable {
+private static final Logger LOG = Logger.getLogger(RootCAProvider.class);
+
+public static final Integer caValidityYears = 30;
+public static final String caAlias = "root";
+public static final String managementAlias = "management";
+
+private static KeyPair caKeyPair = null;
+private static X509Certificate caCertificate = null;
+
+@Inject
+private ConfigurationDao configDao;
+@Inject
+private CrlDao crlDao;
+
+
+/// Root CA Settings ///
+
+
+private static ConfigKey rootCAPrivateKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.private.key",
+null,
+"The ROOT CA private key.", true);
+
+private static ConfigKey rootCAPublicKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.public.key",
+null,
+"The ROOT CA public key.", true);
+
+private static ConfigKey rootCACertificate = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.ca.certificate",
+null,
+"The ROOT CA certificate.", true);
+
+private static 

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134774348
 
 

 ##
 File path: 
plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java
 ##
 @@ -0,0 +1,572 @@
+// 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 org.apache.cloudstack.ca.provider;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.KeyManagementException;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.SecureRandom;
+import java.security.Security;
+import java.security.SignatureException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.log4j.Logger;
+import org.bouncycastle.jce.PKCS10CertificationRequest;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
+
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.nio.Link;
+import com.google.common.base.Strings;
+
+public final class RootCAProvider extends AdapterBase implements CAProvider, 
Configurable {
+private static final Logger LOG = Logger.getLogger(RootCAProvider.class);
+
+public static final Integer caValidityYears = 30;
+public static final String caAlias = "root";
+public static final String managementAlias = "management";
+
+private static KeyPair caKeyPair = null;
+private static X509Certificate caCertificate = null;
+
+@Inject
+private ConfigurationDao configDao;
+@Inject
+private CrlDao crlDao;
+
+
+/// Root CA Settings ///
+
+
+private static ConfigKey rootCAPrivateKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.private.key",
+null,
+"The ROOT CA private key.", true);
+
+private static ConfigKey rootCAPublicKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.public.key",
+null,
+"The ROOT CA public key.", true);
+
+private static ConfigKey rootCACertificate = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.ca.certificate",
+null,
+"The ROOT CA certificate.", true);
+
+private static 

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134774309
 
 

 ##
 File path: 
plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java
 ##
 @@ -0,0 +1,572 @@
+// 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 org.apache.cloudstack.ca.provider;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.KeyManagementException;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.SecureRandom;
+import java.security.Security;
+import java.security.SignatureException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.log4j.Logger;
+import org.bouncycastle.jce.PKCS10CertificationRequest;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
+
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.nio.Link;
+import com.google.common.base.Strings;
+
+public final class RootCAProvider extends AdapterBase implements CAProvider, 
Configurable {
+private static final Logger LOG = Logger.getLogger(RootCAProvider.class);
+
+public static final Integer caValidityYears = 30;
+public static final String caAlias = "root";
+public static final String managementAlias = "management";
+
+private static KeyPair caKeyPair = null;
+private static X509Certificate caCertificate = null;
+
+@Inject
+private ConfigurationDao configDao;
+@Inject
+private CrlDao crlDao;
+
+
+/// Root CA Settings ///
+
+
+private static ConfigKey rootCAPrivateKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.private.key",
+null,
+"The ROOT CA private key.", true);
+
+private static ConfigKey rootCAPublicKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.public.key",
+null,
+"The ROOT CA public key.", true);
+
+private static ConfigKey rootCACertificate = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.ca.certificate",
+null,
+"The ROOT CA certificate.", true);
+
+private static 

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134773904
 
 

 ##
 File path: 
plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java
 ##
 @@ -0,0 +1,572 @@
+// 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 org.apache.cloudstack.ca.provider;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.KeyManagementException;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.SecureRandom;
+import java.security.Security;
+import java.security.SignatureException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.log4j.Logger;
+import org.bouncycastle.jce.PKCS10CertificationRequest;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
+
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.nio.Link;
+import com.google.common.base.Strings;
+
+public final class RootCAProvider extends AdapterBase implements CAProvider, 
Configurable {
+private static final Logger LOG = Logger.getLogger(RootCAProvider.class);
+
+public static final Integer caValidityYears = 30;
+public static final String caAlias = "root";
+public static final String managementAlias = "management";
+
+private static KeyPair caKeyPair = null;
+private static X509Certificate caCertificate = null;
+
+@Inject
+private ConfigurationDao configDao;
+@Inject
+private CrlDao crlDao;
+
+
+/// Root CA Settings ///
+
+
+private static ConfigKey rootCAPrivateKey = new 
ConfigKey<>("Hidden", String.class,
 
 Review comment:
   Yes, all `Hidden` keys are encrypted.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134773821
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/Certificate.java
 ##
 @@ -0,0 +1,46 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.util.List;
+
+public class Certificate {
+private X509Certificate clientCertificate;
+private PrivateKey privateKey;
+private List caCertificates;
+
+public Certificate(final X509Certificate clientCertificate, final 
PrivateKey privateKey, final List caCertificates) {
+this.clientCertificate = clientCertificate;
+this.privateKey = privateKey;
+this.caCertificates = caCertificates;
+}
+
+public X509Certificate getClientCertificate() {
+return clientCertificate;
+}
+
+public PrivateKey getPrivateKey() {
 
 Review comment:
   Expected, we want to return private key is it's there.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134773695
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAService.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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAService {
+/**
+ * Returns a SSLEngine to be used for handling client connections
+ * @param context
+ * @param remoteAddress
+ * @return
 
 Review comment:
   CloudStack classes are not consume-able as libraries -- the docs are only 
for ACS developers. I also have a dilemma whether to even have these docs as 
it's quite clear what the methods accept, returns in most cases. With this, I 
have added the docs to provide some interface definition but not wrote it 
pendantically to be consumed as a library.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134773185
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
+ */
+boolean canProvisionCertificates();
+
+/**
+ * Returns root CA certificate
+ * @return returns concatenated root CA certificate string
+ */
+List getCaCertificate();
+
+/**
+ * Issues certificate with provided options
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final List domainNames, final 
List ipAddresses, final int validityDays);
+
+/**
+ * Issues certificate using given CSR and other options
+ * @param csr
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final String csr, final List 
domainNames, final List ipAddresses, final int validityDays);
+
+/**
+ * Revokes certificate using certificate serial and CN
+ * @param certSerial
+ * @param certCn
+ * @return returns true on success
+ */
+boolean revokeCertificate(final BigInteger certSerial, final String 
certCn);
+
+/**
+ * This method can add/inject custom TrustManagers for client connection 
validations.
+ * @param sslContext The SSL context used while accepting a client 
connection
+ * @param remoteAddress
+ * @param certMap
+ * @return
+ * @throws GeneralSecurityException
+ * @throws IOException
+ */
+SSLEngine createSSLEngine(final SSLContext sslContext, final String 
remoteAddress, final Map certMap) throws 
GeneralSecurityException, IOException;
+
+/**
+ * Returns the unique name of the provider
+ * @return
+ */
+String getProviderName();
+
+/**
+ * Returns description about the CA provider plugin
+ * @return
 
 Review comment:
   Self explanatory
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134773144
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
+ */
+boolean canProvisionCertificates();
+
+/**
+ * Returns root CA certificate
+ * @return returns concatenated root CA certificate string
+ */
+List getCaCertificate();
+
+/**
+ * Issues certificate with provided options
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final List domainNames, final 
List ipAddresses, final int validityDays);
+
+/**
+ * Issues certificate using given CSR and other options
+ * @param csr
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final String csr, final List 
domainNames, final List ipAddresses, final int validityDays);
+
+/**
+ * Revokes certificate using certificate serial and CN
+ * @param certSerial
+ * @param certCn
+ * @return returns true on success
+ */
+boolean revokeCertificate(final BigInteger certSerial, final String 
certCn);
+
+/**
+ * This method can add/inject custom TrustManagers for client connection 
validations.
+ * @param sslContext The SSL context used while accepting a client 
connection
+ * @param remoteAddress
+ * @param certMap
+ * @return
+ * @throws GeneralSecurityException
+ * @throws IOException
+ */
+SSLEngine createSSLEngine(final SSLContext sslContext, final String 
remoteAddress, final Map certMap) throws 
GeneralSecurityException, IOException;
+
+/**
+ * Returns the unique name of the provider
+ * @return
 
 Review comment:
   Self explanatory
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134773126
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
+ */
+boolean canProvisionCertificates();
+
+/**
+ * Returns root CA certificate
+ * @return returns concatenated root CA certificate string
+ */
+List getCaCertificate();
+
+/**
+ * Issues certificate with provided options
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final List domainNames, final 
List ipAddresses, final int validityDays);
+
+/**
+ * Issues certificate using given CSR and other options
+ * @param csr
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final String csr, final List 
domainNames, final List ipAddresses, final int validityDays);
+
+/**
+ * Revokes certificate using certificate serial and CN
+ * @param certSerial
+ * @param certCn
+ * @return returns true on success
+ */
+boolean revokeCertificate(final BigInteger certSerial, final String 
certCn);
+
+/**
+ * This method can add/inject custom TrustManagers for client connection 
validations.
+ * @param sslContext The SSL context used while accepting a client 
connection
+ * @param remoteAddress
+ * @param certMap
+ * @return
 
 Review comment:
   Self explanatory
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134773048
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
+ */
+boolean canProvisionCertificates();
+
+/**
+ * Returns root CA certificate
+ * @return returns concatenated root CA certificate string
+ */
+List getCaCertificate();
+
+/**
+ * Issues certificate with provided options
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final List domainNames, final 
List ipAddresses, final int validityDays);
+
+/**
+ * Issues certificate using given CSR and other options
+ * @param csr
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final String csr, final List 
domainNames, final List ipAddresses, final int validityDays);
 
 Review comment:
   The CSR may or may-not, for systemvm/kvm agents the CSR generated may not 
know additional names, ips (such as storage, public, link-local, private) etc 
that CloudStack knows so during certificate generation we use primary domain 
name from CSR only, and additional details is provided by CloudStack.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134772656
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
 
 Review comment:
   That's what the description says, it was repetitive so I avoided that.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134772485
 
 

 ##
 File path: engine/schema/src/com/cloud/certificate/dao/CrlDaoImpl.java
 ##
 @@ -0,0 +1,57 @@
+// 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.certificate.dao;
+
+import java.math.BigInteger;
+
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.certificate.CrlVO;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+
+@DB
+public class CrlDaoImpl extends GenericDaoBase implements CrlDao {
+
+private final SearchBuilder CrlBySerialSearch;
+
+public CrlDaoImpl() {
+super();
+
+CrlBySerialSearch = createSearchBuilder();
+CrlBySerialSearch.and("certSerial", 
CrlBySerialSearch.entity().getCertSerial(), SearchCriteria.Op.EQ);
+CrlBySerialSearch.done();
+}
+
+@Override
+public CrlVO findBySerial(final BigInteger certSerial) {
 
 Review comment:
   No, I don't see a risk here. The method needs to be public to allow 
subsystems to find by serial. How do you think it can be a risk, usage-wise?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134772018
 
 

 ##
 File path: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
 ##
 @@ -1073,6 +1079,24 @@ public void orchestrateStart(final String vmUuid, final 
Map

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134771492
 
 

 ##
 File path: 
engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java
 ##
 @@ -495,28 +495,29 @@ public SocketChannel connectToPeer(final String 
peerName, final SocketChannel pr
 }
 final String ip = ms.getServiceIP();
 InetAddress addr;
+int port = Port.value();
 try {
 addr = InetAddress.getByName(ip);
 } catch (final UnknownHostException e) {
 throw new CloudRuntimeException("Unable to resolve " + ip);
 }
 SocketChannel ch1 = null;
 try {
-ch1 = SocketChannel.open(new InetSocketAddress(addr, 
Port.value()));
+ch1 = SocketChannel.open(new InetSocketAddress(addr, 
port));
 
 Review comment:
   we can however, I avoided much changes, as we were doing something with 
handshake etc.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134771334
 
 

 ##
 File path: core/src/com/cloud/agent/api/routing/NetworkElementCommand.java
 ##
 @@ -46,6 +47,18 @@ protected NetworkElementCommand() {
 super();
 }
 
+public void setAccessDetail(final Map details) {
+if (details == null) {
+return;
+}
+for (final Map.Entry detail : details.entrySet()) {
 
 Review comment:
   @DaanHoogland sorry don't understand the comment, the idea was to 
provide/set a detail by passing a map here, which may override a detail (if so 
asked by the developer)
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134771110
 
 

 ##
 File path: api/src/org/apache/cloudstack/api/response/CertificateResponse.java
 ##
 @@ -0,0 +1,58 @@
+// 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 org.apache.cloudstack.api.response;
+
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseResponse;
+
+import com.cloud.serializer.Param;
+import com.google.gson.annotations.SerializedName;
+
+public class CertificateResponse extends BaseResponse {
+@SerializedName(ApiConstants.CERTIFICATE)
+@Param(description = "The client certificate")
+private String certificate = "";
+
+@SerializedName(ApiConstants.PRIVATE_KEY)
+@Param(description = "Private key for the certificate")
+private String privateKey;
+
+@SerializedName(ApiConstants.CA_CERTIFICATES)
+@Param(description = "The CA certificate(s)")
+private String caCertificate;
+
+public CertificateResponse() {
+setObjectName("certificates");
+}
+
+public CertificateResponse(final String objectName) {
+setObjectName(objectName);
+}
+
+public void setCertificate(final String certificate) {
+this.certificate = certificate;
+}
+
+public void setPrivateKey(final String privateKey) {
 
 Review comment:
   The response in this case is necessary, as the issue Cert is supposed to 
return private key when CSR is provided. The API is restricted to root-admin 
usage now, this is necessary. I'll see what I can do.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134770785
 
 

 ##
 File path: 
api/src/org/apache/cloudstack/api/command/admin/ca/IssueCertificateCmd.java
 ##
 @@ -0,0 +1,162 @@
+// 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 org.apache.cloudstack.api.command.admin.ca;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.CertificateResponse;
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+
+import com.cloud.event.EventTypes;
+import com.google.common.base.Strings;
+
+@APICommand(name = IssueCertificateCmd.APINAME,
+description = "Issues a client certificate using configured or 
provided CA plugin",
+responseObject = CertificateResponse.class,
+requestHasSensitiveInfo = false,
+responseHasSensitiveInfo = false,
+since = "4.11.0",
+authorized = {RoleType.Admin})
+public class IssueCertificateCmd extends BaseAsyncCmd {
+private static final Logger LOG = 
Logger.getLogger(IssueCertificateCmd.class);
+
+public static final String APINAME = "issueCertificate";
+
+@Inject
+private CAManager caManager;
+
+/
+ API parameters /
+/
+
+@Parameter(name = ApiConstants.CSR, type = BaseCmd.CommandType.STRING, 
description = "The certificate signing request (in pem format), if CSR is not 
provided then configured/provided options are considered", length = 65535)
+private String csr;
+
+@Parameter(name = ApiConstants.DOMAIN, type = BaseCmd.CommandType.STRING, 
description = "Comma separated list of domains, the certificate should be 
issued for. When csr is not provided, the first domain is used as a subject/CN")
+private String domains;
+
+@Parameter(name = ApiConstants.IP_ADDRESS, type = 
BaseCmd.CommandType.STRING, description = "Comma separated list of IP 
addresses, the certificate should be issued for")
+private String addresses;
+
+@Parameter(name = ApiConstants.DURATION, type = CommandType.INTEGER, 
description = "Certificate validity duration in number of days, when not 
provided the default configured value will be used")
+private Integer validityDuration;
+
+@Parameter(name = ApiConstants.PROVIDER, type = 
BaseCmd.CommandType.STRING, description = "Name of the CA service provider, 
otherwise the default configured provider plugin will be used")
+private String provider;
+
+/
+/// Accessors ///
+/
+
+public String getCsr() {
+return csr;
+}
+
+private List processList(final String string) {
+final List list = new ArrayList<>();
+if (!Strings.isNullOrEmpty(string)) {
+for (final String address: string.split(",")) {
+list.add(address.trim());
+}
+}
+return list;
+}
+
+public List getAddresses() {
+return processList(addresses);
+}
+
+public List getDomains() {
+return processList(domains);
+}
+
+public Integer getValidityDuration() {
+return validityDuration;
+}
+
+public String getProvider() {
+return provider;
+}
+
+/
+/// API 

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134770614
 
 

 ##
 File path: 
api/src/org/apache/cloudstack/api/command/admin/ca/IssueCertificateCmd.java
 ##
 @@ -0,0 +1,162 @@
+// 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 org.apache.cloudstack.api.command.admin.ca;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.CertificateResponse;
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+
+import com.cloud.event.EventTypes;
+import com.google.common.base.Strings;
+
+@APICommand(name = IssueCertificateCmd.APINAME,
+description = "Issues a client certificate using configured or 
provided CA plugin",
+responseObject = CertificateResponse.class,
+requestHasSensitiveInfo = false,
+responseHasSensitiveInfo = false,
+since = "4.11.0",
+authorized = {RoleType.Admin})
+public class IssueCertificateCmd extends BaseAsyncCmd {
+private static final Logger LOG = 
Logger.getLogger(IssueCertificateCmd.class);
+
+public static final String APINAME = "issueCertificate";
+
+@Inject
+private CAManager caManager;
+
+/
+ API parameters /
+/
+
+@Parameter(name = ApiConstants.CSR, type = BaseCmd.CommandType.STRING, 
description = "The certificate signing request (in pem format), if CSR is not 
provided then configured/provided options are considered", length = 65535)
+private String csr;
+
+@Parameter(name = ApiConstants.DOMAIN, type = BaseCmd.CommandType.STRING, 
description = "Comma separated list of domains, the certificate should be 
issued for. When csr is not provided, the first domain is used as a subject/CN")
+private String domains;
+
+@Parameter(name = ApiConstants.IP_ADDRESS, type = 
BaseCmd.CommandType.STRING, description = "Comma separated list of IP 
addresses, the certificate should be issued for")
+private String addresses;
+
+@Parameter(name = ApiConstants.DURATION, type = CommandType.INTEGER, 
description = "Certificate validity duration in number of days, when not 
provided the default configured value will be used")
+private Integer validityDuration;
+
+@Parameter(name = ApiConstants.PROVIDER, type = 
BaseCmd.CommandType.STRING, description = "Name of the CA service provider, 
otherwise the default configured provider plugin will be used")
+private String provider;
+
+/
+/// Accessors ///
+/
+
+public String getCsr() {
+return csr;
+}
+
+private List processList(final String string) {
+final List list = new ArrayList<>();
+if (!Strings.isNullOrEmpty(string)) {
+for (final String address: string.split(",")) {
+list.add(address.trim());
+}
+}
+return list;
+}
+
+public List getAddresses() {
+return processList(addresses);
+}
+
+public List getDomains() {
+return processList(domains);
+}
+
+public Integer getValidityDuration() {
+return validityDuration;
+}
+
+public String getProvider() {
+return provider;
+}
+
+/
+/// API 

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134770495
 
 

 ##
 File path: 
api/src/org/apache/cloudstack/api/command/admin/ca/IssueCertificateCmd.java
 ##
 @@ -0,0 +1,162 @@
+// 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 org.apache.cloudstack.api.command.admin.ca;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.CertificateResponse;
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+
+import com.cloud.event.EventTypes;
+import com.google.common.base.Strings;
+
+@APICommand(name = IssueCertificateCmd.APINAME,
+description = "Issues a client certificate using configured or 
provided CA plugin",
+responseObject = CertificateResponse.class,
+requestHasSensitiveInfo = false,
+responseHasSensitiveInfo = false,
+since = "4.11.0",
+authorized = {RoleType.Admin})
+public class IssueCertificateCmd extends BaseAsyncCmd {
+private static final Logger LOG = 
Logger.getLogger(IssueCertificateCmd.class);
+
+public static final String APINAME = "issueCertificate";
+
+@Inject
+private CAManager caManager;
 
 Review comment:
   -do-
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134770453
 
 

 ##
 File path: 
api/src/org/apache/cloudstack/api/command/admin/ca/IssueCertificateCmd.java
 ##
 @@ -0,0 +1,162 @@
+// 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 org.apache.cloudstack.api.command.admin.ca;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.CertificateResponse;
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+
+import com.cloud.event.EventTypes;
+import com.google.common.base.Strings;
+
+@APICommand(name = IssueCertificateCmd.APINAME,
+description = "Issues a client certificate using configured or 
provided CA plugin",
+responseObject = CertificateResponse.class,
+requestHasSensitiveInfo = false,
+responseHasSensitiveInfo = false,
+since = "4.11.0",
+authorized = {RoleType.Admin})
+public class IssueCertificateCmd extends BaseAsyncCmd {
+private static final Logger LOG = 
Logger.getLogger(IssueCertificateCmd.class);
+
+public static final String APINAME = "issueCertificate";
+
+@Inject
+private CAManager caManager;
 
 Review comment:
   Putting inject on `BaseCmd` class would slow the bean creation and 
injections. I avoid such a pattern, keep classes light weight. This is the 
pattern we've been using for several features now, including dynamic roles, 
oobm, host-ha etc.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134770166
 
 

 ##
 File path: api/src/org/apache/cloudstack/alert/AlertService.java
 ##
 @@ -67,6 +67,7 @@ private AlertType(short type, String name, boolean 
isDefault) {
 public static final AlertType ALERT_TYPE_SYNC = new 
AlertType((short)27, "ALERT.TYPE.SYNC", true);
 public static final AlertType ALERT_TYPE_UPLOAD_FAILED = new 
AlertType((short)28, "ALERT.UPLOAD.FAILED", true);
 public static final AlertType ALERT_TYPE_OOBM_AUTH_ERROR = new 
AlertType((short)29, "ALERT.OOBM.AUTHERROR", true);
+public static final AlertType ALERT_TYPE_CA_CERT = new 
AlertType((short)31, "ALERT.CA.CERT", true);
 
 Review comment:
   Yep, 30 is reserved for 'host-ha'. I wanted to avoid changing the numbers.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134770194
 
 

 ##
 File path: api/src/org/apache/cloudstack/alert/AlertService.java
 ##
 @@ -67,6 +67,7 @@ private AlertType(short type, String name, boolean 
isDefault) {
 public static final AlertType ALERT_TYPE_SYNC = new 
AlertType((short)27, "ALERT.TYPE.SYNC", true);
 public static final AlertType ALERT_TYPE_UPLOAD_FAILED = new 
AlertType((short)28, "ALERT.UPLOAD.FAILED", true);
 public static final AlertType ALERT_TYPE_OOBM_AUTH_ERROR = new 
AlertType((short)29, "ALERT.OOBM.AUTHERROR", true);
+public static final AlertType ALERT_TYPE_CA_CERT = new 
AlertType((short)31, "ALERT.CA.CERT", true);
 
 Review comment:
   Yes.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134770033
 
 

 ##
 File path: agent/src/com/cloud/agent/dao/impl/PropertiesStorage.java
 ##
 @@ -51,6 +51,9 @@ public synchronized String get(String key) {
 
 @Override
 public synchronized void persist(String key, String value) {
+if (!loadFromFile(_file)) {
+s_logger.warn("Failed to load changes and then write to them");
 
 Review comment:
   There is nobody to read the values, I refactored existing methods to allow 
persisting passwords/passphrases for keystore. I can refactor this to throw an 
exception, however that changes the previous implementation as persisting 
changes is not mandatory, the agent reads stuff from cmdline (in systemvms) 
etc. and sometimes receives information from readycommand.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134769420
 
 

 ##
 File path: agent/test/com/cloud/agent/AgentShellTest.java
 ##
 @@ -44,4 +48,15 @@ public void loadProperties() throws ConfigurationException {
 Assert.assertNotNull(shell.getProperties());
 Assert.assertFalse(shell.getProperties().entrySet().isEmpty());
 }
+
+@Test
+public void testGetHost() {
+AgentShell shell = new AgentShell();
+List hosts = Arrays.asList("10.1.1.1", "20.2.2.2", "30.3.3.3", 
"2001:db8::1");
+shell.setHost(StringUtils.listToCsvTags(hosts));
+for (String host : hosts) {
 
 Review comment:
   That's the case for now.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134769242
 
 

 ##
 File path: agent/src/com/cloud/agent/AgentShell.java
 ##
 @@ -107,7 +108,16 @@ public String getPod() {
 
 @Override
 public String getHost() {
-return _host;
+String[] hosts = _host.split(",");
+if (_hostCounter >= hosts.length) {
+_hostCounter = 0;
+}
+s_logger.info("Connecting to host: " + hosts[_hostCounter % 
hosts.length]);
+return hosts[_hostCounter++ % hosts.length];
+}
+
+public void setHost(final String host) {
 
 Review comment:
   For historic reasons, I simply created getters/setters for `_host`, this 
method is only used for writing a unit test.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134769071
 
 

 ##
 File path: agent/src/com/cloud/agent/AgentShell.java
 ##
 @@ -107,7 +108,16 @@ public String getPod() {
 
 @Override
 public String getHost() {
-return _host;
+String[] hosts = _host.split(",");
+if (_hostCounter >= hosts.length) {
+_hostCounter = 0;
+}
+s_logger.info("Connecting to host: " + hosts[_hostCounter % 
hosts.length]);
 
 Review comment:
   I'll refactor this, however this prints the next 'host' the `Agent` class 
may attempt to connect to.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134768766
 
 

 ##
 File path: agent/src/com/cloud/agent/Agent.java
 ##
 @@ -464,7 +493,7 @@ protected void processRequest(final Request request, final 
Link link) {
 
 for (int i = 0; i < cmds.length; i++) {
 final Command cmd = cmds[i];
-Answer answer;
+Answer answer = null;
 
 Review comment:
   this can be removed, an uninitialized variable is `null`, it just states it 
explicitly.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

rhtyd commented on a change in pull request #2239: CLOUDSTACK-9993: Securing 
Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134768488
 
 

 ##
 File path: agent/src/com/cloud/agent/Agent.java
 ##
 @@ -166,7 +178,8 @@ public Agent(final IAgentShell shell, final int 
localAgentId, final ServerResour
 throw new ConfigurationException("Unable to configure " + 
_resource.getName());
 }
 
-_connection = new NioClient("Agent", _shell.getHost(), 
_shell.getPort(), _shell.getWorkers(), this);
+final String host = _shell.getHost();
 
 Review comment:
   @DaanHoogland yes, the `getHost` is supposed to internally loop/return the 
next address.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on issue #2245: increased jetty timeout

[git]

rhtyd commented on issue #2245: increased jetty timeout
URL: https://github.com/apache/cloudstack/pull/2245#issuecomment-324352471
 
 
   Actually, this is fixing the symptom and not the problem itself. I've 
addressed this part of the PR #2239, jetty takes time in scanning annotations 
that are not necessary and may be limited like this:
   ```
   +
.*/cloud.*jar$|.*/classes/.*
   ```
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2216: CLOUDSTACK-10027 Repeating the same list for Internal LB in VPC

[git]

rhtyd commented on a change in pull request #2216: CLOUDSTACK-10027 Repeating 
the same list for Internal LB in VPC
URL: https://github.com/apache/cloudstack/pull/2216#discussion_r134766814
 
 

 ##
 File path: ui/scripts/vpc.js
 ##
 @@ -745,12 +745,24 @@
 }
 },
 dataProvider: function(args) {
+var array1 = [];
+if (args.filterBy != null) {
+if (args.filterBy.search != null && 
args.filterBy.search.by != null && args.filterBy.search.value != null) {
+if (args.filterBy.search.by === "name") {
+if (args.filterBy.search.value.length > 0)
+array1.push("=" + 
args.filterBy.search.value);
+}
+}
+}
+var data = {
+page: args.page,
+pageSize: pageSize,
+networkid: args.context.networks[0].id,
+listAll: true
+};
 $.ajax({
-url: createURL('listLoadBalancers'),
-data: {
-networkid: args.context.networks[0].id,
-listAll: true
-},
+url: createURL('listLoadBalancers' + 
array1.join("")),
+data: data,
 success: function(json) {
 
 Review comment:
   @vedulasantosh Alright, I wanted you to explore better implementation. 
Here's one way, I would prefer (see how it remove the need of multiple-nested 
ifs):
   ```
  var data = {
   networkid: args.context.networks[0].id,
   page: args.page,
   pageSize: pageSize,
   listAll: true
  };
  var keyword = (((args || {}).filterBy || {}).search 
|| {}).value;
  if (keyword) {
  data.keyword = keyword;
  }
  # rest of the code
   ```
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] rhtyd commented on a change in pull request #2216: CLOUDSTACK-10027 Repeating the same list for Internal LB in VPC

[git]

rhtyd commented on a change in pull request #2216: CLOUDSTACK-10027 Repeating 
the same list for Internal LB in VPC
URL: https://github.com/apache/cloudstack/pull/2216#discussion_r134763805
 
 

 ##
 File path: ui/scripts/vpc.js
 ##
 @@ -745,12 +745,24 @@
 }
 },
 dataProvider: function(args) {
+var array1 = [];
+if (args.filterBy != null) {
+if (args.filterBy.search != null && 
args.filterBy.search.by != null && args.filterBy.search.value != null) {
+if (args.filterBy.search.by === "name") {
+if (args.filterBy.search.value.length > 0)
+array1.push("=" + 
args.filterBy.search.value);
+}
+}
+}
+var data = {
+page: args.page,
+pageSize: pageSize,
+networkid: args.context.networks[0].id,
+listAll: true
+};
 $.ajax({
-url: createURL('listLoadBalancers'),
-data: {
-networkid: args.context.networks[0].id,
-listAll: true
-},
+url: createURL('listLoadBalancers' + 
array1.join("")),
+data: data,
 success: function(json) {
 
 Review comment:
   @vedulasantosh much better, however, that's not why I meant. It still does 
not remove the array/join usage. On line 764, remove appending of 
`array1.join()` usage, instead move the `data` object after line 747, and at 
line 753, push the key/value to the `data` object. 
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland opened a new pull request #2246: CLOUDSTACK-10046 checksum validation for any java supported Digests-type

[git]

DaanHoogland opened a new pull request #2246: CLOUDSTACK-10046 checksum 
validation for any java supported Digests-type
URL: https://github.com/apache/cloudstack/pull/2246
 
 
   This fixes a very old regression where the checksum wouldn't be checked on 
downloaded templates.
   
   It checks again now but also allows to specify an algorithm like 
"{SHA-1}98765". No algorithm assumes md5 ("MD5") for now
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #1960: [4.11/Future] CLOUDSTACK-9782: Host HA and KVM HA provider

[git]

blueorangutan commented on issue #1960: [4.11/Future] CLOUDSTACK-9782: Host HA 
and KVM HA provider
URL: https://github.com/apache/cloudstack/pull/1960#issuecomment-324329059
 
 
   Trillian test result (tid-1412)
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40748 seconds
   Marvin logs: 
https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr1960-t1412-kvm-centos7.zip
   Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm_agent.py
   Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
   Intermitten failure detected: /marvin/tests/smoke/test_iso.py
   Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
   Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Test completed. 57 look OK, 4 have error(s)
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_vpc_remote_access_vpn | `Failure` | 55.93 | test_vpc_vpn.py
   test_04_rvpc_privategw_static_routes | `Failure` | 360.65 | 
test_privategw_acl.py
   test_05_iso_permissions | `Failure` | 0.06 | test_iso.py
   test_02_edit_iso | `Failure` | 0.05 | test_iso.py
   test_ha_kvm_host_recovering | `Error` | 36.00 | test_hostha_kvm_agent.py
   test_ha_kvm_host_fencing | `Error` | 651.26 | test_hostha_kvm_agent.py
   test_ha_kvm_host_fencing | `Error` | 656.45 | test_hostha_kvm_agent.py
   test_change_service_offering_for_vm_with_snapshots | Skipped | 0.00 | 
test_vm_snapshots.py
   test_09_copy_delete_template | Skipped | 0.02 | test_templates.py
   test_06_copy_template | Skipped | 0.00 | test_templates.py
   test_static_role_account_acls | Skipped | 0.02 | test_staticroles.py
   test_11_ss_nfs_version_on_ssvm | Skipped | 0.02 | test_ssvm.py
   test_01_scale_vm | Skipped | 0.00 | test_scale_vm.py
   test_01_primary_storage_iscsi | Skipped | 0.04 | test_primary_storage.py
   test_vm_nic_adapter_vmxnet3 | Skipped | 0.00 | test_nic_adapter_type.py
   test_nested_virtualization_vmware | Skipped | 0.00 | 
test_nested_virtualization.py
   test_06_copy_iso | Skipped | 0.00 | test_iso.py
   test_list_ha_for_host_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_list_ha_for_host | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_enable_feature_without_setting_provider | Skipped | 0.03 | 
test_hostha_simulator.py
   test_hostha_enable_feature_valid | Skipped | 0.02 | test_hostha_simulator.py
   test_hostha_disable_feature_valid | Skipped | 0.03 | test_hostha_simulator.py
   test_hostha_configure_invalid_provider | Skipped | 0.02 | 
test_hostha_simulator.py
   test_hostha_configure_default_driver | Skipped | 0.02 | 
test_hostha_simulator.py
   test_ha_verify_fsm_recovering | Skipped | 0.03 | test_hostha_simulator.py
   test_ha_verify_fsm_fenced | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_verify_fsm_degraded | Skipped | 0.03 | test_hostha_simulator.py
   test_ha_verify_fsm_available | Skipped | 0.03 | test_hostha_simulator.py
   test_ha_multiple_mgmt_server_ownership | Skipped | 0.04 | 
test_hostha_simulator.py
   test_ha_list_providers | Skipped | 0.03 | test_hostha_simulator.py
   test_ha_enabledisable_across_clusterzones | Skipped | 0.03 | 
test_hostha_simulator.py
   test_ha_enable_feature_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_ha_disable_feature_invalid | Skipped | 0.02 | test_hostha_simulator.py
   test_configure_ha_provider_valid | Skipped | 0.03 | test_hostha_simulator.py
   test_configure_ha_provider_invalid | Skipped | 0.03 | 
test_hostha_simulator.py
   test_deploy_vgpu_enabled_vm | Skipped | 0.03 | test_deploy_vgpu_enabled_vm.py
   test_3d_gpu_support | Skipped | 0.04 | test_deploy_vgpu_enabled_vm.py
   
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] vedulasantosh commented on a change in pull request #2216: CLOUDSTACK-10027 Repeating the same list for Internal LB in VPC

[git]

vedulasantosh commented on a change in pull request #2216: CLOUDSTACK-10027 
Repeating the same list for Internal LB in VPC
URL: https://github.com/apache/cloudstack/pull/2216#discussion_r134726488
 
 

 ##
 File path: ui/scripts/vpc.js
 ##
 @@ -745,12 +745,24 @@
 }
 },
 dataProvider: function(args) {
+var array1 = [];
+if (args.filterBy != null) {
+if (args.filterBy.search != null && 
args.filterBy.search.by != null && args.filterBy.search.value != null) {
+if (args.filterBy.search.by === "name") {
+if (args.filterBy.search.value.length > 0)
+array1.push("=" + 
args.filterBy.search.value);
+}
+}
+}
+var data = {
+page: args.page,
+pageSize: pageSize,
+networkid: args.context.networks[0].id,
+listAll: true
+};
 $.ajax({
-url: createURL('listLoadBalancers'),
-data: {
-networkid: args.context.networks[0].id,
-listAll: true
-},
+url: createURL('listLoadBalancers' + 
array1.join("")),
+data: data,
 success: function(json) {
 
 Review comment:
   @rhtyd Changed the code as mentioned.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2214: Speed-up VR initialisation/configuration

[git]

blueorangutan commented on issue #2214: Speed-up VR initialisation/configuration
URL: https://github.com/apache/cloudstack/pull/2214#issuecomment-324295242
 
 
   Packaging result: ?centos6 ?centos7 ?debian. JID-1011
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] cloudmonger commented on issue #2054: CLOUDSTACK-9886 : After restarting cloudstack-management , It takes time to connect hosts

[git]

cloudmonger commented on issue #2054: CLOUDSTACK-9886 : After restarting 
cloudstack-management , It takes time to connect hosts
URL: https://github.com/apache/cloudstack/pull/2054#issuecomment-324293478
 
 
   ### ACS CI BVT Run
**Sumarry:**
Build Number 1132
Hypervisor xenserver
NetworkType Advanced
Passed=108
Failed=6
Skipped=12
   
   _Link to logs Folder (search by build_no):_ 
https://www.dropbox.com/sh/r2si930m8xxzavs/AAAzNrnoF1fC3auFrvsKo_8-a?dl=0
   
   **Failed tests:**
   * test_router_dns.py
   
* test_router_dns_guestipquery Failing since 1 runs
   
   * test_non_contigiousvlan.py
   
* test_extendPhysicalNetworkVlan Failing since 2 runs
   
   * test_volumes.py
   
* test_06_download_detached_volume Failed
   
   * test_routers_network_ops.py
   
* test_02_isolate_network_FW_PF_default_routes_egress_false Failing since 
127 runs
   
* test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failing since 
123 runs
   
* test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failing since 
123 runs
   
   
   **Skipped tests:**
   test_vm_nic_adapter_vmxnet3
   test_01_verify_libvirt
   test_02_verify_libvirt_after_restart
   test_03_verify_libvirt_attach_disk
   test_04_verify_guest_lspci
   test_05_change_vm_ostype_restart
   test_06_verify_guest_lspci_again
   test_static_role_account_acls
   test_11_ss_nfs_version_on_ssvm
   test_nested_virtualization_vmware
   test_3d_gpu_support
   test_deploy_vgpu_enabled_vm
   
   **Passed test suits:**
   test_deploy_vm_with_userdata.py
   test_affinity_groups_projects.py
   test_portable_publicip.py
   test_vm_snapshots.py
   test_over_provisioning.py
   test_global_settings.py
   test_router_dnsservice.py
   test_scale_vm.py
   test_service_offerings.py
   test_routers_iptables_default_policy.py
   test_loadbalance.py
   test_routers.py
   test_reset_vm_on_reboot.py
   test_deploy_vms_with_varied_deploymentplanners.py
   test_network.py
   test_login.py
   test_deploy_vm_iso.py
   test_list_ids_parameter.py
   test_public_ip_range.py
   test_multipleips_per_nic.py
   test_metrics_api.py
   test_regions.py
   test_affinity_groups.py
   test_network_acl.py
   test_pvlan.py
   test_nic.py
   test_deploy_vm_root_resize.py
   test_resource_detail.py
   test_secondary_storage.py
   test_vm_life_cycle.py
   test_disk_offerings.py
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] blueorangutan commented on issue #2214: Speed-up VR initialisation/configuration

[git]

blueorangutan commented on issue #2214: Speed-up VR initialisation/configuration
URL: https://github.com/apache/cloudstack/pull/2214#issuecomment-324290676
 
 
   @DagSonsteboSB a Jenkins job has been kicked to build packages. I'll keep 
you posted as I make progress.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DagSonsteboSB commented on issue #2214: Speed-up VR initialisation/configuration

[git]

DagSonsteboSB commented on issue #2214: Speed-up VR initialisation/configuration
URL: https://github.com/apache/cloudstack/pull/2214#issuecomment-324290551
 
 
   @blueorangutan package
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134705330
 
 

 ##
 File path: server/src/org/apache/cloudstack/ca/CAManagerImpl.java
 ##
 @@ -0,0 +1,427 @@
+// 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 org.apache.cloudstack.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.ca.IssueCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCAProvidersCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCaCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ProvisionCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.RevokeCertificateCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollManager;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.alert.AlertManager;
+import com.cloud.certificate.CrlVO;
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+
+public class CAManagerImpl extends ManagerBase implements CAManager {
+public static final Logger LOG = Logger.getLogger(CAManagerImpl.class);
+
+@Inject
+private CrlDao crlDao;
+@Inject
+private HostDao hostDao;
+@Inject
+private AgentManager agentManager;
+@Inject
+private BackgroundPollManager backgroundPollManager;
+@Inject
+private AlertManager alertManager;
+
+private static CAProvider configuredCaProvider;
+private static Map caProviderMap = new HashMap<>();
+private static Map alertMap = new ConcurrentHashMap<>();
+private static Map activeCertMap = new 
ConcurrentHashMap<>();
+
+private List caProviders;
+
+private CAProvider getConfiguredCaProvider() {
+if (configuredCaProvider == null && 
caProviderMap.containsKey(CAProviderPlugin.value())) {
+configuredCaProvider = caProviderMap.get(CAProviderPlugin.value());
+}
+if (configuredCaProvider == null) {
+throw new CloudRuntimeException("Failed to find default configured 
CA provider plugin");
+}
+return configuredCaProvider;
+}
+
+private CAProvider getCAProvider(final String provider) {
+if (Strings.isNullOrEmpty(provider)) {
+return getConfiguredCaProvider();
+}
+final String caProviderName = provider.toLowerCase();
+if (!caProviderMap.containsKey(caProviderName)) {
+

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134695744
 
 

 ##
 File path: 
plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java
 ##
 @@ -0,0 +1,572 @@
+// 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 org.apache.cloudstack.ca.provider;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.KeyManagementException;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.SecureRandom;
+import java.security.Security;
+import java.security.SignatureException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.log4j.Logger;
+import org.bouncycastle.jce.PKCS10CertificationRequest;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
+
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.nio.Link;
+import com.google.common.base.Strings;
+
+public final class RootCAProvider extends AdapterBase implements CAProvider, 
Configurable {
+private static final Logger LOG = Logger.getLogger(RootCAProvider.class);
+
+public static final Integer caValidityYears = 30;
+public static final String caAlias = "root";
+public static final String managementAlias = "management";
+
+private static KeyPair caKeyPair = null;
+private static X509Certificate caCertificate = null;
+
+@Inject
+private ConfigurationDao configDao;
+@Inject
+private CrlDao crlDao;
+
+
+/// Root CA Settings ///
+
+
+private static ConfigKey rootCAPrivateKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.private.key",
+null,
+"The ROOT CA private key.", true);
+
+private static ConfigKey rootCAPublicKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.public.key",
+null,
+"The ROOT CA public key.", true);
+
+private static ConfigKey rootCACertificate = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.ca.certificate",
+null,
+"The ROOT CA certificate.", true);
+
+private 

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134409796
 
 

 ##
 File path: api/src/org/apache/cloudstack/alert/AlertService.java
 ##
 @@ -67,6 +67,7 @@ private AlertType(short type, String name, boolean 
isDefault) {
 public static final AlertType ALERT_TYPE_SYNC = new 
AlertType((short)27, "ALERT.TYPE.SYNC", true);
 public static final AlertType ALERT_TYPE_UPLOAD_FAILED = new 
AlertType((short)28, "ALERT.UPLOAD.FAILED", true);
 public static final AlertType ALERT_TYPE_OOBM_AUTH_ERROR = new 
AlertType((short)29, "ALERT.OOBM.AUTHERROR", true);
+public static final AlertType ALERT_TYPE_CA_CERT = new 
AlertType((short)31, "ALERT.CA.CERT", true);
 
 Review comment:
   we are skipping alert type 30, here. Is that intentional?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134431321
 
 

 ##
 File path: 
engine/orchestration/src/com/cloud/agent/manager/ClusteredAgentManagerImpl.java
 ##
 @@ -495,28 +495,29 @@ public SocketChannel connectToPeer(final String 
peerName, final SocketChannel pr
 }
 final String ip = ms.getServiceIP();
 InetAddress addr;
+int port = Port.value();
 try {
 addr = InetAddress.getByName(ip);
 } catch (final UnknownHostException e) {
 throw new CloudRuntimeException("Unable to resolve " + ip);
 }
 SocketChannel ch1 = null;
 try {
-ch1 = SocketChannel.open(new InetSocketAddress(addr, 
Port.value()));
+ch1 = SocketChannel.open(new InetSocketAddress(addr, 
port));
 
 Review comment:
   SocketChannel is a Closable. We should reduce our proprietary error handling 
here and try-with-resource.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134696200
 
 

 ##
 File path: 
plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java
 ##
 @@ -0,0 +1,572 @@
+// 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 org.apache.cloudstack.ca.provider;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.KeyManagementException;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.SecureRandom;
+import java.security.Security;
+import java.security.SignatureException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.log4j.Logger;
+import org.bouncycastle.jce.PKCS10CertificationRequest;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
+
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.nio.Link;
+import com.google.common.base.Strings;
+
+public final class RootCAProvider extends AdapterBase implements CAProvider, 
Configurable {
+private static final Logger LOG = Logger.getLogger(RootCAProvider.class);
+
+public static final Integer caValidityYears = 30;
+public static final String caAlias = "root";
+public static final String managementAlias = "management";
+
+private static KeyPair caKeyPair = null;
+private static X509Certificate caCertificate = null;
+
+@Inject
+private ConfigurationDao configDao;
+@Inject
+private CrlDao crlDao;
+
+
+/// Root CA Settings ///
+
+
+private static ConfigKey rootCAPrivateKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.private.key",
+null,
+"The ROOT CA private key.", true);
+
+private static ConfigKey rootCAPublicKey = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.public.key",
+null,
+"The ROOT CA public key.", true);
+
+private static ConfigKey rootCACertificate = new 
ConfigKey<>("Hidden", String.class,
+"ca.plugin.root.ca.certificate",
+null,
+"The ROOT CA certificate.", true);
+
+private 

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134686114
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/Certificate.java
 ##
 @@ -0,0 +1,46 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.security.PrivateKey;
+import java.security.cert.X509Certificate;
+import java.util.List;
+
+public class Certificate {
+private X509Certificate clientCertificate;
+private PrivateKey privateKey;
+private List caCertificates;
+
+public Certificate(final X509Certificate clientCertificate, final 
PrivateKey privateKey, final List caCertificates) {
+this.clientCertificate = clientCertificate;
+this.privateKey = privateKey;
+this.caCertificates = caCertificates;
+}
+
+public X509Certificate getClientCertificate() {
+return clientCertificate;
+}
+
+public PrivateKey getPrivateKey() {
 
 Review comment:
   this is returned in responses. I have not found a point of control for it 
yet.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134685640
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
+ */
+boolean canProvisionCertificates();
+
+/**
+ * Returns root CA certificate
+ * @return returns concatenated root CA certificate string
+ */
+List getCaCertificate();
+
+/**
+ * Issues certificate with provided options
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final List domainNames, final 
List ipAddresses, final int validityDays);
+
+/**
+ * Issues certificate using given CSR and other options
+ * @param csr
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final String csr, final List 
domainNames, final List ipAddresses, final int validityDays);
+
+/**
+ * Revokes certificate using certificate serial and CN
+ * @param certSerial
+ * @param certCn
+ * @return returns true on success
+ */
+boolean revokeCertificate(final BigInteger certSerial, final String 
certCn);
+
+/**
+ * This method can add/inject custom TrustManagers for client connection 
validations.
+ * @param sslContext The SSL context used while accepting a client 
connection
+ * @param remoteAddress
+ * @param certMap
+ * @return
+ * @throws GeneralSecurityException
+ * @throws IOException
+ */
+SSLEngine createSSLEngine(final SSLContext sslContext, final String 
remoteAddress, final Map certMap) throws 
GeneralSecurityException, IOException;
+
+/**
+ * Returns the unique name of the provider
+ * @return
+ */
+String getProviderName();
+
+/**
+ * Returns description about the CA provider plugin
+ * @return
 
 Review comment:
   empty doctag
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134688213
 
 

 ##
 File path: 
plugins/ca/root-ca/src/org/apache/cloudstack/ca/provider/RootCAProvider.java
 ##
 @@ -0,0 +1,572 @@
+// 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 org.apache.cloudstack.ca.provider;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.InvalidKeyException;
+import java.security.KeyManagementException;
+import java.security.KeyPair;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.NoSuchProviderException;
+import java.security.SecureRandom;
+import java.security.Security;
+import java.security.SignatureException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.security.spec.InvalidKeySpecException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.KeyManagerFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.TrustManagerFactory;
+import javax.net.ssl.X509TrustManager;
+
+import org.apache.cloudstack.ca.CAManager;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.KeyStoreUtils;
+import org.apache.log4j.Logger;
+import org.bouncycastle.jce.PKCS10CertificationRequest;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.util.io.pem.PemObject;
+import org.bouncycastle.util.io.pem.PemReader;
+
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.utils.PropertiesUtil;
+import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.db.DbProperties;
+import com.cloud.utils.db.GlobalLock;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.nio.Link;
+import com.google.common.base.Strings;
+
+public final class RootCAProvider extends AdapterBase implements CAProvider, 
Configurable {
+private static final Logger LOG = Logger.getLogger(RootCAProvider.class);
+
+public static final Integer caValidityYears = 30;
+public static final String caAlias = "root";
+public static final String managementAlias = "management";
+
+private static KeyPair caKeyPair = null;
+private static X509Certificate caCertificate = null;
+
+@Inject
+private ConfigurationDao configDao;
+@Inject
+private CrlDao crlDao;
+
+
+/// Root CA Settings ///
+
+
+private static ConfigKey rootCAPrivateKey = new 
ConfigKey<>("Hidden", String.class,
 
 Review comment:
   hidden is fine :+1: but will they be encrypted in the configuration table?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134705103
 
 

 ##
 File path: server/src/org/apache/cloudstack/ca/CAManagerImpl.java
 ##
 @@ -0,0 +1,427 @@
+// 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 org.apache.cloudstack.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.ca.IssueCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCAProvidersCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCaCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ProvisionCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.RevokeCertificateCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollManager;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.alert.AlertManager;
+import com.cloud.certificate.CrlVO;
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+
+public class CAManagerImpl extends ManagerBase implements CAManager {
+public static final Logger LOG = Logger.getLogger(CAManagerImpl.class);
+
+@Inject
+private CrlDao crlDao;
+@Inject
+private HostDao hostDao;
+@Inject
+private AgentManager agentManager;
+@Inject
+private BackgroundPollManager backgroundPollManager;
+@Inject
+private AlertManager alertManager;
+
+private static CAProvider configuredCaProvider;
+private static Map caProviderMap = new HashMap<>();
+private static Map alertMap = new ConcurrentHashMap<>();
+private static Map activeCertMap = new 
ConcurrentHashMap<>();
+
+private List caProviders;
+
+private CAProvider getConfiguredCaProvider() {
+if (configuredCaProvider == null && 
caProviderMap.containsKey(CAProviderPlugin.value())) {
+configuredCaProvider = caProviderMap.get(CAProviderPlugin.value());
+}
+if (configuredCaProvider == null) {
+throw new CloudRuntimeException("Failed to find default configured 
CA provider plugin");
+}
+return configuredCaProvider;
+}
+
+private CAProvider getCAProvider(final String provider) {
+if (Strings.isNullOrEmpty(provider)) {
+return getConfiguredCaProvider();
+}
+final String caProviderName = provider.toLowerCase();
+if (!caProviderMap.containsKey(caProviderName)) {
+

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134711534
 
 

 ##
 File path: 
utils/src/main/java/org/apache/cloudstack/utils/security/KeyStoreUtils.java
 ##
 @@ -0,0 +1,70 @@
+//
+// 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 org.apache.cloudstack.utils.security;
+
+import java.io.File;
+import java.io.IOException;
+
+import com.cloud.utils.script.Script;
+import com.google.common.base.Strings;
+
+public class KeyStoreUtils {
+
+public static String defaultTmpKeyStoreFile = "/tmp/tmp.jks";
+public static String defaultKeystoreFile = "/cloud.jks";
 
 Review comment:
   with "/" seems dangerous/vulnerable. I'd change it to "~/" or none (and move 
the '/' to a directory configuration.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134400617
 
 

 ##
 File path: agent/src/com/cloud/agent/Agent.java
 ##
 @@ -166,7 +178,8 @@ public Agent(final IAgentShell shell, final int 
localAgentId, final ServerResour
 throw new ConfigurationException("Unable to configure " + 
_resource.getName());
 }
 
-_connection = new NioClient("Agent", _shell.getHost(), 
_shell.getPort(), _shell.getWorkers(), this);
+final String host = _shell.getHost();
 
 Review comment:
   This final seems strange, as we intend to loop over adresses, right? (I 
might be mixing things up here, please do tell)
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134405232
 
 

 ##
 File path: agent/src/com/cloud/agent/Agent.java
 ##
 @@ -464,7 +493,7 @@ protected void processRequest(final Request request, final 
Link link) {
 
 for (int i = 0; i < cmds.length; i++) {
 final Command cmd = cmds[i];
-Answer answer;
+Answer answer = null;
 
 Review comment:
   not a new bug but this explicit nulling sparks the thought that at line 574, 
we might have a nullpointer in answer.getResult(). By the looks each branch is 
covered, though. The null is used for the "unsupported" situation. Does it make 
sense to set the unsupported answer here and then override it if it is 
supported anyway?
   i.e. create the unsupported answer outside the loop and assign it on this 
line in every iteration, with a setCmd() kind of call.
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134700719
 
 

 ##
 File path: 
server/src/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java
 ##
 @@ -125,6 +137,73 @@ public boolean processTimeout(long agentId, long seq) {
 return false;
 }
 
+private void setupAgentSecurity(final Connection sshConnection, final 
String agentIp, final String agentHostname) {
 
 Review comment:
   rather big method. Can we split in keystore setup, certificate creation, 
certificate loading?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134707291
 
 

 ##
 File path: setup/db/server-setup.sql
 ##
 @@ -27,3 +27,6 @@ INSERT INTO `cloud`.`configuration` (category, instance, 
component, name, value,
 
 -- Enable dynamic RBAC by default for fresh deployments
 INSERT INTO `cloud`.`configuration` (category, instance, component, name, 
value) VALUES ('Advanced', 'DEFAULT', 'RoleService', 
'dynamic.apichecker.enabled', 'true');
+
+-- Enable RootCA auth strictness for fresh deployments
+INSERT INTO `cloud`.`configuration` (category, instance, component, name, 
value) VALUES ('Advanced', 'DEFAULT', 'RootCAProvider', 
'ca.plugin.root.auth.strictness', 'true');
 
 Review comment:
   isn't ConfigKey taking care of this?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134685039
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
+ */
+boolean canProvisionCertificates();
+
+/**
+ * Returns root CA certificate
+ * @return returns concatenated root CA certificate string
+ */
+List getCaCertificate();
+
+/**
+ * Issues certificate with provided options
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
 
 Review comment:
   empty annotation
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134685571
 
 

 ##
 File path: framework/ca/src/org/apache/cloudstack/framework/ca/CAProvider.java
 ##
 @@ -0,0 +1,93 @@
+// 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 org.apache.cloudstack.framework.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
+import java.util.List;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+public interface CAProvider {
+
+/**
+ * Method returns capability of the plugin to participate in certificate 
issuance, revocation and provisioning
+ * @return
+ */
+boolean canProvisionCertificates();
+
+/**
+ * Returns root CA certificate
+ * @return returns concatenated root CA certificate string
+ */
+List getCaCertificate();
+
+/**
+ * Issues certificate with provided options
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final List domainNames, final 
List ipAddresses, final int validityDays);
+
+/**
+ * Issues certificate using given CSR and other options
+ * @param csr
+ * @param domainNames
+ * @param ipAddresses
+ * @param validityDays
+ * @return
+ */
+Certificate issueCertificate(final String csr, final List 
domainNames, final List ipAddresses, final int validityDays);
+
+/**
+ * Revokes certificate using certificate serial and CN
+ * @param certSerial
+ * @param certCn
+ * @return returns true on success
+ */
+boolean revokeCertificate(final BigInteger certSerial, final String 
certCn);
+
+/**
+ * This method can add/inject custom TrustManagers for client connection 
validations.
+ * @param sslContext The SSL context used while accepting a client 
connection
+ * @param remoteAddress
+ * @param certMap
+ * @return
+ * @throws GeneralSecurityException
+ * @throws IOException
+ */
+SSLEngine createSSLEngine(final SSLContext sslContext, final String 
remoteAddress, final Map certMap) throws 
GeneralSecurityException, IOException;
+
+/**
+ * Returns the unique name of the provider
+ * @return
 
 Review comment:
   empty annotation
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134704534
 
 

 ##
 File path: server/src/org/apache/cloudstack/ca/CAManagerImpl.java
 ##
 @@ -0,0 +1,427 @@
+// 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 org.apache.cloudstack.ca;
+
+import java.io.IOException;
+import java.math.BigInteger;
+import java.security.GeneralSecurityException;
+import java.security.cert.CertificateExpiredException;
+import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLEngine;
+
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.admin.ca.IssueCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCAProvidersCmd;
+import org.apache.cloudstack.api.command.admin.ca.ListCaCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.ProvisionCertificateCmd;
+import org.apache.cloudstack.api.command.admin.ca.RevokeCertificateCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.framework.ca.CAProvider;
+import org.apache.cloudstack.framework.ca.Certificate;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.poll.BackgroundPollManager;
+import org.apache.cloudstack.poll.BackgroundPollTask;
+import org.apache.cloudstack.utils.identity.ManagementServerNode;
+import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.alert.AlertManager;
+import com.cloud.certificate.CrlVO;
+import com.cloud.certificate.dao.CrlDao;
+import com.cloud.event.ActionEvent;
+import com.cloud.event.EventTypes;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.Host;
+import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
+import com.cloud.utils.component.ManagerBase;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.google.common.base.Strings;
+
+public class CAManagerImpl extends ManagerBase implements CAManager {
+public static final Logger LOG = Logger.getLogger(CAManagerImpl.class);
+
+@Inject
+private CrlDao crlDao;
+@Inject
+private HostDao hostDao;
+@Inject
+private AgentManager agentManager;
+@Inject
+private BackgroundPollManager backgroundPollManager;
+@Inject
+private AlertManager alertManager;
+
+private static CAProvider configuredCaProvider;
+private static Map caProviderMap = new HashMap<>();
+private static Map alertMap = new ConcurrentHashMap<>();
+private static Map activeCertMap = new 
ConcurrentHashMap<>();
+
+private List caProviders;
+
+private CAProvider getConfiguredCaProvider() {
+if (configuredCaProvider == null && 
caProviderMap.containsKey(CAProviderPlugin.value())) {
+configuredCaProvider = caProviderMap.get(CAProviderPlugin.value());
+}
+if (configuredCaProvider == null) {
+throw new CloudRuntimeException("Failed to find default configured 
CA provider plugin");
+}
+return configuredCaProvider;
+}
+
+private CAProvider getCAProvider(final String provider) {
+if (Strings.isNullOrEmpty(provider)) {
+return getConfiguredCaProvider();
+}
+final String caProviderName = provider.toLowerCase();
+if (!caProviderMap.containsKey(caProviderName)) {
+

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134683772
 
 

 ##
 File path: engine/schema/src/com/cloud/certificate/dao/CrlDaoImpl.java
 ##
 @@ -0,0 +1,57 @@
+// 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.certificate.dao;
+
+import java.math.BigInteger;
+
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.certificate.CrlVO;
+import com.cloud.utils.db.DB;
+import com.cloud.utils.db.GenericDaoBase;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+
+@DB
+public class CrlDaoImpl extends GenericDaoBase implements CrlDao {
+
+private final SearchBuilder CrlBySerialSearch;
+
+public CrlDaoImpl() {
+super();
+
+CrlBySerialSearch = createSearchBuilder();
+CrlBySerialSearch.and("certSerial", 
CrlBySerialSearch.entity().getCertSerial(), SearchCriteria.Op.EQ);
+CrlBySerialSearch.done();
+}
+
+@Override
+public CrlVO findBySerial(final BigInteger certSerial) {
 
 Review comment:
   from the looks of this (public) method it seems that an error in the 
format/serialisation of the serial or in the db access, the certificate might 
accidentally be approved. Do we run that risk?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: Securing Agents Communications

[git]

DaanHoogland commented on a change in pull request #2239: CLOUDSTACK-9993: 
Securing Agents Communications
URL: https://github.com/apache/cloudstack/pull/2239#discussion_r134706989
 
 

 ##
 File path: 
services/secondary-storage/controller/src/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
 ##
 @@ -1118,7 +1119,7 @@ public boolean 
finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl
 
 StringBuilder buf = profile.getBootArgsBuilder();
 buf.append(" template=domP type=secstorage");
-buf.append(" 
host=").append(ApiServiceConfiguration.ManagementHostIPAdr.value());
+buf.append(" 
host=").append(StringUtils.shuffleCSVList(ApiServiceConfiguration.ManagementHostIPAdr.value()));
 
 Review comment:
   is this the only algorithm and do we always want to apply it?
 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services