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
rgw: use insecure TLS for bucket health check #8712
Conversation
I think this change makes sense in the context, and I think it makes sense that we should be able to verify the health in cases where the cert understandably doesn't have the endpoint we need for the health check. I don't know how to evaluate this change in terms of possible security concerns, however. Perhaps @yuvalif might have more ability to comment on that aspect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am going to request changes for now. I messaged Yuval to see if he would mind checking out the security concerns of this change. If the security aspect sounds okay to him, I think we can merge this.
Backport rook#8712 And also apply the patch on user creation
Backport rook#8712 And also apply the patch on user creation
Backport rook#8712 And also apply the patch on user creation
I wonder if we could modify the TLS integration test to catch this case. It seems like maybe we could just remove the RGW service from the certificate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
0188dc1
to
33d5d28
Compare
@BlaineEXE @thotz I've fixed the TLS behavior for insecure and added unit tests. PTAL. |
88c9080
to
8e6fb5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TestCephObjectSuite/TestWithBrokenTLS/verify_object_store_status
test is failing consistently
This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/latest/development-flow.html#updating-your-fork |
5b5e5e7
to
abb7b14
Compare
We have seen cases where the signed certificate used for the RGW does not contain the internal DNS endpoint, resulting in the health check to fail since the certificate is not valid for this domain. People consuming the gateways by external clients and for specific domains do not necessarily have the internal DNS configured in the certificate. So let's be a bit more flexible and simply ensure a connectivity check and bypass the certificate validation. Also, this is fixing the tls code in newS3Agent and adds unit tests. Closes: rook#8663 Signed-off-by: Sébastien Han <seb@redhat.com>
rgw: use insecure TLS for bucket health check (backport #8712)
rgw: use insecure TLS for bucket health check rook#8712 only solved bucket health check
rgw: use insecure TLS for bucket health check rook#8712 only solved bucket health check
Description of your changes:
We have seen cases where the signed certificate used for the RGW does not
contain the internal DNS endpoint, resulting in the health check to fail
since the certificate is not valid for this domain.
People consuming the gateways by external clients and for specific
domains do not necessarily have the internal DNS configured in the
certificate.
So let's be a bit more flexible and simply ensure a connectivity check
and bypass the certificate validation.
Closes: #8663
Signed-off-by: Sébastien Han seb@redhat.com
Which issue is resolved by this Pull Request:
Resolves #
Checklist:
make codegen
) has been run to update object specifications, if necessary.