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

[ca] Fix the UpdateRootCA function to also update the ExternalCA's rootCA #2210

Merged
merged 1 commit into from
Jun 5, 2017

Conversation

cyli
Copy link
Contributor

@cyli cyli commented Jun 1, 2017

Since if it's not updated when getting a cert signed by an external CA, no intermediates are appended.

Previously the test for this (TestRequestAndSaveNewCertificatesWithIntermediates) passed due a combination of both this bug and a bug in how the test CA utility worked (it updated the RootCA to one without intermediates, but since updating the root CA didn't actually propagate to the external CA signing object, the external CA signer did produce a cert with intermediates).

I've updated the test CA utility and the tests as well. I'll also open a PR against the 17.06 branch.

cc @aaronlehmann @diogomonica @jlhawn

@cyli cyli changed the title Fix the UpdateRootCA function to also update the ExternalCA's rootCA [ca] Fix the UpdateRootCA function to also update the ExternalCA's rootCA Jun 1, 2017
@codecov
Copy link

codecov bot commented Jun 1, 2017

Codecov Report

Merging #2210 into master will decrease coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2210      +/-   ##
==========================================
- Coverage   60.23%   60.16%   -0.07%     
==========================================
  Files         124      124              
  Lines       20143    20121      -22     
==========================================
- Hits        12133    12106      -27     
- Misses       6647     6664      +17     
+ Partials     1363     1351      -12

@cyli cyli added this to the 17.06 milestone Jun 2, 2017
ca/config.go Outdated

s.externalCA.mu.Lock()
s.externalCA.rootCA = rootCA
s.externalCA.mu.Unlock()
Copy link
Collaborator

Choose a reason for hiding this comment

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

The rootCA field is used without holding the mutex in Sign.

Also, could you please add a setter method for this? There is already one for URLs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks, added.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not seeing the setter used here, but GitHub seems to be acting up, so maybe it's not showing the full change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Github is 500'ing for me a bunch. :| I'll force push again maybe.

since if it's not updated when getting a cert signed by an external CA,
no intermediates are appended

Signed-off-by: Ying Li <ying.li@docker.com>
@aaronlehmann
Copy link
Collaborator

LGTM

@aaronlehmann aaronlehmann merged commit e9b3c09 into moby:master Jun 5, 2017
@cyli cyli deleted the fix-external-root-ca branch June 5, 2017 19:21
tiborvass pushed a commit to aluzzardi/docker-ce that referenced this pull request Jun 6, 2017
Includes:
- moby/swarmkit#2203
- moby/swarmkit#2210
- moby/swarmkit#2212

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Feb 3, 2020
Includes:
- moby/swarmkit#2203
- moby/swarmkit#2210
- moby/swarmkit#2212

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Feb 3, 2020
Includes:
- moby/swarmkit#2203
- moby/swarmkit#2210
- moby/swarmkit#2212

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Mar 10, 2020
Includes:
- moby/swarmkit#2203
- moby/swarmkit#2210
- moby/swarmkit#2212

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Mar 23, 2020
Includes:
- moby/swarmkit#2203
- moby/swarmkit#2210
- moby/swarmkit#2212

Signed-off-by: Andrea Luzzardi <aluzzardi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants