Issue 10580028: Experimental support for SSL in platform apps sockets.

Issue 10580028: Experimental support for SSL in platform apps sockets. (Closed)

Created:
8 years, 6 months ago by zarko1

Modified:
5 years, 7 months ago

Reviewers:
Ryan Sleevi

CC:
chromium-reviews, Aaron Boodman, mihaip-chromium-reviews_chromium.org

Base URL:
http://git.chromium.org/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

Experimental support for SSL in platform apps sockets.

Patch Set 3 : Rebase #

Created: 8 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+449 lines, -20 lines)			Patch
M	chrome/browser/extensions/api/socket/socket.h	View		2 chunks	+4 lines, -0 lines	0 comments	Download
M	chrome/browser/extensions/api/socket/socket.cc	View		2 chunks	+10 lines, -0 lines	0 comments	Download
M	chrome/browser/extensions/api/socket/socket_api.h	View	1 2	2 chunks	+20 lines, -1 line	0 comments	Download
M	chrome/browser/extensions/api/socket/socket_api.cc	View	1 2	5 chunks	+32 lines, -1 line	0 comments	Download
A +	chrome/browser/extensions/api/socket/ssl_socket.h	View	1	3 chunks	+36 lines, -16 lines	0 comments	Download
A	chrome/browser/extensions/api/socket/ssl_socket.cc	View	1	1 chunk	+325 lines, -0 lines	0 comments	Download
M	chrome/chrome_browser_extensions.gypi	View	1 2	1 chunk	+2 lines, -0 lines	0 comments	Download
M	chrome/common/extensions/api/experimental_socket.idl	View	1	5 chunks	+20 lines, -2 lines	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

Ryan Sleevi

drive by comments: https://chromiumcodereview.appspot.com/10580028/diff/2001/chrome/browser/extensions/api/socket/ssl_socket.cc File chrome/browser/extensions/api/socket/ssl_socket.cc (right): https://chromiumcodereview.appspot.com/10580028/diff/2001/chrome/browser/extensions/api/socket/ssl_socket.cc#newcode149 chrome/browser/extensions/api/socket/ssl_socket.cc:149: net::SSLConfig ssl_config; This is not a ...

8 years, 5 months ago (2012-07-09 20:55:45 UTC) #1

zarko1

8 years, 5 months ago (2012-07-11 17:41:40 UTC) #2

Thanks for the comments!

From the discussion on http://code.google.com/p/chromium/issues/detail?id=132896
and elsewhere, it looks like this oversimplifies the interface a little too much
to be useful. As an experiment it's been handy for doing some local testing, so
I'll continue rebasing, but I agree that more thought should be put into the
feature before it's made available.

On 2012/07/09 20:55:45, Ryan Sleevi wrote:
> drive by comments:
> 
>
https://chromiumcodereview.appspot.com/10580028/diff/2001/chrome/browser/exte...
> File chrome/browser/extensions/api/socket/ssl_socket.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10580028/diff/2001/chrome/browser/exte...
> chrome/browser/extensions/api/socket/ssl_socket.cc:149: net::SSLConfig
> ssl_config;
> This is not a good pattern. You should use the Profile's SSLConfigService.
There
> are a number of compatibility issues at the network stack due to
> incorrect/misconfigured servers. For users to be able to work around these
> servers (false start intolerant, rev checking required, TLS 1.1 intolerant),
we
> expose enterprise & command-line options. Extensions should respect these.
> 
>
https://chromiumcodereview.appspot.com/10580028/diff/2001/chrome/browser/exte...
> File chrome/browser/extensions/api/socket/ssl_socket.h (right):
> 
>
https://chromiumcodereview.appspot.com/10580028/diff/2001/chrome/browser/exte...
> chrome/browser/extensions/api/socket/ssl_socket.h:77:
> scoped_ptr<net::CertVerifier> cert_verifier_;
> drive-by: The CertVerifier should be passed in as a creation parameter, not
> created on demand.
> 
> This is particularly important for performance, so that existing verifications
> (which may involve fetching slow resources such as AIA, CRLs, or OCSP) can be
> cached between different sockets. Arguably, it should be the same CertVerifier
> associated with the profile's URLRequestContext bits.

Expand Messages | Collapse Messages