Skip to content

Commit

Permalink
Default to SSL to comply with pve and fix typo
Browse files Browse the repository at this point in the history 
SSL encrypted communication, so we set it as
default. As pve generates a self signed certificate we set
insecure ssl as default option. A unencrypted connection is rejected
by the PVE proxy so it isn't needed as an option.
Also a typo (opts instead of options) was corrected.
  • Loading branch information
@ThomasLamprecht
ThomasLamprecht committed Aug 7, 2015
1 parent 3eec096 commit c0a6b25
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions fence/agents/pve/fence_pve.py 100644 → 100755
View file
Expand Up @@ -106,11 +106,10 @@ def send_cmd(options, cmd, post=None):
conn.setopt(pycurl.POSTFIELDS, urllib.urlencode(post))
conn.setopt(pycurl.WRITEFUNCTION, output_buffer.write)
conn.setopt(pycurl.TIMEOUT, int(options["--shell-timeout"]))
if opt.has_key("--ssl") or opt.has_key("--ssl-secure"):
if options.has_key("--ssl") or options.has_key("--ssl-secure"):
conn.setopt(pycurl.SSL_VERIFYPEER, 1)
conn.setopt(pycurl.SSL_VERIFYHOST, 2)

if opt.has_key("--ssl-insecure"):
else:
conn.setopt(pycurl.SSL_VERIFYPEER, 0)
conn.setopt(pycurl.SSL_VERIFYHOST, 0)

Expand Down

3 comments on commit c0a6b25

@howdoicomputer
Copy link

Choose a reason for hiding this comment

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

Also, as it stands, --ssl, --ssl-secure, and --ssl-insecure are not registered keys in the options dictionary so every one of those options would fail.

Something like this would be needed.

        all_opt["ssl-insecure"] = {
                "getopt": "q",
                "longopt": "ssl-insecure",
                "help": "-q --ssl-insecure",
                "required": "0",
                "shortdesc": "Don't verify SSL connections"
        }

@ThomasLamprecht
Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, that with the options clearly my fault, I missed that completely and relied uppon the existence of this two as they were already in the code, my bad sorry!

I now also think it's better to default to SSL verification, as even if self signed certificates are default on Proxmox VE you should explicitly state that it's okay to not verify it.

Do you already have a patch prepared? Else I would make the "dirty work", I mean it's partly my fault.

Thanks for noticing this!

@ThomasLamprecht
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh follow up, maybe I should drink a coffee before answering such stuff.

Naturally ssl, ssl-secure and ssl-insecure exist! They only are not in this file as they get inherited from the base fencing class. See

fence-agents/fence/agents/lib/fencing.py.py

Line 183 in 7e65180

"ssl" : {
and following lines

The fence agents test system in fact runs regression test on all agents so that an error like a wrong option would fail the build, I did test this change it was only a bit to long ago to remember :)

So nothing needs to be changed, at least regarding this.

Please sign in to comment.