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

Support fetching resources over HTTPS #423

Closed
GoogleCodeExporter opened this Issue Apr 6, 2015 · 23 comments

Comments

Projects
None yet
1 participant
@GoogleCodeExporter
Copy link

GoogleCodeExporter commented Apr 6, 2015

I've attached a patch against revision 1533 that supports fetching resources 
over HTTPS.  In particular, this patch:

1.) Updates dependencies to use serf 1.0.3.
2.) Adds a dependency on OpenSSL in Chromium's OpenSSL GIT repository.
3.) Adds HTTPS support to the URL fetcher.
4.) Adds minimal support for SSL certificate error detection (which could be 
enhanced for customization in a future patch).  Right now, self-signed 
certificate and unknown certificate authority errors are allowed, while expired 
certificate, certificate not active, and unknown errors are not allowed.
5.) Refines the logic in the rewrite driver with respect to fetcher HTTPS 
support.
6.) Changes the domain lawyer to allow origin domains to be mapped to/from 
http/https.
7.) Updates Google's customizations of the serf library.
8.) Adds some source files in serf that are required for SSL support.
9.) Updates HTTPS tests.

If this patch gets committed, then I'll do work on customizing which SSL errors 
are acceptable as well.

Thanks.

Original issue reported on code.google.com by surfacep...@gmail.com on 21 Apr 2012 at 12:17

Attachments:

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Thanks very much for the patch!  We will take a look.

Original comment by jmara...@google.com on 21 Apr 2012 at 12:49

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 31 May 2012 at 7:34

  • Changed state: Accepted
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 5 Jun 2012 at 8:15

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

I have patched in this change and I'm now testing/tweaking a little.

Original comment by jmara...@google.com on 27 Aug 2012 at 12:40

  • Changed state: Started
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Note that in the trunk, we are now upgraded to Serf 1.1.0.  We need more 
testing prior to checking in the direct HTTPS fetch support.

I'm curious if others are interested in direct HTTPS fetches from 
mod_pagespeed, or whether the existing support based on 
origin-mapping/LoadFromFile is sufficient.

Original comment by jmara...@google.com on 5 Sep 2012 at 1:32

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

We are waiting for this patch, so that we can start using mod_pagespeed. We 
have a Liferay production and development setup with lots of updates on files.

Original comment by k...@martinsen.fi on 5 Sep 2012 at 5:17

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

We are also waiting on this patch, having been highly interested in direct 
HTTPS fetch support from the start. 

Original comment by dan...@djmillers.com on 9 Sep 2012 at 9:38

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

The original patch in this Issue does appear to work, if you are willing to 
build from patched source.  But as this is HTTPS, we need to do a bit more 
testing before we incorporate it into the trunk.  Note that the original patch 
upgrades the Serf version used by mod_pagespeed from 0.8 to 1.0.3, but that 
part is not necessary anymore (and will probably not work) because our trunk is 
now using Serf 1.1.0.

I'm curious: what are the reasons why fetching via HTTP origin-map or 
loading-from-files is not sufficient.  Is this because you are fetching 
origin-resources over an untrusted network?

Original comment by jmara...@google.com on 10 Sep 2012 at 2:16

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

That's exactly it. Unfortunately, for PCI compliance, SSL must be enabled 
end-to-end, even if the content lies on the same LAN. We utilize pagespeed as 
part of a reverse proxy system, which means we must keep traffic SSL encrypted 
as it flows through the server. On sites with changing dynamic content, it 
isn't as practical to store a copy of the static content locally, which means 
it must be retrieved via SSL...we've been very impressed with the optimization 
provided by pagespeed, but can't risk it causing the failure of a PCI 
compliance scan on our several sites. 

Thanks for the patch! I'll see if I can get it to build with the serf changes. 

Original comment by dan...@djmillers.com on 10 Sep 2012 at 2:55

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

The patch works successfully on the latest beta branch. After some tweaking, I 
believe I got it to work with the latest trunk as well, but the build fails due 
to (I believe) changes between serf versions. 

out/Debug/obj.target/net/instaweb/../../instaweb_apr/net/instaweb/apache/serf_ur
l_async_fetcher.o: In function 
`net_instaweb::SerfFetch::ConnectionSetup(apr_socket_t*, serf_bucket_t**, 
serf_bucket_t**, void*, apr_pool_t*)':
/root/mod_pagespeed/src/net/instaweb/apache/serf_url_async_fetcher.cc:240: 
undefined reference to `serf_bucket_ssl_decrypt_create'
/root/mod_pagespeed/src/net/instaweb/apache/serf_url_async_fetcher.cc:242: 
undefined reference to `serf_bucket_ssl_decrypt_context_get'
/root/mod_pagespeed/src/net/instaweb/apache/serf_url_async_fetcher.cc:246: 
undefined reference to `serf_ssl_server_cert_callback_set'
/root/mod_pagespeed/src/net/instaweb/apache/serf_url_async_fetcher.cc:254: 
undefined reference to `serf_ssl_set_hostname'
/root/mod_pagespeed/src/net/instaweb/apache/serf_url_async_fetcher.cc:257: 
undefined reference to `serf_bucket_ssl_encrypt_create'
collect2: ld returned 1 exit status
make: *** [out/Debug/mod_pagespeed_test] Error 1

Original comment by dan...@djmillers.com on 11 Sep 2012 at 7:06

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

To build this on trunk, you need to modify to src/third_party/serf/serf.gyp.  
The comments in that file show what you need to do.  Look for "openssl" and 
"ssl_buckets.c".

You also need to look for "openssl" in src/DEPS and uncomment that.

Original comment by jmara...@google.com on 11 Sep 2012 at 2:57

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Hi -- updating this bug.

I am going to try to clean this out by checking in the support-code for HTTPS 
but leave it ifdef'd out.  You will need to do a custom build of mod_pagespeed 
to compile in https support against openssl.

Original comment by jmara...@google.com on 17 Dec 2012 at 6:41

  • Changed state: Accepted
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

With http://code.google.com/p/modpagespeed/source/detail?r=2337 I have put all 
the code into our system to support HTTPS fetching, but it is not compiled in 
by default.  You must build from source after making a few small patches.

I have attached a small diff that can be patched in to turn on the feature.

Original comment by jmara...@google.com on 22 Dec 2012 at 12:00

  • Added labels: Milestone-v25, release-note

Attachments:

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Original comment by jmara...@google.com on 22 Dec 2012 at 12:01

  • Changed state: Fixed
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Josh, should this really be considered fixed until it's compiled in by default?

Original comment by morlov...@google.com on 14 Feb 2013 at 4:34

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

re-opening until it's compiled by default.

Original comment by jmara...@google.com on 17 Mar 2013 at 2:13

  • Changed state: Accepted
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

HTTPS support will now be built into the binaries and in the default setup for 
source builds, though you still have to turn it on with a switch, which will be 
doc'd in v28.

Original comment by jmara...@google.com on 11 May 2013 at 1:36

  • Changed state: Fixed
  • Added labels: Milestone-v28
  • Removed labels: Milestone-v25
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

This was fixed in https://code.google.com/p/modpagespeed/source/detail?r=2999

Original comment by jmara...@google.com on 11 May 2013 at 1:45

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

Some HTTPS tests were failing on some platforms, so I commented them out.  If 
the problem can be resolve then we will can close this bug, otherwise we may 
have more rollbacks needed as the problematic HTTPS code is still in the system 
(though turned off by default in pagespeed.conf).

Original comment by jmara...@google.com on 14 May 2013 at 2:10

  • Changed state: Started
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

The known problems have been resolved and this is slated for release with the 
next beta.

Original comment by jmara...@google.com on 12 Jun 2013 at 1:26

  • Changed state: Fixed
  • Added labels: Milestone-v29
  • Removed labels: Milestone-v28
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

We had to pull support (doc mainly) for this feature from 1.6 due to a 
link-clash issue, but this is now resolved and will be in 1.7.

Original comment by jmara...@google.com on 2 Oct 2013 at 2:51

  • Added labels: Milestone-v30
  • Removed labels: Milestone-v29
@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

What is the release schedule for 1.7? And is there a general roadmap for the 
project?

Original comment by k...@martinsen.fi on 2 Oct 2013 at 4:11

@GoogleCodeExporter

This comment has been minimized.

Copy link

GoogleCodeExporter commented Apr 6, 2015

1.7: expect it soon.  We cannot give you a date yet but we are close.

Roadmap: no such thing is published but I can tell you that mobile is important 
to us.  In terms of what bugs get fixed when, we use activity on these issues 
as a signal of what's important to our users so vote early and vote often.

Original comment by jmara...@google.com on 2 Oct 2013 at 4:30

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