Stop using HMAC for gpu mailboxes

For a long time, we've been using HMAC to sign the mailbox names on the GPU
process. However, we don't actually need the mailbox names to be guaranteed to
be generated by the GPU process, we only need to ensure they're unguessable.

The HMAC signature and verification has several costs:
- the cost to sign the mailbox
- the cost to verify the mailbox
- the initialization cost (e.g. starting NSS in the GPU process)

We can actually save all that cost, by just making the mailbox names
(crypto-)random.

BUG=311259

Review URL: https://codereview.chromium.org/157243002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@249922 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 24 insertions, 55 deletions

diff --git a/content/common/gpu/gpu_channel.cc b/content/common/gpu/gpu_channel.cc
index 7ce8d30..6ddebc4 100644
--- a/content/common/gpu/gpu_channel.cc
+++ b/content/common/gpu/gpu_channel.cc
@@ -15,7 +15,6 @@
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/message_loop/message_loop_proxy.h"
-#include "base/rand_util.h"
 #include "base/strings/string_util.h"
 #include "base/timer/timer.h"
 #include "content/common/gpu/devtools_gpu_agent.h"
@@ -24,7 +23,7 @@
 #include "content/common/gpu/media/gpu_video_encode_accelerator.h"
 #include "content/common/gpu/sync_point_manager.h"
 #include "content/public/common/content_switches.h"
-#include "crypto/hmac.h"
+#include "crypto/random.h"
 #include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/command_buffer/service/gpu_scheduler.h"
 #include "gpu/command_buffer/service/image_manager.h"
@@ -75,8 +74,7 @@ const int64 kStopPreemptThresholdMs = kVsyncIntervalMs;
 class GpuChannelMessageFilter : public IPC::ChannelProxy::MessageFilter {
  public:
   // Takes ownership of gpu_channel (see below).
-  GpuChannelMessageFilter(const std::string& private_key,
-                          base::WeakPtr<GpuChannel>* gpu_channel,
+  GpuChannelMessageFilter(base::WeakPtr<GpuChannel>* gpu_channel,
                           scoped_refptr<SyncPointManager> sync_point_manager,
                           scoped_refptr<base::MessageLoopProxy> message_loop)
       : preemption_state_(IDLE),
@@ -85,10 +83,7 @@ class GpuChannelMessageFilter : public IPC::ChannelProxy::MessageFilter {
         sync_point_manager_(sync_point_manager),
         message_loop_(message_loop),
         messages_forwarded_to_channel_(0),
-        a_stub_is_descheduled_(false),
-        hmac_(crypto::HMAC::SHA256) {
-    bool success = hmac_.Init(base::StringPiece(private_key));
-    DCHECK(success);
+        a_stub_is_descheduled_(false) {
   }
 
   virtual void OnFilterAdded(IPC::Channel* channel) OVERRIDE {
@@ -179,18 +174,8 @@ class GpuChannelMessageFilter : public IPC::ChannelProxy::MessageFilter {
 
     result->resize(num);
 
-    for (unsigned i = 0; i < num; ++i) {
-      char name[GL_MAILBOX_SIZE_CHROMIUM];
-      base::RandBytes(name, sizeof(name) / 2);
-
-      bool success = hmac_.Sign(
-          base::StringPiece(name, sizeof(name) / 2),
-          reinterpret_cast<unsigned char*>(name) + sizeof(name) / 2,
-          sizeof(name) / 2);
-      DCHECK(success);
-
-      (*result)[i].SetName(reinterpret_cast<int8*>(name));
-    }
+    for (unsigned i = 0; i < num; ++i)
+      crypto::RandBytes((*result)[i].name, sizeof((*result)[i].name));
   }
 
   void OnGenerateMailboxNamesAsync(unsigned num) {
@@ -423,8 +408,6 @@ class GpuChannelMessageFilter : public IPC::ChannelProxy::MessageFilter {
   base::OneShotTimer<GpuChannelMessageFilter> timer_;
 
   bool a_stub_is_descheduled_;
-
-  crypto::HMAC hmac_;
 };
 
 GpuChannel::GpuChannel(GpuChannelManager* gpu_channel_manager,
@@ -474,7 +457,6 @@ bool GpuChannel::Init(base::MessageLoopProxy* io_message_loop,
       weak_factory_.GetWeakPtr()));
 
   filter_ = new GpuChannelMessageFilter(
-      mailbox_manager_->private_key(),
       weak_ptr,
       gpu_channel_manager_->sync_point_manager(),
       base::MessageLoopProxy::current());
diff --git a/content/common/gpu/gpu_command_buffer_stub.cc b/content/common/gpu/gpu_command_buffer_stub.cc
index ad048c6..13b7b22 100644
--- a/content/common/gpu/gpu_command_buffer_stub.cc
+++ b/content/common/gpu/gpu_command_buffer_stub.cc
@@ -608,11 +608,12 @@ void GpuCommandBufferStub::OnSetGetBuffer(int32 shm_id,
 
 void GpuCommandBufferStub::OnProduceFrontBuffer(const gpu::Mailbox& mailbox) {
   TRACE_EVENT0("gpu", "GpuCommandBufferStub::OnProduceFrontBuffer");
-  if (!decoder_)
+  if (!decoder_) {
     LOG(ERROR) << "Can't produce front buffer before initialization.";
+    return;
+  }
 
-  if (!decoder_->ProduceFrontBuffer(mailbox))
-    LOG(ERROR) << "Failed to produce front buffer.";
+  decoder_->ProduceFrontBuffer(mailbox);
 }
 
 void GpuCommandBufferStub::OnGetState(IPC::Message* reply_message) {
diff --git a/content/common/gpu/texture_image_transport_surface.cc b/content/common/gpu/texture_image_transport_surface.cc
index e1db18a..f13a986 100644
--- a/content/common/gpu/texture_image_transport_surface.cc
+++ b/content/common/gpu/texture_image_transport_surface.cc
@@ -15,15 +15,16 @@
 #include "content/public/common/content_switches.h"
 #include "gpu/command_buffer/service/context_group.h"
 #include "gpu/command_buffer/service/gpu_scheduler.h"
+#include "gpu/command_buffer/service/mailbox_manager.h"
 #include "ui/gl/scoped_binders.h"
 
 using gpu::gles2::ContextGroup;
 using gpu::gles2::GLES2Decoder;
 using gpu::gles2::MailboxManager;
-using gpu::gles2::MailboxName;
 using gpu::gles2::Texture;
 using gpu::gles2::TextureManager;
 using gpu::gles2::TextureRef;
+using gpu::Mailbox;
 
 namespace content {
 namespace {
@@ -218,8 +219,8 @@ bool TextureImageTransportSurface::SwapBuffers() {
   params.size = backbuffer_size();
   params.scale_factor = scale_factor_;
   params.mailbox_name.assign(
-      reinterpret_cast<const char*>(&back_mailbox_name_),
-      sizeof(back_mailbox_name_));
+      reinterpret_cast<const char*>(&back_mailbox_),
+      sizeof(back_mailbox_));
 
   glFlush();
 
@@ -258,8 +259,8 @@ bool TextureImageTransportSurface::PostSubBuffer(
   params.width = width;
   params.height = height;
   params.mailbox_name.assign(
-      reinterpret_cast<const char*>(&back_mailbox_name_),
-      sizeof(back_mailbox_name_));
+      reinterpret_cast<const char*>(&back_mailbox_),
+      sizeof(back_mailbox_));
 
   glFlush();
 
@@ -323,7 +324,7 @@ void TextureImageTransportSurface::BufferPresentedImpl(
   if (!mailbox_name.empty()) {
     DCHECK(mailbox_name.length() == GL_MAILBOX_SIZE_CHROMIUM);
     if (!memcmp(mailbox_name.data(),
-                &back_mailbox_name_,
+                &back_mailbox_,
                 mailbox_name.length())) {
       // The browser has skipped the frame to unblock the GPU process, waiting
       // for one of the right size, and returned the back buffer, so don't swap.
@@ -332,7 +333,7 @@ void TextureImageTransportSurface::BufferPresentedImpl(
   }
   if (swap) {
     std::swap(backbuffer_, frontbuffer_);
-    std::swap(back_mailbox_name_, front_mailbox_name_);
+    std::swap(back_mailbox_, front_mailbox_);
   }
 
   // We're relying on the fact that the parent context is
@@ -362,7 +363,7 @@ void TextureImageTransportSurface::OnResizeViewACK() {
 void TextureImageTransportSurface::ReleaseBackTexture() {
   DCHECK(IsContextValid(helper_.get()));
   backbuffer_ = NULL;
-  back_mailbox_name_ = MailboxName();
+  back_mailbox_ = Mailbox();
   glFlush();
   CHECK_GL_ERROR();
 }
@@ -370,7 +371,7 @@ void TextureImageTransportSurface::ReleaseBackTexture() {
 void TextureImageTransportSurface::ReleaseFrontTexture() {
   DCHECK(IsContextValid(helper_.get()));
   frontbuffer_ = NULL;
-  front_mailbox_name_ = MailboxName();
+  front_mailbox_ = Mailbox();
   glFlush();
   CHECK_GL_ERROR();
   helper_->SendAcceleratedSurfaceRelease();
@@ -391,15 +392,13 @@ void TextureImageTransportSurface::CreateBackTexture() {
   TextureManager* texture_manager =
       decoder->GetContextGroup()->texture_manager();
   if (!backbuffer_.get()) {
-    mailbox_manager_->GenerateMailboxName(&back_mailbox_name_);
+    mailbox_manager_->GenerateMailbox(&back_mailbox_);
     GLuint service_id;
     glGenTextures(1, &service_id);
     backbuffer_ = TextureRef::Create(texture_manager, 0, service_id);
     texture_manager->SetTarget(backbuffer_.get(), GL_TEXTURE_2D);
     Texture* texture = texture_manager->Produce(backbuffer_.get());
-    bool success = mailbox_manager_->ProduceTexture(
-        GL_TEXTURE_2D, back_mailbox_name_, texture);
-    DCHECK(success);
+    mailbox_manager_->ProduceTexture(GL_TEXTURE_2D, back_mailbox_, texture);
   }
 
   {
diff --git a/content/common/gpu/texture_image_transport_surface.h b/content/common/gpu/texture_image_transport_surface.h
index 2162ffe..ab3eb44 100644
--- a/content/common/gpu/texture_image_transport_surface.h
+++ b/content/common/gpu/texture_image_transport_surface.h
@@ -11,7 +11,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "content/common/gpu/gpu_command_buffer_stub.h"
 #include "content/common/gpu/image_transport_surface.h"
-#include "gpu/command_buffer/service/mailbox_manager.h"
+#include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/command_buffer/service/texture_manager.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_surface.h"
@@ -86,8 +86,8 @@ class TextureImageTransportSurface
 
   // The mailbox name for the current backbuffer texture. Needs to be unique per
   // GL texture and is invalid while service_id is zero.
-  gpu::gles2::MailboxName back_mailbox_name_;
-  gpu::gles2::MailboxName front_mailbox_name_;
+  gpu::Mailbox back_mailbox_;
+  gpu::Mailbox front_mailbox_;
 
   // The current size of the GLSurface. Used to disambiguate from the current
   // texture size which might be outdated (since we use two buffers).
diff --git a/content/gpu/gpu_main.cc b/content/gpu/gpu_main.cc
index 713b337..fdc7de4 100644
--- a/content/gpu/gpu_main.cc
+++ b/content/gpu/gpu_main.cc
@@ -29,7 +29,6 @@
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/main_function_params.h"
-#include "crypto/hmac.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "gpu/config/gpu_info_collector.h"
 #include "gpu/config/gpu_util.h"
@@ -385,18 +384,6 @@ bool WarmUpSandbox(const CommandLine& command_line) {
     // platforms.
     (void) base::RandUint64();
   }
-  {
-    TRACE_EVENT0("gpu", "Warm up HMAC");
-    // Warm up the crypto subsystem, which needs to done pre-sandbox on all
-    // platforms.
-    crypto::HMAC hmac(crypto::HMAC::SHA256);
-    unsigned char key = '\0';
-    if (!hmac.Init(&key, sizeof(key))) {
-      LOG(ERROR) << "WarmUpSandbox() failed with crypto::HMAC::Init()";
-      return false;
-    }
-  }
-
   return true;
 }