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

No verification of server certificates #13

Closed
ericwb opened this issue Feb 7, 2014 · 12 comments
Closed

No verification of server certificates #13

ericwb opened this issue Feb 7, 2014 · 12 comments

Comments

@ericwb
Copy link

ericwb commented Feb 7, 2014

Pyvmomi uses httplib and urllib which do no do any certificate verification. But most products do prefer to have some control of how host certificates are verified in order to avoid man-in-the-middle attacks.

Probably the easiest way to get this supported properly is to switch from httplib to python requests - http://docs.python-requests.org/en/latest/

@joshk0 joshk0 self-assigned this Feb 7, 2014
@joshk0
Copy link
Contributor

joshk0 commented Feb 7, 2014

I would love that. I'm actually working really closely with Requests in my current position, so I'll see what I can do.

@pcn
Copy link

pcn commented Mar 6, 2014

👍 requests is vastly more useable, and by useable I mean it's harder to make mistakes and easier to get up and running.

@hartsock
Copy link
Member

hartsock commented Jun 4, 2014

How is this going? I'm willing to slate it for the next release if it's close.

@hartsock
Copy link
Member

I noticed this code looks like an attempt to add cert verification but it was commented out for some reason.

michaelrice pushed a commit to michaelrice/pyvmomi that referenced this issue Jun 19, 2014
@hartsock hartsock assigned hartsock and unassigned joshk0 Jun 24, 2014
hartsock pushed a commit to hartsock/pyvmomi that referenced this issue Jun 26, 2014
hartsock pushed a commit to hartsock/pyvmomi that referenced this issue Jul 10, 2014
migrate from urllib2 to requests in the connect.py module. starting to
fix issue vmware#13
hartsock pushed a commit to hartsock/pyvmomi that referenced this issue Jul 10, 2014
migrate from urllib2 to requests in the connect.py module. starting to
fix issue vmware#13
hartsock pushed a commit to hartsock/pyvmomi that referenced this issue Jul 24, 2014
migrate from urllib2 to requests in the connect.py module. starting to
fix issue vmware#13

partial: vmware#55

see also: vmware#66
@hartsock hartsock removed their assignment Jul 28, 2014
@hartsock hartsock self-assigned this Jul 31, 2014
@hartsock hartsock modified the milestones: pyVmomi 5.5.0-2014.1, pyVmomi 5.5.0-2014.2 Aug 13, 2014
@hartsock
Copy link
Member

I'm moving this to the next release. We'll accomplish this by moving the library completely onto requests ... I'll want a longer review cycle to evaluate that kind of change.

@hartsock hartsock assigned hartsock and unassigned hartsock Aug 18, 2014
@hartsock hartsock modified the milestones: pyVmomi 5.5.0-2014.2, pyVmomi 2015.1 Dec 22, 2014
@jantman
Copy link

jantman commented May 21, 2015

Just as a comment, if certificate verification is added, please make sure there's an option to disable it.

@mpontillo
Copy link

Actually, I would argue that (in order to maintain compatibility with previous versions) verification should be disabled by default. (not because I think not verifying certificates is a good idea in general, but because it would break API compatibility with previous revisions... and I expect that many people would not bother to configure trusted certificates for this purpose, since pyvmomi will usually be run on what amounts to a relatively trusted network.)

@hartsock
Copy link
Member

I'll try and do a pull request for this shortly. My instinct is to add option flags to go with default = disabled through out the API. But, do something clever so that when in a "production" class environment we trip something and fall into enabled. Maybe a magic local environment variable for development?

We can discuss it in the pull request.

@pcn
Copy link

pcn commented May 21, 2015

I really think that a default option is the right way to go - eliminating surprise. Sites/individuals can wrap that and make their own determination as to whether they want a prod/dev split in how certs are treated. In most ops situations your sysadmins will be much happier if dev/stage/qa/test/ci/production all have the same behavior and the same flags enabled.

@hartsock
Copy link
Member

@pcn okay, we'll go with easy first.

@hartsock
Copy link
Member

hartsock commented Jun 2, 2015

Note: see #252 and #212 which are related to this issue.

@hartsock hartsock assigned tianhao64 and unassigned hartsock Dec 29, 2015
@tianhao64
Copy link
Contributor

The httplib has already been replaced by request. Thx @michaelrice .Since change 92c1de5, ssoContext can be passed to Connect and SmartConnect to disable the certificate verification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants