From 217bd7931eeaeb277c9d719cf10c9b84d0652b38 Mon Sep 17 00:00:00 2001
From: hai007 <hais@google.com>
Date: Tue, 13 Apr 2021 13:54:18 -0700
Subject: [PATCH] Internal change

PiperOrigin-RevId: 368288116
---
 cpp/core/internal/base_endpoint_channel.cc    | 20 +++++---
 .../internal/base_endpoint_channel_test.cc    | 47 +++++++++++++++++++
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/cpp/core/internal/base_endpoint_channel.cc b/cpp/core/internal/base_endpoint_channel.cc
index b09edd3e..df5c2d59 100644
--- a/cpp/core/internal/base_endpoint_channel.cc
+++ b/cpp/core/internal/base_endpoint_channel.cc
@@ -137,12 +137,20 @@ ExceptionOr<ByteArray> BaseEndpointChannel::Read() {
         // TODO(apolyudov): verify this happens at most once per session.
         result = {};
         auto parsed = parser::FromBytes(ByteArray(input));
-        if (parsed.ok() &&
-            parser::GetFrameType(parsed.result()) == V1Frame::KEEP_ALIVE) {
-          NEARBY_LOGS(INFO)
-              << __func__
-              << ": Read unencrypted KEEP_ALIVE on encrypted channel.";
-          result = ByteArray(input);
+        if (parsed.ok()) {
+          if (parser::GetFrameType(parsed.result()) == V1Frame::KEEP_ALIVE) {
+            NEARBY_LOGS(INFO)
+                << __func__
+                << ": Read unencrypted KEEP_ALIVE on encrypted channel.";
+            result = ByteArray(input);
+          } else {
+            NEARBY_LOGS(WARNING)
+                << __func__ << ": Read unexpected unencrypted frame of type "
+                << parser::GetFrameType(parsed.result());
+          }
+        } else {
+          NEARBY_LOGS(WARNING)
+              << __func__ << ": Unable to parse data as unencrypted message.";
         }
       }
       if (result.Empty()) {
diff --git a/cpp/core/internal/base_endpoint_channel_test.cc b/cpp/core/internal/base_endpoint_channel_test.cc
index 80668a44..e60ae1d3 100644
--- a/cpp/core/internal/base_endpoint_channel_test.cc
+++ b/cpp/core/internal/base_endpoint_channel_test.cc
@@ -17,7 +17,9 @@
 #include <utility>
 
 #include "core/internal/encryption_runner.h"
+#include "core/internal/offline_frames.h"
 #include "platform/base/byte_array.h"
+#include "platform/base/exception.h"
 #include "platform/base/input_stream.h"
 #include "platform/base/output_stream.h"
 #include "platform/public/count_down_latch.h"
@@ -354,6 +356,51 @@ TEST(BaseEndpointChannelTest, ReadAfterInputStreamClosed) {
   ASSERT_TRUE(read_data.GetException().Raised(Exception::kIo));
 }
 
+TEST(BaseEndpointChannelTest, ReadUnencryptedFrameOnEncryptedChannel) {
+  // Setup test communication environment.
+  Pipe pipe_a;  // channel_a writes to pipe_a, reads from pipe_b.
+  Pipe pipe_b;  // channel_b writes to pipe_b, reads from pipe_a.
+  TestEndpointChannel channel_a(&pipe_b.GetInputStream(),
+                                &pipe_a.GetOutputStream());
+  TestEndpointChannel channel_b(&pipe_a.GetInputStream(),
+                                &pipe_b.GetOutputStream());
+
+  ON_CALL(channel_a, GetMedium).WillByDefault([]() {
+    return Medium::BLUETOOTH;
+  });
+  ON_CALL(channel_b, GetMedium).WillByDefault([]() {
+    return Medium::BLUETOOTH;
+  });
+
+  // Run DH key exchange; setup encryption contexts for channels. But only
+  // encrypt |channel_b|.
+  auto [context_a, context_b] = DoDhKeyExchange(&channel_a, &channel_b);
+  ASSERT_NE(context_a, nullptr);
+  ASSERT_NE(context_b, nullptr);
+  channel_b.EnableEncryption(context_b);
+
+  EXPECT_EQ(channel_a.GetType(), "BLUETOOTH");
+  EXPECT_EQ(channel_b.GetType(), "ENCRYPTED_BLUETOOTH");
+
+  // An unencrypted KeepAlive should succeed.
+  ByteArray keep_alive_message = parser::ForKeepAlive();
+  channel_a.Write(keep_alive_message);
+  ExceptionOr<ByteArray> result = channel_b.Read();
+  EXPECT_TRUE(result.ok());
+  EXPECT_EQ(result.result(), keep_alive_message);
+
+  // An unencrypted data frame should fail.
+  ByteArray tx_message{"data message"};
+  channel_a.Write(tx_message);
+  result = channel_b.Read();
+  EXPECT_FALSE(result.ok());
+  EXPECT_EQ(result.exception(), Exception::kInvalidProtocolBuffer);
+
+  // Shutdown test environment.
+  channel_a.Close(DisconnectionReason::LOCAL_DISCONNECTION);
+  channel_b.Close(DisconnectionReason::REMOTE_DISCONNECTION);
+}
+
 }  // namespace
 }  // namespace connections
 }  // namespace nearby