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

Fixed TLS certs validation for consenters (release-2.2) #2005

Merged
merged 4 commits into from Oct 14, 2020

Conversation

wlahti
Copy link
Contributor

@wlahti wlahti commented Oct 12, 2020

Type of change

  • Bug fix
  • Test update

Description

First commit:

  • Fixes usecase when you add new org with a consenter.
    Verification of consenter's TLS certs against simulated config, not the last one. To achieve that, metadata validator interface was changed, now it requires orderer config instead of just consensus metadata. Also, TLS verification was moved to VerifyConfigMetadata function. Added ignoreCertExpiration option to ignore expiration errors when validating config metadata.

Second commit:

  • Integration test to confirm that updating MSP and consenters set in a single config update is successful.

Third commit:

  • Bug fix for validateConsenterTLSCerts. It was accidentally verifying only the clientCert and not the cert that was passed in.

Related issues

FAB-18192
FAB-18269

@wlahti wlahti requested a review from a team as a code owner October 12, 2020 19:29
@wlahti wlahti marked this pull request as draft October 12, 2020 19:39
kopaygorodsky and others added 3 commits October 12, 2020 17:22
* FAB-18192 Fixed TLS certs validation for consenters.
Verification of TLS cert against simulated config, not the last one. To achieve that, metadata validator interface was changed, now it requires orderer config instead of just consensus metadata. Also, TLS verification was moved to VerifyMetadata function, it shouldn't have been part of ComputeMembershipChanges. Fixed tests.

Signed-off-by: Vladyslav Kopaihorodskyi <vlad.kopaygorodsky@gmail.com>
FAB-18192

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
It was accidentally verifying only the clientCert and
not the cert that was passed in.

FAB-18269

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
@wlahti wlahti marked this pull request as ready for review October 12, 2020 21:33
Signed-off-by: Will Lahti <wtlahti@us.ibm.com>
Copy link
Contributor

@mastersingh24 mastersingh24 left a comment

Choose a reason for hiding this comment

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

This looks ok to me, but maybe @jyellick can take a quick look as well.

Copy link
Contributor

@jyellick jyellick left a comment

Choose a reason for hiding this comment

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

Looks good to me -- is this last commit already forward ported to master?

@jyellick jyellick merged commit 5a37306 into hyperledger:release-2.2 Oct 14, 2020
@wlahti
Copy link
Contributor Author

wlahti commented Oct 14, 2020

Looks good to me -- is this last commit already forward ported to master?

Yes, it was merged yesterday. Thanks, Gari and Jason!

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.

None yet

4 participants