search

GoogleCloudPlatform / python-compat-runtime

DEPRECATED: gcr.io/google_appengine/python-compat-multicore
Apache License 2.0
22 stars 31 forks source link

Fix option to disable ssl validation in devserver urlfetch #124

Open jchorl opened 6 years ago

jchorl commented 6 years ago

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.)

duggelz commented 6 years ago

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

jchorl commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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!