Skip to content
This repository has been archived by the owner on Jan 12, 2022. It is now read-only.

Fix option to disable ssl validation in devserver urlfetch #124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jchorl
Copy link

@jchorl jchorl commented Mar 12, 2018

The devserver urlfetch stub does not currently honour the
AllowInvalidServerCertificate option. This likely came from a breaking
change to httplib
(https://docs.python.org/2/library/httplib.html#httplib.HTTPSConnection).
Specifically, "This class now performs all the necessary certificate and
hostname checks by default. To revert to the previous, unverified,
behavior ssl._create_unverified_context() can be passed to the context
parameter."

This PR passes the unverified context when the caller specifies to allow
invalid certs.

(I'm not sure if this is the right place for this PR, but this is still broken in the latest devserver in the latest gcloud docker image and I couldn't find the source code for that.)

The devserver urlfetch stub does not currently honour the
AllowInvalidServerCertificate option. This likely came from a breaking
change to httplib
(https://docs.python.org/2/library/httplib.html#httplib.HTTPSConnection).
Specifically, "This class now performs all the necessary certificate and
hostname checks by default. To revert to the previous, unverified,
behavior ssl._create_unverified_context() can be passed to the context
parameter."

This PR passes the unverified context when the caller specifies to allow
invalid certs.
@duggelz
Copy link
Contributor

duggelz commented Mar 12, 2018

@rahulrv1980 Can you see if this makes sense to apply to the internal dev_appserver? Or have Kai look at this?

@jchorl
Copy link
Author

jchorl commented Mar 12, 2018

Thanks for following up! The real reason I hit this was looking to pin a specific server CA cert for a urlfetch request. I looked at the protobuf and noticed that isn't really possible (because there is just a field to specify whether or not to validate tls), but this is possible in the standard go libraries. If you have any suggestions on validating the server cert myself on AE, that would be splendid.

@cgwy
Copy link

cgwy commented Mar 13, 2018

Hi Josh, fetch methods in appengine.api.urlfetch already have the "validate_certificate" option (doc), For your use case, would disabling validate_certificate solve your problem?

@jchorl
Copy link
Author

jchorl commented Mar 13, 2018

Unfortunately, it does not. I would like to provide a specific set of certs that would be valid (i.e. cert pinning), which would not be trusted by a standard system but that I want urlfetch to trust (and only trust that set of certs). I'm basically looking to do this. From looking at the protobuf interface for urlfetch, it seems that this functionality isn't possible at all on appengine.

(but this is a side note, the PR actually fixes broken functionality in dev_server!)

@cgwy
Copy link

cgwy commented Mar 19, 2018

Cool. I've made corresponding code change to the mainline of urlfetch_stub. It will take a feel weeks to release. Fortunately you've already self-unblocked.

Thank you for contributing on this issue!

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

Successfully merging this pull request may close these issues.

4 participants