Move HostKeyPair into protocol::KeyPair.

remoting/base/rsa_key_pair_unittest.cc

+// Copyright (c) 2012 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 <string>
+
+#include "remoting/base/rsa_key_pair.h"
+#include "remoting/base/test_rsa_key_pair.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+
+namespace {
+const char kTestMessage[] = "Test Message";
+
+// |kTestMessage| signed with the key from |kTestHostKeyPair|.
+const char kExpectedSignature[] =
+"LfUyXU2AiKM4rpWivUR3bLiQiRt1W3iIenNfJEB8RWyoEfnvSBoD52x8q9yFvtLFDEMPWyIrwM+N2"
+"LuaWBKG1c0R7h+twBgvpExzZneJl+lbGMRx9ba8m/KAFrUWA/NRzOen2NHCuPybOEasgrPgGWBrmf"
+"gDcvyW8QiGuKLopGj/4c5CQT4yE8JjsyU3Qqo2ZPK4neJYQhOmAlg+Q5dAPLpzWMj5HQyOVHJaSXZ"
+"Y8vl/LiKvbdofYLeYNVKAE4q5mfpQMrsysPYpbxBV60AhFyrvtC040MFGcflKQRZNiZwMXVb7DclC"
+"BPgvK7rI5Y0ERtVm+yNmH7vCivfyAnDUYA==";
+}  // namespace
+
+class RsaKeyPairTest : public testing::Test {

[Wez, 2013/03/06 00:43:25]

Why do we need a base class for these tests, since it contains nothing?

[rmsousa, 2013/03/06 04:36:49]

Organization - All unit tests for class X should be in a class named <X>Test.
This way you can run the tests with a filter like <x>Test.*.

+};
+
+TEST_F(RsaKeyPairTest, LoadSaveLoad) {

[Wez, 2013/03/06 00:43:25]

nit: LoadSaveLoad -> ImportExportImport, since load/save implies storing values
somewhere.

[rmsousa, 2013/03/06 04:36:49]

Done.

+  // Load a key to a string, convert to string, load again, and verify that we
+  // generate the same signature with both keys.

[Wez, 2013/03/06 00:43:25]

nit: This comment seems to be out of date; we don't "load a key to a string",
for example?

[rmsousa, 2013/03/06 04:36:49]

Done.

+  scoped_refptr<RsaKeyPair> exported_key = new RsaKeyPair();
+  exported_key->LoadFromString(kTestHostKeyPair);
+  scoped_refptr<RsaKeyPair> imported_key = new RsaKeyPair();
+  imported_key->LoadFromString(exported_key->GetAsString());
+
+  ASSERT_EQ(exported_key->GetSignature(kTestMessage),
+            imported_key->GetSignature(kTestMessage));
+}
+
+TEST_F(RsaKeyPairTest, Signatures) {
+  // Sign a message and check that we get expected signature.

[Wez, 2013/03/06 00:43:25]

nit: ... get the expected ...

[rmsousa, 2013/03/06 04:36:49]

Done.

+  scoped_refptr<RsaKeyPair> key_pair = new RsaKeyPair();
+  key_pair->LoadFromString(kTestHostKeyPair);
+
+  std::string signature_base64 = key_pair->GetSignature(kTestMessage);
+  ASSERT_EQ(signature_base64, std::string(kExpectedSignature));
+}
+
+TEST_F(RsaKeyPairTest, SignaturesDiffer) {
+  // Sign using different keys/messages and check that signatures are different.
+  scoped_refptr<RsaKeyPair> key_pair1 = new RsaKeyPair();
+  key_pair1->Generate();
+  scoped_refptr<RsaKeyPair> key_pair2 = new RsaKeyPair();
+  key_pair2->Generate();

[Wez, 2013/03/06 00:43:25]

nit: Generating key pairs has an infinitesimally small risk of a clash, and in
the real-world it's not cheap - hard-code some keys and test Generate() once in
a separate test?

[rmsousa, 2013/03/06 04:36:49]

Done.

+
+  std::string signature_kp1_msg1_base64 = key_pair1->GetSignature(kTestMessage);
+  std::string signature_kp2_msg1_base64 = key_pair2->GetSignature(kTestMessage);
+  std::string signature_kp1_msg2_base64 = key_pair1->GetSignature("Different");
+  ASSERT_NE(signature_kp1_msg1_base64, signature_kp2_msg1_base64);
+  ASSERT_NE(signature_kp1_msg1_base64, signature_kp1_msg2_base64);

[Wez, 2013/03/06 00:43:25]

nit: May as well also compare 2_1 to 1_2 while you're here. :)

[rmsousa, 2013/03/06 04:36:49]

There's no guarantee we need to assert about signing different messages with
different keys. They could legitimately be the same.

+  ASSERT_NE(key_pair1->GetPublicKey(), key_pair2->GetPublicKey());

[Wez, 2013/03/06 00:43:25]

This check is a requirement for the test to be valid, so it really belongs with
the test init block, above, immediately after the keys are generated.

[rmsousa, 2013/03/06 04:36:49]

The *private keys* being different is the precondition. It's not a given that
the public keys for different private keys are different.

+}
+
+}  // namespace remoting