Stop bitstream audio in IPC layer if not supported

This reduces the attack surface of the browser process.

Bug: 792422
Change-Id: Ie50557d0bc3ee9ace3aad234501bc390d73ab476
Reviewed-on: https://chromium-review.googlesource.com/811265Reviewed-by: Sergey Volk <servolk@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523065}

content/browser/bad_message.h

@@ -214,6 +214,8 @@ enum BadMessageReason {
   RMF_BAD_URL_CACHEABLE_METADATA = 187,
   RFH_INTERFACE_PROVIDER_MISSING = 188,
   RFH_INTERFACE_PROVIDER_SUPERFLUOUS = 189,
+  AIRH_UNEXPECTED_BITSTREAM = 190,
+  ARH_UNEXPECTED_BITSTREAM = 191,
 
   // Please add new elements here. The naming convention is abbreviated class
   // name (e.g. RenderFrameHost becomes RFH) plus a unique description of the

content/browser/renderer_host/media/audio_input_renderer_host.cc

@@ -138,6 +138,13 @@ void AudioInputRendererHost::OnCreateStream(
     const AudioInputHostMsg_CreateStream_Config& config) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  if (config.params.IsBitstreamFormat()) {
+    // Bitstream formats are only supported for output.
+    bad_message::ReceivedBadMessage(this,
+                                    bad_message::AIRH_UNEXPECTED_BITSTREAM);
+    return;
+  }
+
 #if defined(OS_CHROMEOS)
   if (config.params.channel_layout() ==
       media::CHANNEL_LAYOUT_STEREO_AND_KEYBOARD_MIC) {

content/browser/renderer_host/media/audio_renderer_host.cc

@@ -10,6 +10,7 @@
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram_macros.h"
+#include "build/build_config.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/media/audio_stream_monitor.h"
@@ -262,6 +263,16 @@ void AudioRendererHost::OnCreateStream(int stream_id,
     return;
   }
 
+#if !defined(OS_ANDROID)
+  if (params.IsBitstreamFormat()) {
+    // Bitstream formats are only supported on Android, and shouldn't be created
+    // on other platforms.
+    bad_message::ReceivedBadMessage(this,
+                                    bad_message::ARH_UNEXPECTED_BITSTREAM);
+    return;
+  }
+#endif
+
   // Post a task to the UI thread to check that the |render_frame_id| references
   // a valid render frame. This validation is important for all the reasons
   // stated in the comments above. This does not block stream creation, but will

media/mojo/DEPS

@@ -6,6 +6,9 @@ include_rules = [
   "+mojo/logging",
   "+mojo/public",
 
+  # For changing the bad message handler for tests.
+  "+mojo/edk/embedder/embedder.h",
+
   # TODO(xhwang): Ideally media should not worry about sandbox. Find a way to
   # remove this dependency.
   "+sandbox/mac",

media/mojo/services/BUILD.gn

@@ -224,6 +224,7 @@ source_set("unit_tests") {
 
   sources = [
     "mojo_audio_input_stream_unittest.cc",
+    "mojo_audio_output_stream_provider_unittest.cc",
     "mojo_audio_output_stream_unittest.cc",
     "mojo_jpeg_decode_accelerator_service_unittest.cc",
     "mojo_video_encode_accelerator_service_unittest.cc",
@@ -237,6 +238,7 @@ source_set("unit_tests") {
     "//components/ukm:test_support",
     "//media:test_support",
     "//media/mojo:test_support",
+    "//mojo/edk/system",
     "//services/metrics/public/cpp:ukm_builders",
     "//testing/gmock",
     "//testing/gtest",

media/mojo/services/mojo_audio_output_stream_provider.cc

@@ -6,6 +6,9 @@
 
 #include <utility>
 
+#include "build/build_config.h"
+#include "mojo/public/cpp/bindings/message.h"
+
 namespace media {
 
 MojoAudioOutputStreamProvider::MojoAudioOutputStreamProvider(
@@ -36,11 +39,17 @@ void MojoAudioOutputStreamProvider::Acquire(
     const AudioParameters& params,
     AcquireCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+#if !defined(OS_ANDROID)
+  if (params.IsBitstreamFormat()) {
+    // Bitstream streams are only supported on Android.
+    BadMessage(
+        "Attempted to acquire a bitstream audio stream on a platform where "
+        "it's not supported");
+    return;
+  }
+#endif
   if (audio_output_) {
-    LOG(ERROR) << "Output acquired twice.";
-    binding_.Unbind();
-    observer_binding_.Unbind();
-    std::move(deleter_callback_).Run(this);  // deletes |this|.
+    BadMessage("Output acquired twice.");
     return;
   }
 
@@ -61,4 +70,13 @@ void MojoAudioOutputStreamProvider::OnError() {
   std::move(deleter_callback_).Run(this);
 }
 
+void MojoAudioOutputStreamProvider::BadMessage(const std::string& error) {
+  mojo::ReportBadMessage(error);
+  if (binding_.is_bound())
+    binding_.Unbind();
+  if (observer_binding_.is_bound())
+    observer_binding_.Unbind();
+  std::move(deleter_callback_).Run(this);  // deletes |this|.
+}
+
 }  // namespace media

media/mojo/services/mojo_audio_output_stream_provider.h

@@ -50,6 +50,9 @@ class MEDIA_MOJO_EXPORT MojoAudioOutputStreamProvider
   // Called when |audio_output_| had an error.
   void OnError();
 
+  // Closes mojo connections, reports a bad message, and self-destructs.
+  void BadMessage(const std::string& error);
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   base::Optional<MojoAudioOutputStream> audio_output_;

media/mojo/services/mojo_audio_output_stream_provider_unittest.cc

+// Copyright 2017 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 "media/mojo/services/mojo_audio_output_stream_provider.h"
+
+#include <utility>
+
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/test/mock_callback.h"
+#include "build/build_config.h"
+#include "media/audio/audio_output_delegate.h"
+#include "media/base/audio_parameters.h"
+#include "mojo/edk/embedder/embedder.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// Basic usage of MojoAudioOutputStreamProvider is tested in
+// the RenderFrameAudioOutputStreamFactoryTest tests.
+// These additional tests test some error conditions.
+
+namespace media {
+
+namespace {
+
+using testing::DeleteArg;
+using testing::Mock;
+using testing::StrictMock;
+
+using MockDeleter = base::MockCallback<
+    base::OnceCallback<void(mojom::AudioOutputStreamProvider*)>>;
+
+void FakeAcquireCallback(mojo::ScopedSharedBufferHandle, mojo::ScopedHandle) {}
+
+class FakeObserver : public mojom::AudioOutputStreamObserver {
+ public:
+  FakeObserver() = default;
+  ~FakeObserver() override = default;
+
+  void DidStartPlaying() override {}
+  void DidStopPlaying() override {}
+  void DidChangeAudibleState(bool is_audible) override {}
+};
+
+class FakeDelegate : public AudioOutputDelegate {
+ public:
+  explicit FakeDelegate(mojom::AudioOutputStreamObserverPtr observer)
+      : observer_(std::move(observer)) {}
+  ~FakeDelegate() override = default;
+
+  int GetStreamId() override { return 0; }
+  void OnPlayStream() override {}
+  void OnPauseStream() override {}
+  void OnSetVolume(double) override {}
+
+ private:
+  mojom::AudioOutputStreamObserverPtr observer_;
+};
+
+std::unique_ptr<AudioOutputDelegate> CreateFakeDelegate(
+    const AudioParameters& params,
+    mojom::AudioOutputStreamObserverPtr observer,
+    AudioOutputDelegate::EventHandler*) {
+  return std::make_unique<FakeDelegate>(std::move(observer));
+}
+
+}  // namespace
+
+TEST(MojoAudioOutputStreamProviderTest, AcquireTwice_BadMessage) {
+  base::MessageLoop loop;
+  bool got_bad_message = false;
+  mojo::edk::SetDefaultProcessErrorCallback(
+      base::BindRepeating([](bool* got_bad_message,
+                             const std::string& s) { *got_bad_message = true; },
+                          &got_bad_message));
+
+  mojom::AudioOutputStreamProviderPtr provider_ptr;
+  StrictMock<MockDeleter> deleter;
+
+  // Freed by deleter.
+  auto* provider = new MojoAudioOutputStreamProvider(
+      mojo::MakeRequest(&provider_ptr), base::BindOnce(&CreateFakeDelegate),
+      deleter.Get(), std::make_unique<FakeObserver>());
+
+  mojom::AudioOutputStreamPtr stream_1;
+  mojom::AudioOutputStreamClientPtr client_1;
+  mojom::AudioOutputStreamClientRequest client_request_1 =
+      mojo::MakeRequest(&client_1);
+
+  mojom::AudioOutputStreamPtr stream_2;
+  mojom::AudioOutputStreamClientPtr client_2;
+  mojom::AudioOutputStreamClientRequest client_request_2 =
+      mojo::MakeRequest(&client_2);
+  provider_ptr->Acquire(mojo::MakeRequest(&stream_1), std::move(client_1),
+                        media::AudioParameters::UnavailableDeviceParams(),
+                        base::BindOnce(&FakeAcquireCallback));
+  provider_ptr->Acquire(mojo::MakeRequest(&stream_2), std::move(client_2),
+                        media::AudioParameters::UnavailableDeviceParams(),
+                        base::BindOnce(&FakeAcquireCallback));
+
+  EXPECT_CALL(deleter, Run(provider)).WillOnce(DeleteArg<0>());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(got_bad_message);
+  Mock::VerifyAndClear(&deleter);
+}
+
+TEST(MojoAudioOutputStreamProviderTest,
+     Bitstream_BadMessageOnNonAndoirdPlatforms) {
+  base::MessageLoop loop;
+  bool got_bad_message = false;
+  mojo::edk::SetDefaultProcessErrorCallback(
+      base::BindRepeating([](bool* got_bad_message,
+                             const std::string& s) { *got_bad_message = true; },
+                          &got_bad_message));
+
+  mojom::AudioOutputStreamProviderPtr provider_ptr;
+  StrictMock<MockDeleter> deleter;
+  media::AudioParameters params =
+      media::AudioParameters::UnavailableDeviceParams();
+  params.set_format(AudioParameters::AUDIO_BITSTREAM_AC3);
+
+  auto* provider = new MojoAudioOutputStreamProvider(
+      mojo::MakeRequest(&provider_ptr), base::BindOnce(&CreateFakeDelegate),
+      deleter.Get(), std::make_unique<FakeObserver>());
+
+  mojom::AudioOutputStreamPtr stream;
+  mojom::AudioOutputStreamClientPtr client;
+  mojom::AudioOutputStreamClientRequest client_request =
+      mojo::MakeRequest(&client);
+  provider_ptr->Acquire(mojo::MakeRequest(&stream), std::move(client), params,
+                        base::BindOnce(&FakeAcquireCallback));
+
+#if defined(OS_ANDROID)
+  base::RunLoop().RunUntilIdle();
+  // Creating bitstream streams is allowed on Android.
+  EXPECT_FALSE(got_bad_message);
+  // |deleter| shouldn't have been called, so delete manually.
+  Mock::VerifyAndClear(&deleter);
+  delete provider;
+#else
+  EXPECT_CALL(deleter, Run(provider)).WillOnce(DeleteArg<0>());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(got_bad_message);
+  Mock::VerifyAndClear(&deleter);
+#endif
+}
+
+}  // namespace media

tools/metrics/histograms/enums.xml

@@ -2847,6 +2847,8 @@ uploading your change for review.  These are checked by presubmit scripts.
   <int value="187" label="RMF_BAD_URL_CACHEABLE_METADATA"/>
   <int value="188" label="RFH_INTERFACE_PROVIDER_MISSING"/>
   <int value="189" label="RFH_INTERFACE_PROVIDER_SUPERFLUOUS"/>
+  <int value="190" label="AIRH_UNEXPECTED_BITSTREAM"/>
+  <int value="191" label="ARH_UNEXPECTED_BITSTREAM"/>
 </enum>
 
 <enum name="BadMessageReasonExtensions">