-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Use TLS 1.2 to access healthz API #28895
Conversation
2da2ab1
to
0d34e6c
Compare
@@ -1211,7 +1211,13 @@ function wait-master() { | |||
echo " up." | |||
echo | |||
|
|||
until $(curl --insecure --user ${KUBE_USER}:${KUBE_PASSWORD} --max-time 5 \ | |||
if [[ $(curl -V | awk '{print $2}' | head -1) > 7.34.0 ]]; then | |||
TLS_OPTS="--insecure --tlsv1.2" |
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.
It seems like none of these are useful because we are always using --insecure. What extra security does this provide?
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.
Mike,
--insecure
tells libcurl to not verify the peer (https://curl.haxx.se/docs/sslcerts.html). If you see the man page, it has additional details
(SSL) This option explicitly allows curl to perform "insecure" SSL connections and transfers. All SSL connections are attempted to be made secure by using the CA certificate bundle installed by default. This makes all connections considered "insecure" fail unless -k, --insecure is used.
fyi. bug reporter has confirmed that adding this option makes the scenario work.
the bug reporter has confirmed that adding --tlsv1.2 works as well.
Thanks,
Dims
In the following PR, TLS was set to 1.2 as the minimum because TLS1.0 and TLS1.1 are vulnerable: kubernetes#26169 However the scripts that used curl were not updated to match the TLS version. Since --tlsv1.2 was introduced in curl 7.34.0, we should check the version before using the option. Fixes kubernetes#28888
GCE e2e build/test passed for commit 1eb2348. |
@mikedanese Does not look like there's much interest in this PR. Closing for now |
In the following PR, TLS was set to 1.2 as the minimum
because TLS1.0 and TLS1.1 are vulnerable:
#26169
However the scripts that used curl were not updated to match
the TLS version.
Fixes #28888