ClusterLabs / fence-agents

Fence agents
101 stars 155 forks source link

Make optional the SSL verification for Docker #581

Closed marjus45 closed 1 month ago

marjus45 commented 1 month ago

I understand that the fence_docker agent has set the default value of --ssl to true, and there is no way to set it to false via the command line. Is this true?

root@playground-worker:/# fence_docker -a 172.18.0.3 -n playground-worker2
2024-06-03 12:18:15,411 ERROR: Failed. If --ssl option is used, You have to also specify: --tlscert, --tlskey and --tlscacert

2024-06-03 12:18:15,411 ERROR: Please use '-h' for usage

and my attempt to set it to false:

root@playground-worker:/# fence_docker -a 172.18.0.3 -n playground-worker2 --ssl=false
ERROR:root:Parse error: option --ssl must not have an argument

ERROR:root:Please use '-h' for usage

If so, would you be open to set this flag to false by default to make optional the TLS verification? Another option is to configure the flag to accept an argument, but this might change the user interface.

oalbrigt commented 1 month ago

You have to use --ssl-insecure to skip SSL verification.

marjus45 commented 1 month ago

Thanks for the answer @oalbrigt , but that doesn't work for me either, because I want to run the command over http not https. If you set the --ssl flag to true, then the code assumes that this is an HTTPS request, which is too restrictive.

Why do you need this parameter to be set to true?

oalbrigt commented 1 month ago

We need it to be true to avoid customers setting up insecure solutions and not being informed about it.

Look for disable_ssl in fence_zvmip for an example for how you can disable it, or you can update the if/else on this line https://github.com/ClusterLabs/fence-agents/pull/583/files#diff-323a7bda23d47d2a6cfc14837ada72209ffe9d2b8a76a26f6da9424e22d8191bR46 if sockets always use http.

marjus45 commented 1 month ago

I am okay in adding the --disable-ssl flag as well. The important part is to allow the user to run over HTTP if they want, which now I understand is not supported.

I think it's a bit counter-intuitive though to have a command line argument which doesn't do anything. If --ssl is always set to true for protecting the customers this is fine, but then only the --disable-ssl should exist.

oalbrigt commented 1 month ago

That would be ideal, but it's part of the fencing library, and some agents that might still be used but arent provided in commercial distros still have ssl disabled by default, so they need it.

And you can also set ssl=0 when you set it up in Pacemaker via pcs or crmsh, so it's not working when you test it manually.