diff options
author | boliu <boliu@chromium.org> | 2016-03-02 16:27:47 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-03 00:29:22 +0000 |
commit | 7fa22e7520009140156013bc87f39a39fe8bd2ca (patch) | |
tree | 11949b4663cd5fb32a2e6b56440ed3bb7eec3adb /blimp | |
parent | 432564c6ecff52ab735bdc35adcf537d89b13589 (diff) | |
download | chromium_src-7fa22e7520009140156013bc87f39a39fe8bd2ca.zip chromium_src-7fa22e7520009140156013bc87f39a39fe8bd2ca.tar.gz chromium_src-7fa22e7520009140156013bc87f39a39fe8bd2ca.tar.bz2 |
Revert of Blimp: add support for SSL connections. (patchset #20 id:380001 of https://codereview.chromium.org/1696563002/ )
Reason for revert:
Broke blimp_unittests_apk building on android:
FAILED: python "/b/build/slave/android_build/build/src/build/toolchain/gcc_solink_wrapper.py" --readelf="../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-readelf" --nm="../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-nm" --strip=../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-strip --sofile="./lib.unstripped/libblimp_client_android.so" --tocfile="./libblimp_client_android.so.TOC" --output="./libblimp_client_android.so" -- ../../third_party/android_tools/ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-g++ -shared -Wl,--fatal-warnings -fPIC -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro -Wl,-z,defs -fuse-ld=gold -Wl,--no-undefined -Wl,--exclude-libs=libgcc.a -Wl,--exclude-libs=libc++_static.a -Wl,--exclude-libs=libvpx_assembly_arm.a -Wl,--icf=all -Wl,--warn-shared-textrel -Wl,-O1 -Wl,--as-needed -nostdlib -Wl,--warn-shared-textrel --sysroot=../../third_party/android_tools/ndk/platforms/android-16/arch-arm -Wl,--version-script=/b/build/slave/android_build/build/src/build/android/android_no_jni_exports.lst -L../../third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a -o "./lib.unstripped/libblimp_client_android.so" -Wl,-soname="libblimp_client_android.so" @"./libblimp_client_android.so.rsp"
../../content/browser/android/in_process/synchronous_compositor_factory_impl.cc:125: error: undefined reference to 'content::SynchronousCompositorExternalBeginFrameSource::SynchronousCompositorExternalBeginFrameSource(int, content::SynchronousCompositorRegistry*)'
../../content/browser/android/in_process/synchronous_compositor_factory_impl.cc:113: error: undefined reference to 'content::SynchronousCompositorOutputSurface::SynchronousCompositorOutputSurface(scoped_refptr<cc::ContextProvider> const&, scoped_refptr<cc::ContextProvider> const&, int, content::SynchronousCompositorRegistry*, scoped_refptr<content::FrameSwapMessageQueue>)'
../../base/memory/ref_counted.h:193: error: undefined reference to 'content::FrameSwapMessageQueue::~FrameSwapMessageQueue()'
../../content/browser/android/in_process/synchronous_compositor_factory_impl.cc:93: error: undefined reference to 'content::SynchronousCompositorFactory::SetInstance(content::SynchronousCompositorFactory*)'
../../content/browser/android/in_process/synchronous_compositor_factory_impl.cc:197: error: undefined reference to 'content::StreamTextureFactorySynchronousImpl::Create(base::Callback<scoped_refptr<content::StreamTextureFactorySynchronousImpl::ContextProvider> ()> const&)'
../../content/browser/android/in_process/synchronous_compositor_factory_impl.cc:272: error: undefined reference to 'content::RenderThreadImpl::SetStreamTextureFactory(scoped_refptr<content::StreamTextureFactory>)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:102: error: undefined reference to 'content::SynchronousCompositorOutputSurface::SetTreeActivationCallback(base::Callback<void ()> const&)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:125: error: undefined reference to 'content::SynchronousCompositorOutputSurface::SetSyncClient(content::SynchronousCompositorOutputSurfaceClient*)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:126: error: undefined reference to 'content::SynchronousCompositorExternalBeginFrameSource::SetClient(content::SynchronousCompositorExternalBeginFrameSourceClient*)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:135: error: undefined reference to 'content::SynchronousCompositorOutputSurface::SetTreeActivationCallback(base::Callback<void ()> const&)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:141: error: undefined reference to 'content::SynchronousCompositorExternalBeginFrameSource::SetClient(content::SynchronousCompositorExternalBeginFrameSourceClient*)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:142: error: undefined reference to 'content::SynchronousCompositorOutputSurface::SetSyncClient(content::SynchronousCompositorOutputSurfaceClient*)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:178: error: undefined reference to 'content::SynchronousCompositorOutputSurface::ReturnResources(cc::CompositorFrameAck const&)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:215: error: undefined reference to 'content::SynchronousCompositorOutputSurface::SetMemoryPolicy(unsigned int)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:275: error: undefined reference to 'content::SynchronousCompositorExternalBeginFrameSource::BeginFrame(cc::BeginFrameArgs const&)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:320: error: undefined reference to 'content::SynchronousCompositorOutputSurface::GetMessagesToDeliver(std::__1::vector<scoped_ptr<IPC::Message, std::__1::default_delete<IPC::Message> >, std::__1::allocator<scoped_ptr<IPC::Message, std::__1::default_delete<IPC::Message> > > >*)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:167: error: undefined reference to 'content::SynchronousCompositorOutputSurface::DemandDrawHw(gfx::Size const&, gfx::Transform const&, gfx::Rect const&, gfx::Rect const&, gfx::Rect const&, gfx::Transform const&)'
../../content/browser/android/in_process/synchronous_compositor_impl.cc:187: error: undefined reference to 'content::SynchronousCompositorOutputSurface::DemandDrawSw(SkCanvas*)'
../../content/browser/android/in_process/synchronous_compositor_renderer_statics.cc:12: error: undefined reference to 'content::SynchronousCompositorProxy::SetSkCanvasForDraw(SkCanvas*)'
../../content/browser/android/synchronous_compositor_base.cc:32: error: undefined reference to 'content::InProcessGpuThread::InProcessGpuThread(content::InProcessChildThreadParams const&, gpu::GpuPreferences const&, gpu::SyncPointManager*)'
collect2: error: ld returned 1 exit status
Original issue's description:
> Blimp: add support for SSL connections.
>
> This CL allows the Blimp client to establish TLS-protected channels with the backend engine. The authenticity of the engine is validated by checking if its cert is an exact match of a certificate provided separately by the Assigner API.
>
> * Create new Blimp SSL transport class: SSLClientTransport.
> * Create custom CertValidator for checking an exact cert match against the SSL peer's cert
> * Integrate SSLClientTransport with BlimpClientSession.
> * Assignment: add certificate field.
> * AssignmentSource: add certificate file reading; PEM file parsing;
> X509 certificate parsing.
> * Created new DEPS entries as appropriate.
>
> R=wez@chromium.org
> CC=rsleevi@chromium.org
> BUG=585279,589202
>
> Committed: https://crrev.com/c80f5095f045ad1712f1f1075a44547a561f774a
> Cr-Commit-Position: refs/heads/master@{#378839}
TBR=wez@chromium.org,dtrainor@chromium.org,rsesek@chromium.org,rsleevi@chromium.org,kmarshall@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=585279,589202
Review URL: https://codereview.chromium.org/1757153002
Cr-Commit-Position: refs/heads/master@{#378892}
Diffstat (limited to 'blimp')
27 files changed, 283 insertions, 923 deletions
diff --git a/blimp/client/BUILD.gn b/blimp/client/BUILD.gn index 465adb7..d2ddc2b 100644 --- a/blimp/client/BUILD.gn +++ b/blimp/client/BUILD.gn @@ -27,7 +27,6 @@ source_set("blimp_client") { defines = [ "BLIMP_CLIENT_IMPLEMENTATION=1" ] public_deps = [ - "//components/safe_json", "//ui/events", ] @@ -50,6 +49,12 @@ source_set("blimp_client_unit_tests") { sources = [] + # TODO(dtrainor): Fix the test harness to allow this to run on Android. + # See crbug.com/588240. + if (is_linux) { + sources += [ "session/assignment_source_unittest.cc" ] + } + deps = [ ":blimp_client", "//base", @@ -58,19 +63,6 @@ source_set("blimp_client_unit_tests") { "//testing/gmock", "//testing/gtest", ] - - data = [] - - # TODO(dtrainor): Fix the test harness to allow this to run on Android. - # See crbug.com/588240. - if (is_linux) { - sources += [ "session/assignment_source_unittest.cc" ] - deps += [ - "//components/safe_json:test_support", - "//net:test_support", - ] - data += [ "session/test_selfsigned_cert.pem" ] - } } source_set("app_unit_tests") { @@ -350,7 +342,6 @@ if (is_android) { "//base", "//blimp/common/proto", "//blimp/net:blimp_net", - "//components/safe_json/android:safe_json_jni_headers", "//skia", "//ui/gfx/geometry", "//ui/gl", @@ -384,7 +375,6 @@ if (is_android) { ":blimp_java", ":blimp_java_resources", "//base:base_java", - "//components/safe_json/android:safe_json_java", "//net/android:net_java", google_play_services_resources, ] diff --git a/blimp/client/DEPS b/blimp/client/DEPS index e1fa29a..62e4870 100644 --- a/blimp/client/DEPS +++ b/blimp/client/DEPS @@ -2,8 +2,7 @@ include_rules = [ "+base", "+cc", "-cc/blink", - "-chrome", - "+components/safe_json", + "-chrome" "-content", "+gpu", "+jni", diff --git a/blimp/client/app/android/blimp_jni_registrar.cc b/blimp/client/app/android/blimp_jni_registrar.cc index 2f6ad4d..0ad0bcb 100644 --- a/blimp/client/app/android/blimp_jni_registrar.cc +++ b/blimp/client/app/android/blimp_jni_registrar.cc @@ -10,7 +10,6 @@ #include "blimp/client/app/android/blimp_view.h" #include "blimp/client/app/android/tab_control_feature_android.h" #include "blimp/client/app/android/toolbar.h" -#include "components/safe_json/android/component_jni_registrar.h" namespace blimp { namespace client { @@ -21,7 +20,6 @@ base::android::RegistrationMethod kBlimpRegistrationMethods[] = { {"Toolbar", Toolbar::RegisterJni}, {"BlimpView", BlimpView::RegisterJni}, {"BlimpClientSessionAndroid", BlimpClientSessionAndroid::RegisterJni}, - {"SafeJson", safe_json::android::RegisterSafeJsonJni}, {"TabControlFeatureAndroid", TabControlFeatureAndroid::RegisterJni}, }; diff --git a/blimp/client/app/blimp_client_switches.cc b/blimp/client/app/blimp_client_switches.cc index da7dcda..cfcf2a0 100644 --- a/blimp/client/app/blimp_client_switches.cc +++ b/blimp/client/app/blimp_client_switches.cc @@ -7,13 +7,11 @@ namespace blimp { namespace switches { -const char kEngineCertPath[] = "engine-cert-path"; - -const char kEngineIP[] = "engine-ip"; - -const char kEnginePort[] = "engine-port"; - -const char kEngineTransport[] = "engine-transport"; +// Specifies the blimplet scheme, IP-address and port to connect to, e.g.: +// --blimplet-host="tcp:127.0.0.1:25467". Valid schemes are "ssl", +// "tcp", and "quic". +// TODO(nyquist): Add support for DNS-lookup. See http://crbug.com/576857. +const char kBlimpletEndpoint[] = "blimplet-endpoint"; } // namespace switches } // namespace blimp diff --git a/blimp/client/app/blimp_client_switches.h b/blimp/client/app/blimp_client_switches.h index b06958f..19b91dd 100644 --- a/blimp/client/app/blimp_client_switches.h +++ b/blimp/client/app/blimp_client_switches.h @@ -10,25 +10,7 @@ namespace blimp { namespace switches { -// The path to the engine's PEM-encoded X509 certificate. -// If specified, SSL connected Engines must supply this certificate -// for the connection to be valid. -// e.g.: -// --engine-cert-path=/home/blimp/certs/cert.pem -extern const char kEngineCertPath[]; - -// Specifies the engine's IP address. Must be used in conjunction with -// --engine-port and --engine-transport. -extern const char kEngineIP[]; - -// Specifies the engine's listening port (1-65535). -// Must be used in conjunction with --engine-ip and --engine-transport. -extern const char kEnginePort[]; - -// Specifies the transport used to communicate with the engine. -// Can be "tcp" or "ssl". -// Must be used in conjunction with --engine-ip and --engine-port. -extern const char kEngineTransport[]; +extern const char kBlimpletEndpoint[]; } // namespace switches } // namespace blimp diff --git a/blimp/client/session/assignment_source.cc b/blimp/client/session/assignment_source.cc index bbd8d1a..242d783 100644 --- a/blimp/client/session/assignment_source.cc +++ b/blimp/client/session/assignment_source.cc @@ -7,22 +7,19 @@ #include "base/bind.h" #include "base/callback_helpers.h" #include "base/command_line.h" -#include "base/files/file_util.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" #include "base/location.h" -#include "base/memory/ref_counted.h" #include "base/numerics/safe_conversions.h" #include "base/strings/string_number_conversions.h" -#include "base/task_runner_util.h" #include "base/values.h" #include "blimp/client/app/blimp_client_switches.h" #include "blimp/common/protocol_version.h" -#include "components/safe_json/safe_json_parser.h" #include "net/base/ip_address.h" #include "net/base/ip_endpoint.h" #include "net/base/load_flags.h" #include "net/base/net_errors.h" +#include "net/base/url_util.h" #include "net/http/http_status_code.h" #include "net/proxy/proxy_config_service.h" #include "net/proxy/proxy_service.h" @@ -43,11 +40,55 @@ const char kProtocolVersionKey[] = "protocol_version"; const char kClientTokenKey[] = "clientToken"; const char kHostKey[] = "host"; const char kPortKey[] = "port"; +const char kCertificateFingerprintKey[] = "certificateFingerprint"; const char kCertificateKey[] = "certificate"; -// Possible arguments for the "--engine-transport" command line parameter. -const char kSSLTransportValue[] = "ssl"; -const char kTCPTransportValue[] = "tcp"; +// URL scheme constants for custom assignments. See the '--blimplet-endpoint' +// documentation in blimp_client_switches.cc for details. +const char kCustomSSLScheme[] = "ssl"; +const char kCustomTCPScheme[] = "tcp"; +const char kCustomQUICScheme[] = "quic"; + +Assignment GetCustomBlimpletAssignment() { + GURL url(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + switches::kBlimpletEndpoint)); + + std::string host; + int port; + if (url.is_empty() || !url.is_valid() || !url.has_scheme() || + !net::ParseHostAndPort(url.path(), &host, &port)) { + return Assignment(); + } + + net::IPAddress ip_address; + if (!ip_address.AssignFromIPLiteral(host)) { + CHECK(false) << "Invalid BlimpletAssignment host " << host; + } + + if (!base::IsValueInRangeForNumericType<uint16_t>(port)) { + CHECK(false) << "Invalid BlimpletAssignment port " << port; + } + + Assignment::TransportProtocol protocol = + Assignment::TransportProtocol::UNKNOWN; + if (url.has_scheme()) { + if (url.SchemeIs(kCustomSSLScheme)) { + protocol = Assignment::TransportProtocol::SSL; + } else if (url.SchemeIs(kCustomTCPScheme)) { + protocol = Assignment::TransportProtocol::TCP; + } else if (url.SchemeIs(kCustomQUICScheme)) { + protocol = Assignment::TransportProtocol::QUIC; + } else { + CHECK(false) << "Invalid BlimpletAssignment scheme " << url.scheme(); + } + } + + Assignment assignment; + assignment.transport_protocol = protocol; + assignment.ip_endpoint = net::IPEndPoint(ip_address, port); + assignment.client_token = kDummyClientToken; + return assignment; +} GURL GetBlimpAssignerURL() { // TODO(dtrainor): Add a way to specify another assigner. @@ -57,8 +98,8 @@ GURL GetBlimpAssignerURL() { class SimpleURLRequestContextGetter : public net::URLRequestContextGetter { public: SimpleURLRequestContextGetter( - scoped_refptr<base::SingleThreadTaskRunner> io_loop_task_runner) - : io_loop_task_runner_(std::move(io_loop_task_runner)), + const scoped_refptr<base::SingleThreadTaskRunner>& io_loop_task_runner) + : io_loop_task_runner_(io_loop_task_runner), proxy_config_service_(net::ProxyService::CreateSystemProxyConfigService( io_loop_task_runner_, io_loop_task_runner_)) {} @@ -95,142 +136,45 @@ class SimpleURLRequestContextGetter : public net::URLRequestContextGetter { DISALLOW_COPY_AND_ASSIGN(SimpleURLRequestContextGetter); }; -bool IsValidIpPortNumber(unsigned port) { - return port > 0 && port <= 65535; -} - -// Populates an Assignment using command-line parameters, if provided. -// Returns a null Assignment if no parameters were set. -// Must be called on a thread suitable for file IO. -Assignment GetAssignmentFromCommandLine() { - Assignment assignment; - assignment.client_token = kDummyClientToken; - - unsigned port_parsed = 0; - if (!base::StringToUint( - base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( - switches::kEnginePort), - &port_parsed) || - !IsValidIpPortNumber(port_parsed)) { - DLOG(FATAL) << "--engine-port must be a value between 1 and 65535."; - return Assignment(); - } - - net::IPAddress ip_address; - std::string ip_str = - base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( - switches::kEngineIP); - if (!ip_address.AssignFromIPLiteral(ip_str)) { - DLOG(FATAL) << "Invalid engine IP " << ip_str; - return Assignment(); - } - assignment.engine_endpoint = - net::IPEndPoint(ip_address, base::checked_cast<uint16_t>(port_parsed)); - - std::string transport_str = - base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( - switches::kEngineTransport); - if (transport_str == kSSLTransportValue) { - assignment.transport_protocol = Assignment::TransportProtocol::SSL; - } else if (transport_str == kTCPTransportValue) { - assignment.transport_protocol = Assignment::TransportProtocol::TCP; - } else { - DLOG(FATAL) << "Invalid engine transport " << transport_str; - return Assignment(); - } - - scoped_refptr<net::X509Certificate> cert; - if (assignment.transport_protocol == Assignment::TransportProtocol::SSL) { - base::FilePath cert_path = - base::CommandLine::ForCurrentProcess()->GetSwitchValuePath( - switches::kEngineCertPath); - if (cert_path.empty()) { - DLOG(FATAL) << "Missing required parameter --" - << switches::kEngineCertPath << "."; - return Assignment(); - } - std::string cert_str; - if (!base::ReadFileToString(cert_path, &cert_str)) { - DLOG(FATAL) << "Couldn't read from file: " - << cert_path.LossyDisplayName(); - return Assignment(); - } - net::CertificateList cert_list = - net::X509Certificate::CreateCertificateListFromBytes( - cert_str.data(), cert_str.size(), - net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE); - DLOG_IF(FATAL, (cert_list.size() != 1u)) - << "Only one cert is allowed in PEM cert list."; - assignment.cert = std::move(cert_list[0]); - } - - if (!assignment.IsValid()) { - DLOG(FATAL) << "Invalid command-line assignment."; - return Assignment(); - } - - return assignment; -} - } // namespace Assignment::Assignment() : transport_protocol(TransportProtocol::UNKNOWN) {} Assignment::~Assignment() {} -bool Assignment::IsValid() const { - if (engine_endpoint.address().empty() || engine_endpoint.port() == 0 || - transport_protocol == TransportProtocol::UNKNOWN) { - return false; - } - if (transport_protocol == TransportProtocol::SSL && !cert) { - return false; - } - return true; +bool Assignment::is_null() const { + return ip_endpoint.address().empty() || ip_endpoint.port() == 0 || + transport_protocol == TransportProtocol::UNKNOWN; } AssignmentSource::AssignmentSource( - const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner, - const scoped_refptr<base::SingleThreadTaskRunner>& file_task_runner) - : file_task_runner_(std::move(file_task_runner)), - url_request_context_( - new SimpleURLRequestContextGetter(network_task_runner)), - weak_factory_(this) {} + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner, + const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner) + : main_task_runner_(main_task_runner), + url_request_context_(new SimpleURLRequestContextGetter(io_task_runner)) {} AssignmentSource::~AssignmentSource() {} void AssignmentSource::GetAssignment(const std::string& client_auth_token, const AssignmentCallback& callback) { - DCHECK(callback_.is_null()); - callback_ = AssignmentCallback(callback); + DCHECK(main_task_runner_->BelongsToCurrentThread()); - if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kEngineIP)) { - base::PostTaskAndReplyWithResult( - file_task_runner_.get(), FROM_HERE, - base::Bind(&GetAssignmentFromCommandLine), - base::Bind(&AssignmentSource::OnGetAssignmentFromCommandLineDone, - weak_factory_.GetWeakPtr(), client_auth_token)); - } else { - QueryAssigner(client_auth_token); + // Cancel any outstanding callback. + if (!callback_.is_null()) { + base::ResetAndReturn(&callback_) + .Run(AssignmentSource::Result::RESULT_SERVER_INTERRUPTED, Assignment()); } -} + callback_ = AssignmentCallback(callback); -void AssignmentSource::OnGetAssignmentFromCommandLineDone( - const std::string& client_auth_token, - Assignment parsed_assignment) { - // If GetAssignmentFromCommandLine succeeded, then return its output. - if (parsed_assignment.IsValid()) { - base::ResetAndReturn(&callback_) - .Run(AssignmentSource::RESULT_OK, parsed_assignment); + Assignment assignment = GetCustomBlimpletAssignment(); + if (!assignment.is_null()) { + // Post the result so that the behavior of this function is consistent. + main_task_runner_->PostTask( + FROM_HERE, base::Bind(base::ResetAndReturn(&callback_), + AssignmentSource::Result::RESULT_OK, assignment)); return; } - // If no assignment was passed via the command line, then fall back on - // querying the Assigner service. - QueryAssigner(client_auth_token); -} - -void AssignmentSource::QueryAssigner(const std::string& client_auth_token) { // Call out to the network for a real assignment. Build the network request // to hit the assigner. url_fetcher_ = net::URLFetcher::Create(GetBlimpAssignerURL(), @@ -247,10 +191,12 @@ void AssignmentSource::QueryAssigner(const std::string& client_auth_token) { std::string json; base::JSONWriter::Write(dictionary, &json); url_fetcher_->SetUploadData("application/json", json); + url_fetcher_->Start(); } void AssignmentSource::OnURLFetchComplete(const net::URLFetcher* source) { + DCHECK(main_task_runner_->BelongsToCurrentThread()); DCHECK(!callback_.is_null()); DCHECK_EQ(url_fetcher_.get(), source); @@ -307,14 +253,14 @@ void AssignmentSource::ParseAssignerResponse() { return; } - safe_json::SafeJsonParser::Parse( - response, - base::Bind(&AssignmentSource::OnJsonParsed, weak_factory_.GetWeakPtr()), - base::Bind(&AssignmentSource::OnJsonParseError, - weak_factory_.GetWeakPtr())); -} + // Attempt to interpret the response as JSON and treat it as a dictionary. + scoped_ptr<base::Value> json = base::JSONReader::Read(response); + if (!json) { + base::ResetAndReturn(&callback_) + .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment()); + return; + } -void AssignmentSource::OnJsonParsed(scoped_ptr<base::Value> json) { const base::DictionaryValue* dict; if (!json->GetAsDictionary(&dict)) { base::ResetAndReturn(&callback_) @@ -326,10 +272,12 @@ void AssignmentSource::OnJsonParsed(scoped_ptr<base::Value> json) { std::string client_token; std::string host; int port; - std::string cert_str; + std::string cert_fingerprint; + std::string cert; if (!(dict->GetString(kClientTokenKey, &client_token) && dict->GetString(kHostKey, &host) && dict->GetInteger(kPortKey, &port) && - dict->GetString(kCertificateKey, &cert_str))) { + dict->GetString(kCertificateFingerprintKey, &cert_fingerprint) && + dict->GetString(kCertificateKey, &cert))) { base::ResetAndReturn(&callback_) .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment()); return; @@ -348,33 +296,18 @@ void AssignmentSource::OnJsonParsed(scoped_ptr<base::Value> json) { return; } - net::CertificateList cert_list = - net::X509Certificate::CreateCertificateListFromBytes( - cert_str.data(), cert_str.size(), - net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE); - if (cert_list.size() != 1) { - base::ResetAndReturn(&callback_) - .Run(AssignmentSource::Result::RESULT_INVALID_CERT, Assignment()); - return; - } - + Assignment assignment; // The assigner assumes SSL-only and all engines it assigns only communicate // over SSL. - Assignment assignment; assignment.transport_protocol = Assignment::TransportProtocol::SSL; - assignment.engine_endpoint = net::IPEndPoint(ip_address, port); + assignment.ip_endpoint = net::IPEndPoint(ip_address, port); assignment.client_token = client_token; - assignment.cert = std::move(cert_list[0]); + assignment.certificate = cert; + assignment.certificate_fingerprint = cert_fingerprint; base::ResetAndReturn(&callback_) .Run(AssignmentSource::Result::RESULT_OK, assignment); } -void AssignmentSource::OnJsonParseError(const std::string& error) { - DLOG(ERROR) << "Error while parsing assigner JSON: " << error; - base::ResetAndReturn(&callback_) - .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment()); -} - } // namespace client } // namespace blimp diff --git a/blimp/client/session/assignment_source.h b/blimp/client/session/assignment_source.h index bafd021..dabe72c 100644 --- a/blimp/client/session/assignment_source.h +++ b/blimp/client/session/assignment_source.h @@ -8,21 +8,17 @@ #include <string> #include "base/callback.h" -#include "base/memory/weak_ptr.h" #include "blimp/client/blimp_client_export.h" #include "net/base/ip_endpoint.h" #include "net/url_request/url_fetcher_delegate.h" namespace base { -class FilePath; class SingleThreadTaskRunner; -class Value; } namespace net { class URLFetcher; class URLRequestContextGetter; -class X509Certificate; } namespace blimp { @@ -42,26 +38,21 @@ struct BLIMP_CLIENT_EXPORT Assignment { UNKNOWN = 0, SSL = 1, TCP = 2, + QUIC = 3, }; Assignment(); ~Assignment(); - // Returns true if the net::IPEndPoint has an unspecified IP, port, or - // transport protocol. - bool IsValid() const; - - // Specifies the transport to use to connect to the engine. TransportProtocol transport_protocol; - - // Specifies the IP address and port on which to reach the engine. - net::IPEndPoint engine_endpoint; - - // Used to authenticate to the specified engine. + net::IPEndPoint ip_endpoint; std::string client_token; + std::string certificate; + std::string certificate_fingerprint; - // Specifies the X.509 certificate that the engine must report. - scoped_refptr<net::X509Certificate> cert; + // Returns true if the net::IPEndPoint has an unspecified IP, port, or + // transport protocol. + bool is_null() const; }; // AssignmentSource provides functionality to find out how a client should @@ -81,21 +72,18 @@ class BLIMP_CLIENT_EXPORT AssignmentSource : public net::URLFetcherDelegate { RESULT_OUT_OF_VMS = 7, RESULT_SERVER_ERROR = 8, RESULT_SERVER_INTERRUPTED = 9, - RESULT_NETWORK_FAILURE = 10, - RESULT_INVALID_CERT = 11, + RESULT_NETWORK_FAILURE = 10 }; typedef base::Callback<void(AssignmentSource::Result, const Assignment&)> AssignmentCallback; - // |network_task_runner|: The task runner to use for querying the Assigner API - // over the network. - // |file_task_runner|: The task runner to use for reading cert files from disk - // (specified on the command line.) + // The |main_task_runner| should be the task runner for the UI thread because + // this will in some cases be used to trigger user interaction on the UI + // thread. AssignmentSource( - const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner, - const scoped_refptr<base::SingleThreadTaskRunner>& file_task_runner); - + const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner, + const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner); ~AssignmentSource() override; // Retrieves a valid assignment for the client and posts the result to the @@ -106,25 +94,20 @@ class BLIMP_CLIENT_EXPORT AssignmentSource : public net::URLFetcherDelegate { void GetAssignment(const std::string& client_auth_token, const AssignmentCallback& callback); - private: - void OnGetAssignmentFromCommandLineDone(const std::string& client_auth_token, - Assignment parsed_assignment); - void QueryAssigner(const std::string& client_auth_token); - void ParseAssignerResponse(); - void OnJsonParsed(scoped_ptr<base::Value> json); - void OnJsonParseError(const std::string& error); - // net::URLFetcherDelegate implementation: void OnURLFetchComplete(const net::URLFetcher* source) override; - // GetAssignment() callback, invoked after URLFetcher completion. - AssignmentCallback callback_; + private: + void ParseAssignerResponse(); + + scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_; - scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_; scoped_refptr<net::URLRequestContextGetter> url_request_context_; scoped_ptr<net::URLFetcher> url_fetcher_; - base::WeakPtrFactory<AssignmentSource> weak_factory_; + // This callback is set during a call to GetAssignment() and is cleared after + // the request has completed (whether it be a success or failure). + AssignmentCallback callback_; DISALLOW_COPY_AND_ASSIGN(AssignmentSource); }; diff --git a/blimp/client/session/assignment_source_unittest.cc b/blimp/client/session/assignment_source_unittest.cc index 54f3e44..d893ae3 100644 --- a/blimp/client/session/assignment_source_unittest.cc +++ b/blimp/client/session/assignment_source_unittest.cc @@ -5,80 +5,68 @@ #include "blimp/client/session/assignment_source.h" #include "base/command_line.h" -#include "base/files/file_path.h" -#include "base/files/file_util.h" #include "base/json/json_reader.h" #include "base/json/json_writer.h" -#include "base/message_loop/message_loop.h" -#include "base/path_service.h" -#include "base/run_loop.h" #include "base/test/test_simple_task_runner.h" #include "base/thread_task_runner_handle.h" #include "base/values.h" #include "blimp/client/app/blimp_client_switches.h" #include "blimp/common/protocol_version.h" -#include "components/safe_json/testing_json_parser.h" -#include "net/base/test_data_directory.h" #include "net/url_request/test_url_fetcher_factory.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" using testing::_; -using testing::DoAll; using testing::InSequence; -using testing::NotNull; -using testing::Return; -using testing::SetArgPointee; namespace blimp { namespace client { namespace { -const uint8_t kTestIpAddress[] = {127, 0, 0, 1}; -const uint16_t kTestPort = 8086; -const char kTestIpAddressString[] = "127.0.0.1"; -const char kTcpTransportName[] = "tcp"; -const char kSslTransportName[] = "ssl"; -const char kCertRelativePath[] = - "blimp/client/session/test_selfsigned_cert.pem"; -const char kTestClientToken[] = "secrett0ken"; -const char kTestAuthToken[] = "UserAuthT0kenz"; - MATCHER_P(AssignmentEquals, assignment, "") { return arg.transport_protocol == assignment.transport_protocol && - arg.engine_endpoint == assignment.engine_endpoint && + arg.ip_endpoint == assignment.ip_endpoint && arg.client_token == assignment.client_token && - ((!assignment.cert && !arg.cert) || - (arg.cert && assignment.cert && - arg.cert->Equals(assignment.cert.get()))); + arg.certificate == assignment.certificate && + arg.certificate_fingerprint == assignment.certificate_fingerprint; +} + +net::IPEndPoint BuildIPEndPoint(const std::string& ip, int port) { + net::IPAddress ip_address; + EXPECT_TRUE(ip_address.AssignFromIPLiteral(ip)); + + return net::IPEndPoint(ip_address, port); +} + +Assignment BuildValidAssignment() { + Assignment assignment; + assignment.transport_protocol = Assignment::TransportProtocol::SSL; + assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500); + assignment.client_token = "SecretT0kenz"; + assignment.certificate_fingerprint = "WhaleWhaleWhale"; + assignment.certificate = "whaaaaaaaaaaaaale"; + return assignment; } -// Converts |value| to a JSON string. -std::string ValueToString(const base::Value& value) { +std::string BuildResponseFromAssignment(const Assignment& assignment) { + base::DictionaryValue dict; + dict.SetString("clientToken", assignment.client_token); + dict.SetString("host", assignment.ip_endpoint.address().ToString()); + dict.SetInteger("port", assignment.ip_endpoint.port()); + dict.SetString("certificateFingerprint", assignment.certificate_fingerprint); + dict.SetString("certificate", assignment.certificate); + std::string json; - base::JSONWriter::Write(value, &json); + base::JSONWriter::Write(dict, &json); return json; } class AssignmentSourceTest : public testing::Test { public: AssignmentSourceTest() - : source_(message_loop_.task_runner(), message_loop_.task_runner()) {} - - void SetUp() override { - base::FilePath src_root; - PathService::Get(base::DIR_SOURCE_ROOT, &src_root); - ASSERT_FALSE(src_root.empty()); - cert_path_ = src_root.Append(kCertRelativePath); - ASSERT_TRUE(base::ReadFileToString(cert_path_, &cert_pem_)); - net::CertificateList cert_list = - net::X509Certificate::CreateCertificateListFromBytes( - cert_pem_.data(), cert_pem_.size(), - net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE); - ASSERT_FALSE(cert_list.empty()); - cert_ = std::move(cert_list[0]); - ASSERT_TRUE(cert_); - } + : task_runner_(new base::TestSimpleTaskRunner), + task_runner_handle_(task_runner_), + source_(task_runner_, task_runner_) {} // This expects the AssignmentSource::GetAssignment to return a custom // endpoint without having to hit the network. This will typically be used @@ -89,7 +77,7 @@ class AssignmentSourceTest : public testing::Test { base::Bind(&AssignmentSourceTest::AssignmentResponse, base::Unretained(this))); EXPECT_EQ(nullptr, factory_.GetFetcherByID(0)); - base::RunLoop().RunUntilIdle(); + task_runner_->RunUntilIdle(); } // See net/base/net_errors.h for possible status errors. @@ -102,10 +90,11 @@ class AssignmentSourceTest : public testing::Test { source_.GetAssignment(client_auth_token, base::Bind(&AssignmentSourceTest::AssignmentResponse, base::Unretained(this))); - base::RunLoop().RunUntilIdle(); net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0); + task_runner_->RunUntilIdle(); + EXPECT_NE(nullptr, fetcher); EXPECT_EQ(kDefaultAssignerURL, fetcher->GetOriginalURL().spec()); @@ -136,74 +125,30 @@ class AssignmentSourceTest : public testing::Test { fetcher->SetResponseString(response); fetcher->delegate()->OnURLFetchComplete(fetcher); - base::RunLoop().RunUntilIdle(); + task_runner_->RunUntilIdle(); } MOCK_METHOD2(AssignmentResponse, void(AssignmentSource::Result, const Assignment&)); protected: - Assignment BuildSslAssignment(); - - // Builds simulated JSON response from the Assigner service. - scoped_ptr<base::DictionaryValue> BuildAssignerResponse(); - // Used to drive all AssignmentSource tasks. - // MessageLoop is required by TestingJsonParser's self-deletion logic. - // TODO(bauerb): Replace this with a TestSimpleTaskRunner once - // TestingJsonParser no longer requires having a MessageLoop. - base::MessageLoop message_loop_; + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + base::ThreadTaskRunnerHandle task_runner_handle_; net::TestURLFetcherFactory factory_; - // Path to the PEM-encoded certificate chain. - base::FilePath cert_path_; - - // Payload of PEM certificate chain at |cert_path_|. - std::string cert_pem_; - - // X509 certificate decoded from |cert_path_|. - scoped_refptr<net::X509Certificate> cert_; - AssignmentSource source_; - - // Allows safe_json to parse JSON in-process, instead of depending on a - // utility proces. - safe_json::TestingJsonParser::ScopedFactoryOverride json_parsing_factory_; }; -Assignment AssignmentSourceTest::BuildSslAssignment() { - Assignment assignment; - assignment.transport_protocol = Assignment::TransportProtocol::SSL; - assignment.engine_endpoint = net::IPEndPoint(kTestIpAddress, kTestPort); - assignment.client_token = kTestClientToken; - assignment.cert = cert_; - return assignment; -} - -scoped_ptr<base::DictionaryValue> -AssignmentSourceTest::BuildAssignerResponse() { - scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); - dict->SetString("clientToken", kTestClientToken); - dict->SetString("host", kTestIpAddressString); - dict->SetInteger("port", kTestPort); - dict->SetString("certificate", cert_pem_); - return dict; -} - TEST_F(AssignmentSourceTest, TestTCPAlternateEndpointSuccess) { Assignment assignment; assignment.transport_protocol = Assignment::TransportProtocol::TCP; - assignment.engine_endpoint = net::IPEndPoint(kTestIpAddress, kTestPort); + assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500); assignment.client_token = kDummyClientToken; - assignment.cert = scoped_refptr<net::X509Certificate>(nullptr); base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kEngineIP, kTestIpAddressString); - base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kEnginePort, std::to_string(kTestPort)); - base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kEngineTransport, kTcpTransportName); + switches::kBlimpletEndpoint, "tcp:100.150.200.250:500"); EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK, AssignmentEquals(assignment))) @@ -215,18 +160,27 @@ TEST_F(AssignmentSourceTest, TestTCPAlternateEndpointSuccess) { TEST_F(AssignmentSourceTest, TestSSLAlternateEndpointSuccess) { Assignment assignment; assignment.transport_protocol = Assignment::TransportProtocol::SSL; - assignment.engine_endpoint = net::IPEndPoint(kTestIpAddress, kTestPort); + assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500); assignment.client_token = kDummyClientToken; - assignment.cert = cert_; base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kEngineIP, kTestIpAddressString); - base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kEnginePort, std::to_string(kTestPort)); - base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kEngineTransport, kSslTransportName); + switches::kBlimpletEndpoint, "ssl:100.150.200.250:500"); + + EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK, + AssignmentEquals(assignment))) + .Times(1); + + GetAlternateAssignment(); +} + +TEST_F(AssignmentSourceTest, TestQUICAlternateEndpointSuccess) { + Assignment assignment; + assignment.transport_protocol = Assignment::TransportProtocol::QUIC; + assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500); + assignment.client_token = kDummyClientToken; + base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( - switches::kEngineCertPath, cert_path_.value()); + switches::kBlimpletEndpoint, "quic:100.150.200.250:500"); EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK, AssignmentEquals(assignment))) @@ -236,20 +190,44 @@ TEST_F(AssignmentSourceTest, TestSSLAlternateEndpointSuccess) { } TEST_F(AssignmentSourceTest, TestSuccess) { - Assignment assignment = BuildSslAssignment(); + Assignment assignment = BuildValidAssignment(); EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK, AssignmentEquals(assignment))) .Times(1); GetNetworkAssignmentAndWaitForResponse( - net::HTTP_OK, net::Error::OK, ValueToString(*BuildAssignerResponse()), - kTestAuthToken, kEngineVersion); + net::HTTP_OK, net::Error::OK, BuildResponseFromAssignment(assignment), + "UserAuthT0kenz", kEngineVersion); +} + +TEST_F(AssignmentSourceTest, TestSecondRequestInterruptsFirst) { + InSequence sequence; + Assignment assignment = BuildValidAssignment(); + + source_.GetAssignment("", + base::Bind(&AssignmentSourceTest::AssignmentResponse, + base::Unretained(this))); + + EXPECT_CALL(*this, AssignmentResponse( + AssignmentSource::Result::RESULT_SERVER_INTERRUPTED, + AssignmentEquals(Assignment()))) + .Times(1) + .RetiresOnSaturation(); + + EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK, + AssignmentEquals(assignment))) + .Times(1) + .RetiresOnSaturation(); + + GetNetworkAssignmentAndWaitForResponse( + net::HTTP_OK, net::Error::OK, BuildResponseFromAssignment(assignment), + "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestValidAfterError) { InSequence sequence; - Assignment assignment = BuildSslAssignment(); + Assignment assignment = BuildValidAssignment(); EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_NETWORK_FAILURE, _)) @@ -263,11 +241,11 @@ TEST_F(AssignmentSourceTest, TestValidAfterError) { GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::ERR_INSUFFICIENT_RESOURCES, - "", kTestAuthToken, kEngineVersion); + "", "UserAuthT0kenz", kEngineVersion); GetNetworkAssignmentAndWaitForResponse( - net::HTTP_OK, net::Error::OK, ValueToString(*BuildAssignerResponse()), - kTestAuthToken, kEngineVersion); + net::HTTP_OK, net::Error::OK, BuildResponseFromAssignment(assignment), + "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestNetworkFailure) { @@ -275,14 +253,14 @@ TEST_F(AssignmentSourceTest, TestNetworkFailure) { AssignmentSource::Result::RESULT_NETWORK_FAILURE, _)); GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::ERR_INSUFFICIENT_RESOURCES, - "", kTestAuthToken, kEngineVersion); + "", "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestBadRequest) { EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_BAD_REQUEST, _)); GetNetworkAssignmentAndWaitForResponse(net::HTTP_BAD_REQUEST, net::Error::OK, - "", kTestAuthToken, kEngineVersion); + "", "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestUnauthorized) { @@ -290,21 +268,21 @@ TEST_F(AssignmentSourceTest, TestUnauthorized) { AssignmentResponse( AssignmentSource::Result::RESULT_EXPIRED_ACCESS_TOKEN, _)); GetNetworkAssignmentAndWaitForResponse(net::HTTP_UNAUTHORIZED, net::Error::OK, - "", kTestAuthToken, kEngineVersion); + "", "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestForbidden) { EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_USER_INVALID, _)); GetNetworkAssignmentAndWaitForResponse(net::HTTP_FORBIDDEN, net::Error::OK, - "", kTestAuthToken, kEngineVersion); + "", "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestTooManyRequests) { EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_OUT_OF_VMS, _)); GetNetworkAssignmentAndWaitForResponse(static_cast<net::HttpStatusCode>(429), - net::Error::OK, "", kTestAuthToken, + net::Error::OK, "", "UserAuthT0kenz", kEngineVersion); } @@ -312,7 +290,7 @@ TEST_F(AssignmentSourceTest, TestInternalServerError) { EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_SERVER_ERROR, _)); GetNetworkAssignmentAndWaitForResponse(net::HTTP_INTERNAL_SERVER_ERROR, - net::Error::OK, "", kTestAuthToken, + net::Error::OK, "", "UserAuthT0kenz", kEngineVersion); } @@ -320,62 +298,56 @@ TEST_F(AssignmentSourceTest, TestUnexpectedNetCodeFallback) { EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_BAD_RESPONSE, _)); GetNetworkAssignmentAndWaitForResponse(net::HTTP_NOT_IMPLEMENTED, - net::Error::OK, "", kTestAuthToken, + net::Error::OK, "", "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestInvalidJsonResponse) { - Assignment assignment = BuildSslAssignment(); + Assignment assignment = BuildValidAssignment(); // Remove half the response. - std::string response = ValueToString(*BuildAssignerResponse()); + std::string response = BuildResponseFromAssignment(assignment); response = response.substr(response.size() / 2); EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_BAD_RESPONSE, _)); GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, response, - kTestAuthToken, kEngineVersion); + "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestMissingResponsePort) { - scoped_ptr<base::DictionaryValue> response = BuildAssignerResponse(); - response->Remove("port", nullptr); + // Purposely do not add the 'port' field to the response. + base::DictionaryValue dict; + dict.SetString("clientToken", "SecretT0kenz"); + dict.SetString("host", "happywhales"); + dict.SetString("certificateFingerprint", "WhaleWhaleWhale"); + dict.SetString("certificate", "whaaaaaaaaaaaaale"); + + std::string response; + base::JSONWriter::Write(dict, &response); + EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_BAD_RESPONSE, _)); - GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, - ValueToString(*response), - kTestAuthToken, kEngineVersion); + GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, response, + "UserAuthT0kenz", kEngineVersion); } TEST_F(AssignmentSourceTest, TestInvalidIPAddress) { - scoped_ptr<base::DictionaryValue> response = BuildAssignerResponse(); - response->SetString("host", "happywhales.test"); + // Purposely add an invalid IP field to the response. + base::DictionaryValue dict; + dict.SetString("clientToken", "SecretT0kenz"); + dict.SetString("host", "happywhales"); + dict.SetInteger("port", 500); + dict.SetString("certificateFingerprint", "WhaleWhaleWhale"); + dict.SetString("certificate", "whaaaaaaaaaaaaale"); - EXPECT_CALL(*this, AssignmentResponse( - AssignmentSource::Result::RESULT_BAD_RESPONSE, _)); - GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, - ValueToString(*response), - kTestAuthToken, kEngineVersion); -} + std::string response; + base::JSONWriter::Write(dict, &response); -TEST_F(AssignmentSourceTest, TestMissingCert) { - scoped_ptr<base::DictionaryValue> response = BuildAssignerResponse(); - response->Remove("certificate", nullptr); EXPECT_CALL(*this, AssignmentResponse( AssignmentSource::Result::RESULT_BAD_RESPONSE, _)); - GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, - ValueToString(*response), - kTestAuthToken, kEngineVersion); -} - -TEST_F(AssignmentSourceTest, TestInvalidCert) { - scoped_ptr<base::DictionaryValue> response = BuildAssignerResponse(); - response->SetString("certificate", "h4x0rz!"); - EXPECT_CALL(*this, AssignmentResponse( - AssignmentSource::Result::RESULT_INVALID_CERT, _)); - GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, - ValueToString(*response), - kTestAuthToken, kEngineVersion); + GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, response, + "UserAuthT0kenz", kEngineVersion); } } // namespace diff --git a/blimp/client/session/blimp_client_session.cc b/blimp/client/session/blimp_client_session.cc index 7846d59..29c0b32 100644 --- a/blimp/client/session/blimp_client_session.cc +++ b/blimp/client/session/blimp_client_session.cc @@ -22,7 +22,6 @@ #include "blimp/net/client_connection_manager.h" #include "blimp/net/common.h" #include "blimp/net/null_blimp_message_processor.h" -#include "blimp/net/ssl_client_transport.h" #include "blimp/net/tcp_client_transport.h" #include "net/base/address_list.h" #include "net/base/ip_address.h" @@ -67,6 +66,7 @@ class ClientNetworkComponents { // they are used from the UI thread. std::vector<scoped_ptr<BlimpMessageThreadPipe>> outgoing_pipes_; std::vector<scoped_ptr<BlimpMessageProcessor>> outgoing_message_processors_; + DISALLOW_COPY_AND_ASSIGN(ClientNetworkComponents); }; @@ -81,20 +81,8 @@ void ClientNetworkComponents::ConnectWithAssignment( DCHECK(connection_manager_); connection_manager_->set_client_token(assignment.client_token); - switch (assignment.transport_protocol) { - case Assignment::SSL: - DCHECK(assignment.cert); - connection_manager_->AddTransport(make_scoped_ptr(new SSLClientTransport( - assignment.engine_endpoint, std::move(assignment.cert), nullptr))); - break; - case Assignment::TCP: - connection_manager_->AddTransport(make_scoped_ptr( - new TCPClientTransport(assignment.engine_endpoint, nullptr))); - break; - case Assignment::UNKNOWN: - DLOG(FATAL) << "Uknown transport type."; - break; - } + connection_manager_->AddTransport(make_scoped_ptr(new TCPClientTransport( + net::AddressList(assignment.ip_endpoint), nullptr))); connection_manager_->Connect(); } @@ -130,8 +118,8 @@ BlimpClientSession::BlimpClientSession() options.message_loop_type = base::MessageLoop::TYPE_IO; io_thread_.StartWithOptions(options); - assignment_source_.reset( - new AssignmentSource(io_thread_.task_runner(), io_thread_.task_runner())); + assignment_source_.reset(new AssignmentSource( + base::ThreadTaskRunnerHandle::Get(), io_thread_.task_runner())); // Register features' message senders and receivers. tab_control_feature_->set_outgoing_message_processor( diff --git a/blimp/client/session/test_selfsigned_cert.pem b/blimp/client/session/test_selfsigned_cert.pem deleted file mode 100644 index de0e79f..0000000 --- a/blimp/client/session/test_selfsigned_cert.pem +++ /dev/null @@ -1,11 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIBmjCCAQOgAwIBAgIBATANBgkqhkiG9w0BAQUFADATMREwDwYDVQQDEwh1bml0 -dGVzdDAeFw0xMDEyMjMwMjI5NDhaFw0xMDEyMjQwMjI5NDhaMBMxETAPBgNVBAMT -CHVuaXR0ZXN0MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDUrDNORwXwkey6 -1BtbawRswIkKE81+eZhlIOpMyKayUhi4qB5qaAg0Mt0Of7SwXYA/Nk0ADzcbI+jn -bl8y1gSaHN33I/OO9bEEwBtL2c+iF0Z9tI4iQ1lU2LQ2qiW8nfdQ21HUFbcnkk31 -vE8wNzh3c332RgVzvo5nzypVkamLgQIDAQABMA0GCSqGSIb3DQEBBQUAA4GBABu5 -toCzcK6zKviZe+Uj5EVRUMwDywwCA7FadW7JmgCVt6yQ9YXgkqZelk0aodKZs+eS -WHuyx0EKVuZzaIiI2b/PKnfGVIvAu5Svzdqc8mlwEy4cBYoVFnFOIMzN93p9uUER -Y8o1fBKQE2LhRv3v86PYez5EI7xbUc/5ai+9LkXe ------END CERTIFICATE----- diff --git a/blimp/docs/running.md b/blimp/docs/running.md index 8accd3e..a32b436 100644 --- a/blimp/docs/running.md +++ b/blimp/docs/running.md @@ -21,15 +21,10 @@ Set up any command line flags with: ./build/android/adb_blimp_command_line --your-flag-here ``` -To have the client connect to a custom engine use the `--engine-ip`, -`--engine-port`, and `--engine-transport` flags. The possible valid -values for `--engine-transport` are 'tcp' and 'ssl'. -An example valid endpoint would be -`--engine-ip=127.0.0.1 --engine-port=1234 --engine-transport=tcp`. - -SSL-encrypted connections take an additional flag -`--engine-cert-path` which specifies a path to a PEM-encoded certificate -file (e.g. `--engine-cert-path=/path/to/file.pem`.) +To have the client connect to a custom engine use the `--blimplet-endpoint` +flag. This takes values in the form of scheme:ip:port. The possible valid +schemes are 'tcp', 'quic', and 'ssl'. An example valid endpoint would be +`--blimplet-endpoint=tcp:127.0.0.1:25467`. To see your current command line, run `adb_blimp_command_line` without any arguments. diff --git a/blimp/engine/BUILD.gn b/blimp/engine/BUILD.gn index 5bd4c21..b91730f 100644 --- a/blimp/engine/BUILD.gn +++ b/blimp/engine/BUILD.gn @@ -244,7 +244,7 @@ if (is_linux) { # Detail the target & "source"-file dependencies, and output, for GN. deps = [ - "//blimp/engine:blimp_engine_app", + "//blimp/engine:blimp_engine", ] sources = [ _rebased_dockerfile, diff --git a/blimp/net/BUILD.gn b/blimp/net/BUILD.gn index 28c00cd..aeafbae 100644 --- a/blimp/net/BUILD.gn +++ b/blimp/net/BUILD.gn @@ -34,16 +34,12 @@ component("blimp_net") { "engine_authentication_handler.h", "engine_connection_manager.cc", "engine_connection_manager.h", - "exact_match_cert_verifier.cc", - "exact_match_cert_verifier.h", "input_message_converter.cc", "input_message_converter.h", "input_message_generator.cc", "input_message_generator.h", "null_blimp_message_processor.cc", "null_blimp_message_processor.h", - "ssl_client_transport.cc", - "ssl_client_transport.h", "stream_packet_reader.cc", "stream_packet_reader.h", "stream_packet_writer.cc", @@ -98,7 +94,6 @@ source_set("unit_tests") { "engine_authentication_handler_unittest.cc", "engine_connection_manager_unittest.cc", "input_message_unittest.cc", - "ssl_client_transport_unittest.cc", "stream_packet_reader_unittest.cc", "stream_packet_writer_unittest.cc", "tcp_transport_unittest.cc", diff --git a/blimp/net/DEPS b/blimp/net/DEPS index 211c82c..9891086 100644 --- a/blimp/net/DEPS +++ b/blimp/net/DEPS @@ -1,5 +1,6 @@ include_rules = [ - "+net", + "+net/base", + "+net/socket", "+third_party/WebKit/public/platform/WebGestureDevice.h", "+third_party/WebKit/public/web/WebInputEvent.h", ] diff --git a/blimp/net/blimp_transport.h b/blimp/net/blimp_transport.h index 8deabd5..8336912 100644 --- a/blimp/net/blimp_transport.h +++ b/blimp/net/blimp_transport.h @@ -35,7 +35,7 @@ class BlimpTransport { virtual scoped_ptr<BlimpConnection> TakeConnection() = 0; // Gets transport name, e.g. "TCP", "SSL", "mock", etc. - virtual const char* GetName() const = 0; + virtual const std::string GetName() const = 0; }; } // namespace blimp diff --git a/blimp/net/exact_match_cert_verifier.cc b/blimp/net/exact_match_cert_verifier.cc deleted file mode 100644 index c140c36..0000000 --- a/blimp/net/exact_match_cert_verifier.cc +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "blimp/net/exact_match_cert_verifier.h" - -#include "base/callback.h" -#include "base/macros.h" -#include "base/memory/scoped_ptr.h" -#include "net/base/net_errors.h" -#include "net/cert/cert_verifier.h" -#include "net/cert/cert_verify_result.h" -#include "net/cert/x509_certificate.h" - -namespace blimp { - -ExactMatchCertVerifier::ExactMatchCertVerifier( - scoped_refptr<net::X509Certificate> engine_cert) - : engine_cert_(std::move(engine_cert)) { - DCHECK(engine_cert); - - net::SHA1HashValue sha1_hash; - sha1_hash = net::X509Certificate::CalculateFingerprint( - engine_cert_->os_cert_handle()); - engine_cert_hashes_.push_back(net::HashValue(sha1_hash)); - - net::SHA256HashValue sha256_hash; - sha256_hash = net::X509Certificate::CalculateFingerprint256( - engine_cert_->os_cert_handle()); - engine_cert_hashes_.push_back(net::HashValue(sha256_hash)); -} - -ExactMatchCertVerifier::~ExactMatchCertVerifier() {} - -int ExactMatchCertVerifier::Verify(net::X509Certificate* cert, - const std::string& hostname, - const std::string& ocsp_response, - int flags, - net::CRLSet* crl_set, - net::CertVerifyResult* verify_result, - const net::CompletionCallback& callback, - scoped_ptr<Request>* out_req, - const net::BoundNetLog& net_log) { - verify_result->Reset(); - verify_result->verified_cert = engine_cert_; - - if (!cert->Equals(engine_cert_.get())) { - verify_result->cert_status = net::CERT_STATUS_INVALID; - return net::ERR_CERT_INVALID; - } - - verify_result->public_key_hashes = engine_cert_hashes_; - return net::OK; -} - -} // namespace blimp diff --git a/blimp/net/exact_match_cert_verifier.h b/blimp/net/exact_match_cert_verifier.h deleted file mode 100644 index d26d57a..0000000 --- a/blimp/net/exact_match_cert_verifier.h +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef BLIMP_NET_EXACT_MATCH_CERT_VERIFIER_H_ -#define BLIMP_NET_EXACT_MATCH_CERT_VERIFIER_H_ - -#include <string> -#include <vector> - -#include "blimp/net/blimp_net_export.h" -#include "net/cert/cert_verifier.h" - -namespace net { -class HashValue; -} // namespace net - -namespace blimp { - -// Checks if the peer certificate is an exact match to the X.509 certificate -// |engine_cert|, which is specified at class construction time. -class BLIMP_NET_EXPORT ExactMatchCertVerifier : public net::CertVerifier { - public: - // |engine_cert|: The single allowable certificate. - explicit ExactMatchCertVerifier( - scoped_refptr<net::X509Certificate> engine_cert); - - ~ExactMatchCertVerifier() override; - - // net::CertVerifier implementation. - int Verify(net::X509Certificate* cert, - const std::string& hostname, - const std::string& ocsp_response, - int flags, - net::CRLSet* crl_set, - net::CertVerifyResult* verify_result, - const net::CompletionCallback& callback, - scoped_ptr<Request>* out_req, - const net::BoundNetLog& net_log) override; - - private: - scoped_refptr<net::X509Certificate> engine_cert_; - std::vector<net::HashValue> engine_cert_hashes_; - - DISALLOW_COPY_AND_ASSIGN(ExactMatchCertVerifier); -}; - -} // namespace blimp - -#endif // BLIMP_NET_EXACT_MATCH_CERT_VERIFIER_H_ diff --git a/blimp/net/ssl_client_transport.cc b/blimp/net/ssl_client_transport.cc deleted file mode 100644 index 3364093..0000000 --- a/blimp/net/ssl_client_transport.cc +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "blimp/net/ssl_client_transport.h" - -#include "base/callback.h" -#include "base/callback_helpers.h" -#include "blimp/net/exact_match_cert_verifier.h" -#include "blimp/net/stream_socket_connection.h" -#include "net/base/host_port_pair.h" -#include "net/cert/x509_certificate.h" -#include "net/socket/client_socket_factory.h" -#include "net/socket/client_socket_handle.h" -#include "net/socket/ssl_client_socket.h" -#include "net/socket/stream_socket.h" -#include "net/socket/tcp_client_socket.h" -#include "net/ssl/ssl_config.h" - -namespace blimp { - -SSLClientTransport::SSLClientTransport(const net::IPEndPoint& ip_endpoint, - scoped_refptr<net::X509Certificate> cert, - net::NetLog* net_log) - : TCPClientTransport(ip_endpoint, net_log), ip_endpoint_(ip_endpoint) { - // Test code may pass in a null value for |cert|. Only spin up a CertVerifier - // if there is a cert present. - if (cert) { - cert_verifier_.reset(new ExactMatchCertVerifier(std::move(cert))); - } -} - -SSLClientTransport::~SSLClientTransport() {} - -const char* SSLClientTransport::GetName() const { - return "SSL"; -} - -void SSLClientTransport::OnTCPConnectComplete(int result) { - DCHECK_NE(net::ERR_IO_PENDING, result); - - scoped_ptr<net::StreamSocket> tcp_socket = TCPClientTransport::TakeSocket(); - - DVLOG(1) << "TCP connection result=" << result; - if (result != net::OK) { - OnConnectComplete(result); - return; - } - - // Construct arguments to use for the SSL socket factory. - scoped_ptr<net::ClientSocketHandle> socket_handle( - new net::ClientSocketHandle); - socket_handle->SetSocket(std::move(tcp_socket)); - - net::HostPortPair host_port_pair = - net::HostPortPair::FromIPEndPoint(ip_endpoint_); - - net::SSLClientSocketContext create_context; - create_context.cert_verifier = cert_verifier_.get(); - create_context.transport_security_state = &transport_security_state_; - - scoped_ptr<net::StreamSocket> ssl_socket( - socket_factory()->CreateSSLClientSocket(std::move(socket_handle), - host_port_pair, net::SSLConfig(), - create_context)); - - if (!ssl_socket) { - OnConnectComplete(net::ERR_SSL_PROTOCOL_ERROR); - return; - } - - result = ssl_socket->Connect(base::Bind( - &SSLClientTransport::OnSSLConnectComplete, base::Unretained(this))); - SetSocket(std::move(ssl_socket)); - - if (result == net::ERR_IO_PENDING) { - // SSL connection will complete asynchronously. - return; - } - - OnSSLConnectComplete(result); -} - -void SSLClientTransport::OnSSLConnectComplete(int result) { - DCHECK_NE(net::ERR_IO_PENDING, result); - DVLOG(1) << "SSL connection result=" << result; - - OnConnectComplete(result); -} - -} // namespace blimp diff --git a/blimp/net/ssl_client_transport.h b/blimp/net/ssl_client_transport.h deleted file mode 100644 index 0f0a1f4..0000000 --- a/blimp/net/ssl_client_transport.h +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef BLIMP_NET_SSL_CLIENT_TRANSPORT_H_ -#define BLIMP_NET_SSL_CLIENT_TRANSPORT_H_ - -#include <string> -#include "base/callback_forward.h" -#include "base/macros.h" -#include "base/memory/scoped_ptr.h" -#include "blimp/net/blimp_net_export.h" -#include "blimp/net/blimp_transport.h" -#include "blimp/net/exact_match_cert_verifier.h" -#include "blimp/net/tcp_client_transport.h" -#include "net/base/address_list.h" -#include "net/base/net_errors.h" -#include "net/http/transport_security_state.h" - -namespace net { -class ClientSocketFactory; -class NetLog; -class StreamSocket; -class TCPClientSocket; -class TransportSecurityState; -} // namespace net - -namespace blimp { - -class BlimpConnection; - -// Creates and connects SSL socket connections to an Engine. -class BLIMP_NET_EXPORT SSLClientTransport : public TCPClientTransport { - public: - // |ip_endpoint|: the address to connect to. - // |cert|: the certificate required from the remote peer. - // SSL connections that use different certificates are rejected. - // |net_log|: the socket event log (optional). - SSLClientTransport(const net::IPEndPoint& ip_endpoint, - scoped_refptr<net::X509Certificate> cert, - net::NetLog* net_log); - - ~SSLClientTransport() override; - - // BlimpTransport implementation. - const char* GetName() const override; - - private: - // Method called after TCPClientSocket::Connect finishes. - void OnTCPConnectComplete(int result) override; - - // Method called after SSLClientSocket::Connect finishes. - void OnSSLConnectComplete(int result); - - net::IPEndPoint ip_endpoint_; - scoped_ptr<ExactMatchCertVerifier> cert_verifier_; - net::TransportSecurityState transport_security_state_; - - DISALLOW_COPY_AND_ASSIGN(SSLClientTransport); -}; - -} // namespace blimp - -#endif // BLIMP_NET_SSL_CLIENT_TRANSPORT_H_ diff --git a/blimp/net/ssl_client_transport_unittest.cc b/blimp/net/ssl_client_transport_unittest.cc deleted file mode 100644 index 47724ce..0000000 --- a/blimp/net/ssl_client_transport_unittest.cc +++ /dev/null @@ -1,162 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/bind.h" -#include "base/message_loop/message_loop.h" -#include "base/run_loop.h" -#include "blimp/net/blimp_connection.h" -#include "blimp/net/ssl_client_transport.h" -#include "net/base/address_list.h" -#include "net/base/ip_address.h" -#include "net/base/net_errors.h" -#include "net/socket/socket_test_util.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace blimp { -namespace { - -const uint8_t kIPV4Address[] = {127, 0, 0, 1}; -const uint16_t kPort = 6667; - -} // namespace - -class SSLClientTransportTest : public testing::Test { - public: - SSLClientTransportTest() {} - - ~SSLClientTransportTest() override {} - - void TearDown() override { base::RunLoop().RunUntilIdle(); } - - MOCK_METHOD1(ConnectComplete, void(int)); - - protected: - // Methods for provisioning simulated connection outcomes. - void SetupTCPSyncSocketConnect(net::IPEndPoint endpoint, int result) { - tcp_connect_.set_connect_data( - net::MockConnect(net::SYNCHRONOUS, result, endpoint)); - socket_factory_.AddSocketDataProvider(&tcp_connect_); - } - - void SetupTCPAsyncSocketConnect(net::IPEndPoint endpoint, int result) { - tcp_connect_.set_connect_data( - net::MockConnect(net::ASYNC, result, endpoint)); - socket_factory_.AddSocketDataProvider(&tcp_connect_); - } - - void SetupSSLSyncSocketConnect(int result) { - ssl_connect_.reset( - new net::SSLSocketDataProvider(net::SYNCHRONOUS, result)); - socket_factory_.AddSSLSocketDataProvider(ssl_connect_.get()); - } - - void SetupSSLAsyncSocketConnect(int result) { - ssl_connect_.reset(new net::SSLSocketDataProvider(net::ASYNC, result)); - socket_factory_.AddSSLSocketDataProvider(ssl_connect_.get()); - } - - void ConfigureTransport(const net::IPEndPoint& ip_endpoint) { - // The mock does not interact with the cert directly, so just leave it null. - scoped_refptr<net::X509Certificate> cert; - transport_.reset(new SSLClientTransport(ip_endpoint, cert, &net_log_)); - transport_->SetClientSocketFactoryForTest(&socket_factory_); - } - - base::MessageLoop message_loop; - net::NetLog net_log_; - net::StaticSocketDataProvider tcp_connect_; - scoped_ptr<net::SSLSocketDataProvider> ssl_connect_; - net::MockClientSocketFactory socket_factory_; - scoped_ptr<SSLClientTransport> transport_; -}; - -TEST_F(SSLClientTransportTest, ConnectSyncOK) { - net::IPEndPoint endpoint(kIPV4Address, kPort); - ConfigureTransport(endpoint); - for (int i = 0; i < 5; ++i) { - EXPECT_CALL(*this, ConnectComplete(net::OK)); - SetupTCPSyncSocketConnect(endpoint, net::OK); - SetupSSLSyncSocketConnect(net::OK); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); - EXPECT_NE(nullptr, transport_->TakeConnection().get()); - base::RunLoop().RunUntilIdle(); - } -} - -TEST_F(SSLClientTransportTest, ConnectAsyncOK) { - net::IPEndPoint endpoint(kIPV4Address, kPort); - ConfigureTransport(endpoint); - for (int i = 0; i < 5; ++i) { - EXPECT_CALL(*this, ConnectComplete(net::OK)); - SetupTCPAsyncSocketConnect(endpoint, net::OK); - SetupSSLAsyncSocketConnect(net::OK); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); - base::RunLoop().RunUntilIdle(); - EXPECT_NE(nullptr, transport_->TakeConnection().get()); - } -} - -TEST_F(SSLClientTransportTest, ConnectSyncTCPError) { - net::IPEndPoint endpoint(kIPV4Address, kPort); - ConfigureTransport(endpoint); - EXPECT_CALL(*this, ConnectComplete(net::ERR_FAILED)); - SetupTCPSyncSocketConnect(endpoint, net::ERR_FAILED); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); -} - -TEST_F(SSLClientTransportTest, ConnectAsyncTCPError) { - net::IPEndPoint endpoint(kIPV4Address, kPort); - ConfigureTransport(endpoint); - EXPECT_CALL(*this, ConnectComplete(net::ERR_FAILED)); - SetupTCPAsyncSocketConnect(endpoint, net::ERR_FAILED); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); -} - -TEST_F(SSLClientTransportTest, ConnectSyncSSLError) { - net::IPEndPoint endpoint(kIPV4Address, kPort); - ConfigureTransport(endpoint); - EXPECT_CALL(*this, ConnectComplete(net::ERR_FAILED)); - SetupTCPSyncSocketConnect(endpoint, net::OK); - SetupSSLSyncSocketConnect(net::ERR_FAILED); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); -} - -TEST_F(SSLClientTransportTest, ConnectAsyncSSLError) { - net::IPEndPoint endpoint(kIPV4Address, kPort); - ConfigureTransport(endpoint); - EXPECT_CALL(*this, ConnectComplete(net::ERR_FAILED)); - SetupTCPAsyncSocketConnect(endpoint, net::OK); - SetupSSLAsyncSocketConnect(net::ERR_FAILED); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); -} - -TEST_F(SSLClientTransportTest, ConnectAfterError) { - net::IPEndPoint endpoint(kIPV4Address, kPort); - ConfigureTransport(endpoint); - - // TCP connection fails. - EXPECT_CALL(*this, ConnectComplete(net::ERR_FAILED)); - SetupTCPSyncSocketConnect(endpoint, net::ERR_FAILED); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); - base::RunLoop().RunUntilIdle(); - - // Subsequent TCP+SSL connections succeed. - EXPECT_CALL(*this, ConnectComplete(net::OK)); - SetupTCPSyncSocketConnect(endpoint, net::OK); - SetupSSLSyncSocketConnect(net::OK); - transport_->Connect(base::Bind(&SSLClientTransportTest::ConnectComplete, - base::Unretained(this))); - EXPECT_NE(nullptr, transport_->TakeConnection().get()); - base::RunLoop().RunUntilIdle(); -} - -} // namespace blimp diff --git a/blimp/net/tcp_client_transport.cc b/blimp/net/tcp_client_transport.cc index 6a6cc50..9dbb8803 100644 --- a/blimp/net/tcp_client_transport.cc +++ b/blimp/net/tcp_client_transport.cc @@ -9,42 +9,38 @@ #include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "blimp/net/stream_socket_connection.h" -#include "net/socket/client_socket_factory.h" #include "net/socket/stream_socket.h" #include "net/socket/tcp_client_socket.h" namespace blimp { -TCPClientTransport::TCPClientTransport(const net::IPEndPoint& ip_endpoint, +TCPClientTransport::TCPClientTransport(const net::AddressList& addresses, net::NetLog* net_log) - : ip_endpoint_(ip_endpoint), - net_log_(net_log), - socket_factory_(net::ClientSocketFactory::GetDefaultFactory()) {} + : addresses_(addresses), net_log_(net_log) {} TCPClientTransport::~TCPClientTransport() {} -void TCPClientTransport::SetClientSocketFactoryForTest( - net::ClientSocketFactory* factory) { - DCHECK(factory); - socket_factory_ = factory; -} - void TCPClientTransport::Connect(const net::CompletionCallback& callback) { DCHECK(!socket_); DCHECK(!callback.is_null()); - connect_callback_ = callback; - socket_ = socket_factory_->CreateTransportClientSocket( - net::AddressList(ip_endpoint_), net_log_, net::NetLog::Source()); + socket_.reset( + new net::TCPClientSocket(addresses_, net_log_, net::NetLog::Source())); net::CompletionCallback completion_callback = base::Bind( &TCPClientTransport::OnTCPConnectComplete, base::Unretained(this)); int result = socket_->Connect(completion_callback); if (result == net::ERR_IO_PENDING) { + connect_callback_ = callback; return; } - OnTCPConnectComplete(result); + if (result != net::OK) { + socket_ = nullptr; + } + + base::MessageLoop::current()->PostTask(FROM_HERE, + base::Bind(callback, result)); } scoped_ptr<BlimpConnection> TCPClientTransport::TakeConnection() { @@ -53,33 +49,17 @@ scoped_ptr<BlimpConnection> TCPClientTransport::TakeConnection() { return make_scoped_ptr(new StreamSocketConnection(std::move(socket_))); } -const char* TCPClientTransport::GetName() const { +const std::string TCPClientTransport::GetName() const { return "TCP"; } void TCPClientTransport::OnTCPConnectComplete(int result) { DCHECK_NE(net::ERR_IO_PENDING, result); - OnConnectComplete(result); -} - -void TCPClientTransport::OnConnectComplete(int result) { + DCHECK(socket_); if (result != net::OK) { socket_ = nullptr; } base::ResetAndReturn(&connect_callback_).Run(result); } -scoped_ptr<net::StreamSocket> TCPClientTransport::TakeSocket() { - return std::move(socket_); -} - -void TCPClientTransport::SetSocket(scoped_ptr<net::StreamSocket> socket) { - DCHECK(socket); - socket_ = std::move(socket); -} - -net::ClientSocketFactory* TCPClientTransport::socket_factory() const { - return socket_factory_; -} - } // namespace blimp diff --git a/blimp/net/tcp_client_transport.h b/blimp/net/tcp_client_transport.h index 6a879fe..3242883 100644 --- a/blimp/net/tcp_client_transport.h +++ b/blimp/net/tcp_client_transport.h @@ -5,8 +5,6 @@ #ifndef BLIMP_NET_TCP_CLIENT_TRANSPORT_H_ #define BLIMP_NET_TCP_CLIENT_TRANSPORT_H_ -#include <string> - #include "base/callback.h" #include "base/macros.h" #include "base/memory/scoped_ptr.h" @@ -16,7 +14,6 @@ #include "net/base/net_errors.h" namespace net { -class ClientSocketFactory; class NetLog; class StreamSocket; } // namespace net @@ -29,38 +26,21 @@ class BlimpConnection; // |addresses| on each call to Connect(). class BLIMP_NET_EXPORT TCPClientTransport : public BlimpTransport { public: - TCPClientTransport(const net::IPEndPoint& ip_endpoint, net::NetLog* net_log); + TCPClientTransport(const net::AddressList& addresses, net::NetLog* net_log); ~TCPClientTransport() override; - void SetClientSocketFactoryForTest(net::ClientSocketFactory* factory); - // BlimpTransport implementation. void Connect(const net::CompletionCallback& callback) override; scoped_ptr<BlimpConnection> TakeConnection() override; - const char* GetName() const override; - - protected: - // Called when the TCP connection completed. - virtual void OnTCPConnectComplete(int result); - - // Called when the connection attempt completed or failed. - // Resets |socket_| if |result| indicates a failure (!= net::OK). - void OnConnectComplete(int result); - - // Methods for taking and setting |socket_|. Can be used by subclasses to - // swap out a socket for an upgraded one, e.g. adding SSL encryption. - scoped_ptr<net::StreamSocket> TakeSocket(); - void SetSocket(scoped_ptr<net::StreamSocket> socket); - - // Gets the socket factory instance. - net::ClientSocketFactory* socket_factory() const; + const std::string GetName() const override; private: - net::IPEndPoint ip_endpoint_; + void OnTCPConnectComplete(int result); + + net::AddressList addresses_; net::NetLog* net_log_; - net::CompletionCallback connect_callback_; - net::ClientSocketFactory* socket_factory_ = nullptr; scoped_ptr<net::StreamSocket> socket_; + net::CompletionCallback connect_callback_; DISALLOW_COPY_AND_ASSIGN(TCPClientTransport); }; diff --git a/blimp/net/tcp_engine_transport.cc b/blimp/net/tcp_engine_transport.cc index 4606caf..473a1bb 100644 --- a/blimp/net/tcp_engine_transport.cc +++ b/blimp/net/tcp_engine_transport.cc @@ -61,7 +61,7 @@ scoped_ptr<BlimpConnection> TCPEngineTransport::TakeConnection() { new StreamSocketConnection(std::move(accepted_socket_))); } -const char* TCPEngineTransport::GetName() const { +const std::string TCPEngineTransport::GetName() const { return "TCP"; } diff --git a/blimp/net/tcp_engine_transport.h b/blimp/net/tcp_engine_transport.h index ace223e..bf8b7cf 100644 --- a/blimp/net/tcp_engine_transport.h +++ b/blimp/net/tcp_engine_transport.h @@ -33,7 +33,7 @@ class BLIMP_NET_EXPORT TCPEngineTransport : public BlimpTransport { // BlimpTransport implementation. void Connect(const net::CompletionCallback& callback) override; scoped_ptr<BlimpConnection> TakeConnection() override; - const char* GetName() const override; + const std::string GetName() const override; int GetLocalAddressForTesting(net::IPEndPoint* address) const; diff --git a/blimp/net/tcp_transport_unittest.cc b/blimp/net/tcp_transport_unittest.cc index 8f1525a..cef15d3 100644 --- a/blimp/net/tcp_transport_unittest.cc +++ b/blimp/net/tcp_transport_unittest.cc @@ -35,10 +35,10 @@ class TCPTransportTest : public testing::Test { engine_.reset(new TCPEngineTransport(local_address, nullptr)); } - net::IPEndPoint GetLocalEndpoint() const { + net::AddressList GetLocalAddressList() const { net::IPEndPoint local_address; - CHECK_EQ(net::OK, engine_->GetLocalAddressForTesting(&local_address)); - return local_address; + engine_->GetLocalAddressForTesting(&local_address); + return net::AddressList(local_address); } base::MessageLoopForIO message_loop_; @@ -50,7 +50,7 @@ TEST_F(TCPTransportTest, Connect) { engine_->Connect(accept_callback.callback()); net::TestCompletionCallback connect_callback; - TCPClientTransport client(GetLocalEndpoint(), nullptr); + TCPClientTransport client(GetLocalAddressList(), nullptr); client.Connect(connect_callback.callback()); EXPECT_EQ(net::OK, connect_callback.WaitForResult()); @@ -63,11 +63,11 @@ TEST_F(TCPTransportTest, TwoClientConnections) { engine_->Connect(accept_callback1.callback()); net::TestCompletionCallback connect_callback1; - TCPClientTransport client1(GetLocalEndpoint(), nullptr); + TCPClientTransport client1(GetLocalAddressList(), nullptr); client1.Connect(connect_callback1.callback()); net::TestCompletionCallback connect_callback2; - TCPClientTransport client2(GetLocalEndpoint(), nullptr); + TCPClientTransport client2(GetLocalAddressList(), nullptr); client2.Connect(connect_callback2.callback()); EXPECT_EQ(net::OK, connect_callback1.WaitForResult()); @@ -86,7 +86,7 @@ TEST_F(TCPTransportTest, ExchangeMessages) { net::TestCompletionCallback accept_callback; engine_->Connect(accept_callback.callback()); net::TestCompletionCallback client_connect_callback; - TCPClientTransport client(GetLocalEndpoint(), nullptr /* NetLog */); + TCPClientTransport client(GetLocalAddressList(), nullptr /* NetLog */); client.Connect(client_connect_callback.callback()); EXPECT_EQ(net::OK, client_connect_callback.WaitForResult()); EXPECT_EQ(net::OK, accept_callback.WaitForResult()); diff --git a/blimp/net/test_common.cc b/blimp/net/test_common.cc index 92a951d..e6cd956 100644 --- a/blimp/net/test_common.cc +++ b/blimp/net/test_common.cc @@ -26,7 +26,7 @@ scoped_ptr<BlimpConnection> MockTransport::TakeConnection() { return make_scoped_ptr(TakeConnectionPtr()); } -const char* MockTransport::GetName() const { +const std::string MockTransport::GetName() const { return "mock"; } diff --git a/blimp/net/test_common.h b/blimp/net/test_common.h index 380465e..302060a 100644 --- a/blimp/net/test_common.h +++ b/blimp/net/test_common.h @@ -137,7 +137,7 @@ class MockTransport : public BlimpTransport { MOCK_METHOD0(TakeConnectionPtr, BlimpConnection*()); scoped_ptr<BlimpConnection> TakeConnection() override; - const char* GetName() const override; + const std::string GetName() const override; }; class MockConnectionHandler : public ConnectionHandler { |