Allow host cert renewals even if client auth strictness is false by Slair1 · Pull Request #4852 · apache/cloudstack

Slair1 · 2021-03-22T13:44:13Z

Description

This PR allows the management servers to renew certificates issued to hosts, even if ca.plugin.root.auth.strictness is false. There is already a global setting named ca.framework.cert.automatic.renewal that should be controlling the certificate renewals. However, ca.plugin.root.auth.strictness currently circumvents that setting.

More Detail

The ca.plugin.root.auth.strictness setting is checked early in the client certificate checking method and exits the method without adding the certificate to the management server's in-memory certificate map if strictness is false. The certificate renewal process (if enabled with ca.framework.cert.automatic.renewal) reads that in-memory certificate map to determine what certs it could renew. Those two settings shouldn't be so closely related.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
[X ] Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

We've deployed and tested this in our CloudStack environments.

plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java

...ins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java

Slair1 · 2021-04-04T23:17:53Z

Update PR with more tests and now when not in strict mode it will still log issues with certificates, but just accept the cert still.

...ins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java

includes updated tests and logging

Slair1 · 2021-05-12T15:39:39Z

Let me know if i can do anything else to help this along

DaanHoogland · 2021-05-25T08:49:38Z

@sureshanaparti do you approve now?

sureshanaparti

code LGTM.

yadvr · 2021-05-25T13:09:47Z

@blueorangutan package

blueorangutan · 2021-05-25T13:11:02Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-05-25T13:36:24Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 87

yadvr · 2021-05-25T13:41:53Z

@blueorangutan test

blueorangutan · 2021-05-25T13:43:03Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2021-05-26T17:30:03Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2021-05-26T18:02:23Z

Trillian Build Failed (tid-753)

blueorangutan · 2021-05-27T21:24:57Z

Trillian test result (tid-764)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32757 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4852-t764-kvm-centos7.zip
Smoke tests completed. 88 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

yadvr · 2021-06-18T01:10:56Z

Needs some manual testing/validation, otherwise lgtm

nvazquez · 2021-06-23T14:42:41Z

@blueorangutan package

blueorangutan · 2021-06-23T14:43:03Z

@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-06-23T15:49:13Z

Packaging result: ✔️ centos7 ✔️ centos8 ✔️ debian. SL-JID 338

nvazquez · 2021-06-23T17:08:54Z

@blueorangutan test

blueorangutan · 2021-06-23T17:10:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2021-06-24T04:13:46Z

Trillian test result (tid-1070)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37786 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4852-t1070-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 87 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestKubernetesCluster>:teardown	`Error`	71.22	test_kubernetes_clusters.py

nvazquez · 2021-07-13T20:47:41Z

@weizhouapache can you please test this PR?

weizhouapache

@Slair1 @nvazquez
tested ok. except two small questions.

(1) change global settings (just for my testing, do not set it on production)
ca.framework.cert.expiry.alert.period , from 15 to 1500 (days)
ca.framework.background.task.delay, from 3600 to 60 (seconds)
ca.plugin.root.auth.strictness , from true to false.

(2) add debug info in server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java

             try {
-                if (LOG.isTraceEnabled()) {
-                    LOG.trace("CA background task is running...");
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("CA background task is running...");
                 }

(3) without this patch
$ egrep "Certificate is going to expire|CA background task is running" /var/log/cloudstack/management/management-server.log

2021-07-15 07:12:03,827 DEBUG [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-2:ctx-59f9ec37) (logid:7d7a59b2) CA background task is running...
2021-07-15 07:13:30,314 DEBUG [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-5:ctx-e17c3499) (logid:de3f6c2a) CA background task is running...
2021-07-15 07:14:58,516 DEBUG [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-4:ctx-13297c4e) (logid:4dc266a0) CA background task is running...
2021-07-15 07:16:26,420 DEBUG [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-5:ctx-13d9b8c4) (logid:10203dcf) CA background task is running...

(4) with the patch

$ egrep "Certificate is going to expire|CA background task is running" /var/log/cloudstack/management/management-server.log

2021-07-15 09:12:13,759 DEBUG [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-6:ctx-a913172f) (logid:4c23e662) CA background task is running...
2021-07-15 09:12:13,761 WARN [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-6:ctx-a913172f) (logid:4c23e662) Certificate is going to expire for host id=5, uuid=060e2261-8214-4385-b0d4-ce90b6fe2786, name=s-79-VM, ip=10.0.36.8, zone id=1
2021-07-15 09:12:23,912 WARN [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-6:ctx-a913172f) (logid:4c23e662) Certificate is going to expire for host id=2, uuid=fd8384ff-fe21-41e2-84ea-bc70e6d26b97, name=v-1-VM, ip=10.0.36.3, zone id=1
2021-07-15 09:12:34,978 WARN [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-6:ctx-a913172f) (logid:4c23e662) Certificate is going to expire for host id=1, uuid=4ea38782-280c-4122-aea8-63682c16c115, name=ref-trl-1000-k-Mu20-wei-zhou-kvm2, ip=10.0.34.159, zone id=1
2021-07-15 09:12:40,425 WARN [o.a.c.c.CAManagerImpl] (BackgroundTaskPollManager-6:ctx-a913172f) (logid:4c23e662) Certificate is going to expire for host id=4, uuid=d9988a7b-7e02-4e90-9c4a-798c588b1cba, name=ref-trl-1000-k-Mu20-wei-zhou-kvm1, ip=10.0.33.203, zone id=1

...ins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java

weizhouapache · 2021-07-14T13:12:51Z

...ins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java

+            final String errorMsg = String.format("Client certificate has expired with serial=%x, subject=%s from address=%s",
+                    primaryClientCertificate.getSerialNumber(), primaryClientCertificate.getSubjectDN(), clientAddress);
+            LOG.error(errorMsg);
+            if (!allowExpiredCertificate) {


@Slair1 what if allowExpiredCertificate is true ? log the error and silently ignore it ?
should it be appended to exceptionMsg ?

nvazquez · 2021-07-16T03:33:49Z

Thanks @weizhouapache, looks good. @Slair1 can you please check the open comment? Then will be good to go

yadvr · 2021-07-30T05:24:50Z

Ping @slavkap thanks for the PR, it seems like an important fix. Can you address the outstanding comment so we can move forward with this PR?

slavkap · 2021-07-30T07:32:40Z

@rhtyd, unfortunately, I don't have anything with this PR

yadvr · 2021-08-02T10:58:30Z

My bad @slavkap sorry I wrongly selected the Github ID when I typed `@sla... and selected your handle.

I meant @Slair1 can you address the outstanding comment so we can move forward with this PR?

yadvr · 2021-08-09T08:42:02Z

@Slair1 can you address the outstanding comment so we can move forward with this PR?

yadvr · 2021-08-19T07:38:19Z

@blueorangutan package

blueorangutan · 2021-08-19T07:39:02Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2021-08-19T08:16:40Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 929

yadvr · 2021-08-19T10:33:56Z

@blueorangutan test

blueorangutan · 2021-08-19T10:35:02Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2021-08-19T20:37:48Z

Trillian test result (tid-1722)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34368 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4852-t1722-kvm-centos7.zip
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

weizhouapache · 2021-09-01T11:38:08Z

@nvazquez @rhtyd
since we have 2 approvals (including 1 manual test) and integration test passed, can we merge this pr ?

yadvr · 2021-09-01T12:49:16Z

Merging this based on your comment @weizhouapache

DaanHoogland added this to the 4.16.0.0 milestone Mar 23, 2021

DaanHoogland requested a review from sureshanaparti March 23, 2021 08:09

sureshanaparti reviewed Mar 23, 2021

View reviewed changes

plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java Outdated Show resolved Hide resolved

sureshanaparti reviewed Mar 23, 2021

View reviewed changes

...ins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java Outdated Show resolved Hide resolved

sureshanaparti reviewed Mar 23, 2021

View reviewed changes

...ins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java Show resolved Hide resolved

yadvr requested review from nvazquez and yadvr March 24, 2021 06:46

Slair1 force-pushed the sml-master-cert-renewal branch 4 times, most recently from 0d54a45 to c6ba1bf Compare March 24, 2021 16:26

Slair1 marked this pull request as draft March 24, 2021 16:28

Slair1 force-pushed the sml-master-cert-renewal branch from c6ba1bf to 3a90514 Compare March 24, 2021 18:15

Slair1 force-pushed the sml-master-cert-renewal branch 3 times, most recently from 8a83668 to b029acf Compare April 2, 2021 21:29

Slair1 marked this pull request as ready for review April 4, 2021 23:17

yadvr requested a review from sureshanaparti April 5, 2021 09:31

sureshanaparti reviewed Apr 16, 2021

View reviewed changes

...ins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCACustomTrustManager.java Show resolved Hide resolved

allow cert renewal even if auth strictness is false

a56e844

includes updated tests and logging

Slair1 force-pushed the sml-master-cert-renewal branch from b029acf to a56e844 Compare April 19, 2021 17:23

sureshanaparti approved these changes May 25, 2021

View reviewed changes

sureshanaparti requested a review from Spaceman1984 June 27, 2021 16:10

nvazquez added the status:needs-testing label Jul 13, 2021

weizhouapache self-requested a review July 14, 2021 11:45

weizhouapache approved these changes Jul 15, 2021

View reviewed changes

yadvr merged commit 76d5ce3 into apache:main Sep 1, 2021

DaanHoogland removed the status:needs-testing label Jan 28, 2026

Conversation

Slair1 commented Mar 22, 2021

Description

More Detail

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Slair1 commented Apr 4, 2021

Uh oh!

Uh oh!

Slair1 commented May 12, 2021

Uh oh!

DaanHoogland commented May 25, 2021

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented May 25, 2021

Uh oh!

blueorangutan commented May 25, 2021

Uh oh!

blueorangutan commented May 25, 2021

Uh oh!

yadvr commented May 25, 2021

Uh oh!

blueorangutan commented May 25, 2021

Uh oh!

blueorangutan commented May 26, 2021

Uh oh!

blueorangutan commented May 26, 2021

Uh oh!

blueorangutan commented May 27, 2021

Uh oh!

yadvr commented Jun 18, 2021

Uh oh!

nvazquez commented Jun 23, 2021

Uh oh!

blueorangutan commented Jun 23, 2021

Uh oh!

blueorangutan commented Jun 23, 2021

Uh oh!

nvazquez commented Jun 23, 2021

Uh oh!

blueorangutan commented Jun 23, 2021

Uh oh!

blueorangutan commented Jun 24, 2021

Uh oh!

nvazquez commented Jul 13, 2021

Uh oh!

weizhouapache left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weizhouapache Jul 14, 2021

Choose a reason for hiding this comment

Uh oh!

nvazquez commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yadvr commented Jul 30, 2021

Uh oh!

slavkap commented Jul 30, 2021

Uh oh!

yadvr commented Aug 2, 2021

Uh oh!

yadvr commented Aug 9, 2021

Uh oh!

yadvr commented Aug 19, 2021

Uh oh!

blueorangutan commented Aug 19, 2021

Uh oh!

blueorangutan commented Aug 19, 2021

weizhouapache left a comment •

edited

Loading

nvazquez commented Jul 16, 2021 •

edited

Loading