Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check expiry date of the root/intermediate before using it to sign a leaf #10500

Merged
merged 24 commits into from
Jul 13, 2021

Conversation

dhiaayachi
Copy link
Contributor

@dhiaayachi dhiaayachi commented Jun 25, 2021

Fix #9573
This need to be merged after #10479

@dhiaayachi dhiaayachi requested a review from dnephin June 25, 2021 19:18
Copy link
Contributor

@kyhavlov kyhavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, just a few comments around errors/formatting

agent/consul/leader_connect_ca.go Outdated Show resolved Hide resolved
agent/consul/leader_connect_ca.go Show resolved Hide resolved
agent/consul/leader_connect_ca.go Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 5, 2021 12:25 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 5, 2021 12:25 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 5, 2021 12:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 5, 2021 12:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 5, 2021 12:48 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 5, 2021 12:48 Inactive
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I think this is looking good. One question and one suggestion for speeding up the test.

agent/consul/leader_connect_ca.go Outdated Show resolved Hide resolved
agent/consul/leader_connect_ca_test.go Outdated Show resolved Hide resolved
agent/consul/leader_connect_ca_test.go Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – consul July 9, 2021 15:19 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 9, 2021 15:19 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 9, 2021 15:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 9, 2021 15:31 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 9, 2021 18:09 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 9, 2021 18:09 Inactive
dnephin and others added 5 commits July 12, 2021 09:49
This further decouples the CAManager from Server. It reduces the interface between them and
removes the need for the SetLogger method on providers.
To reduce the scope of Server, and keep all the CA logic together
Most of these methods are used exclusively for the AutoConfig RPC
endpoint. This PR uses a pattern that we've used in other places as an
incremental step to reducing the scope of Server.
@dhiaayachi dhiaayachi force-pushed the dnephin/ca-provider-explore-2 branch from ecb30aa to 6356302 Compare July 12, 2021 14:35
@vercel vercel bot temporarily deployed to Preview – consul July 12, 2021 15:11 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 12, 2021 15:11 Inactive
@dnephin dnephin force-pushed the dnephin/ca-provider-explore-2 branch from 4fb20c8 to 5ed56fc Compare July 12, 2021 17:52
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 12, 2021 18:59 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 12, 2021 18:59 Inactive
@dhiaayachi dhiaayachi changed the base branch from dnephin/ca-provider-explore-2 to main July 12, 2021 19:00
@dhiaayachi dhiaayachi changed the base branch from main to dnephin/ca-provider-explore-2 July 12, 2021 19:01
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 12, 2021 20:19 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 12, 2021 20:19 Inactive
Base automatically changed from dnephin/ca-provider-explore-2 to main July 12, 2021 23:03
@vercel vercel bot temporarily deployed to Preview – consul July 13, 2021 00:33 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 13, 2021 00:33 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 13, 2021 00:55 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 13, 2021 00:55 Inactive
Copy link
Contributor

@freddygv freddygv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left one minor suggestion

agent/consul/leader_connect_ca.go Outdated Show resolved Hide resolved
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview – consul July 13, 2021 15:49 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 13, 2021 15:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul July 13, 2021 15:53 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 13, 2021 15:53 Inactive
@dhiaayachi dhiaayachi changed the title check expiry date of the intermediate before using it to sign a leaf check expiry date of the root/intermediate before using it to sign a leaf Jul 13, 2021
@dhiaayachi dhiaayachi merged commit 58bd817 into main Jul 13, 2021
@dhiaayachi dhiaayachi deleted the error-intermediate-cert-expire branch July 13, 2021 16:15
@hc-github-team-consul-core
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/408262.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check validity of certificates before they are used in Consul Connect to sign leaf certs
5 participants