Replace rtc::CryptString with std::string

CryptString is intended for secret data, such as passwords, and the memory is cleared on deallocation. However, where it is used in libjingle_xmpp, the password is copied to other objects without any magic clearing, so the security benefit is questionable. In addition, CryptString is yet another string class, it's unused within webrtc, and essentially unmaintained. If we can replace its use in Chrome, it will be deleted. BUG=webrtc:6424 Review-Url: https://codereview.chromium.org/2738973004 Cr-Commit-Position: refs/heads/master@{#456021}
FairyWorld · Mar 10, 2017 · 871bffc · 871bffc
1 parent 33aee24
commit 871bffc
Show file tree

Hide file tree

Showing 10 changed files with 27 additions and 35 deletions.
diff --git a/jingle/notifier/base/gaia_token_pre_xmpp_auth.cc b/jingle/notifier/base/gaia_token_pre_xmpp_auth.cc
@@ -65,7 +65,7 @@ GaiaTokenPreXmppAuth::~GaiaTokenPreXmppAuth() { }
 void GaiaTokenPreXmppAuth::StartPreXmppAuth(
     const buzz::Jid& jid,
     const rtc::SocketAddress& server,
-    const rtc::CryptString& pass,
+    const std::string& pass,
     const std::string& auth_mechanism,
     const std::string& auth_token) {
   SignalAuthDone();

diff --git a/jingle/notifier/base/gaia_token_pre_xmpp_auth.h b/jingle/notifier/base/gaia_token_pre_xmpp_auth.h
@@ -29,7 +29,7 @@ class GaiaTokenPreXmppAuth : public buzz::PreXmppAuth {
   // this point.
   void StartPreXmppAuth(const buzz::Jid& jid,
                         const rtc::SocketAddress& server,
-                        const rtc::CryptString& pass,
+                        const std::string& pass,
                         const std::string& auth_mechanism,
                         const std::string& auth_token) override;
 

diff --git a/jingle/notifier/base/xmpp_connection_unittest.cc b/jingle/notifier/base/xmpp_connection_unittest.cc
@@ -30,7 +30,6 @@ class Jid;
 }  // namespace buzz
 
 namespace rtc {
-class CryptString;
 class SocketAddress;
 class Task;
 }  // namespace rtc
@@ -52,7 +51,7 @@ class MockPreXmppAuth : public buzz::PreXmppAuth {
   MOCK_METHOD5(StartPreXmppAuth,
                void(const buzz::Jid&,
                     const rtc::SocketAddress&,
-                    const rtc::CryptString&,
+                    const std::string&,
                     const std::string&,
                     const std::string&));
   MOCK_CONST_METHOD0(IsAuthDone, bool());

diff --git a/third_party/libjingle_xmpp/xmpp/plainsaslhandler.h b/third_party/libjingle_xmpp/xmpp/plainsaslhandler.h
@@ -14,13 +14,12 @@
 #include <algorithm>
 #include "third_party/libjingle_xmpp/xmpp/saslhandler.h"
 #include "third_party/libjingle_xmpp/xmpp/saslplainmechanism.h"
-#include "third_party/webrtc/base/cryptstring.h"
 
 namespace buzz {
 
 class PlainSaslHandler : public SaslHandler {
 public:
-  PlainSaslHandler(const Jid & jid, const rtc::CryptString & password,
+  PlainSaslHandler(const Jid & jid, const std::string & password,
       bool allow_plain) : jid_(jid), password_(password),
                           allow_plain_(allow_plain) {}
 
@@ -54,7 +53,7 @@ class PlainSaslHandler : public SaslHandler {
 
 private:
   Jid jid_;
-  rtc::CryptString password_;
+  std::string password_;
   bool allow_plain_;
 };
 

diff --git a/third_party/libjingle_xmpp/xmpp/prexmppauth.h b/third_party/libjingle_xmpp/xmpp/prexmppauth.h
@@ -12,7 +12,6 @@
 #define WEBRTC_LIBJINGLE_XMPP_PREXMPPAUTH_H_
 
 #include "third_party/libjingle_xmpp/xmpp/saslhandler.h"
-#include "third_party/webrtc/base/cryptstring.h"
 #include "third_party/webrtc/base/sigslot.h"
 
 namespace rtc {
@@ -51,7 +50,7 @@ class PreXmppAuth : public SaslHandler {
   virtual void StartPreXmppAuth(
     const Jid& jid,
     const rtc::SocketAddress& server,
-    const rtc::CryptString& pass,
+    const std::string& pass,
     const std::string& auth_mechanism,
     const std::string& auth_token) = 0;
 

diff --git a/third_party/libjingle_xmpp/xmpp/saslplainmechanism.h b/third_party/libjingle_xmpp/xmpp/saslplainmechanism.h
@@ -12,14 +12,13 @@
 #define WEBRTC_LIBJINGLE_XMPP_SASLPLAINMECHANISM_H_
 
 #include "third_party/libjingle_xmpp/xmpp/saslmechanism.h"
-#include "third_party/webrtc/base/cryptstring.h"
 
 namespace buzz {
 
 class SaslPlainMechanism : public SaslMechanism {
 
 public:
-  SaslPlainMechanism(const buzz::Jid user_jid, const rtc::CryptString & password) :
+  SaslPlainMechanism(const buzz::Jid user_jid, const std::string & password) :
     user_jid_(user_jid), password_(password) {}
 
   virtual std::string GetMechanismName() { return "PLAIN"; }
@@ -29,18 +28,18 @@ class SaslPlainMechanism : public SaslMechanism {
     XmlElement * el = new XmlElement(QN_SASL_AUTH, true);
     el->AddAttr(QN_MECHANISM, "PLAIN");
 
-    rtc::FormatCryptString credential;
-    credential.Append("\0", 1);
-    credential.Append(user_jid_.node());
-    credential.Append("\0", 1);
-    credential.Append(&password_);
-    el->AddText(Base64EncodeFromArray(credential.GetData(), credential.GetLength()));
+    std::stringstream ss;
+    ss.write("\0", 1);
+    ss << user_jid_.node();
+    ss.write("\0", 1);
+    ss << password_;
+    el->AddText(Base64EncodeFromArray(ss.str().data(), ss.str().length()));
     return el;
   }
 
 private:
   Jid user_jid_;
-  rtc::CryptString password_;
+  std::string password_;
 };
 
 }

diff --git a/third_party/libjingle_xmpp/xmpp/xmppclient.cc b/third_party/libjingle_xmpp/xmpp/xmppclient.cc
@@ -50,7 +50,7 @@ class XmppClient::Private :
   std::unique_ptr<AsyncSocket> socket_;
   std::unique_ptr<XmppEngine> engine_;
   std::unique_ptr<PreXmppAuth> pre_auth_;
-  rtc::CryptString pass_;
+  std::string pass_;
   std::string auth_mechanism_;
   std::string auth_token_;
   rtc::SocketAddress server_;
@@ -208,13 +208,13 @@ int XmppClient::ProcessStart() {
     d_->pre_auth_->StartPreXmppAuth(
         d_->engine_->GetUser(), d_->server_, d_->pass_,
         d_->auth_mechanism_, d_->auth_token_);
-    d_->pass_.Clear(); // done with this;
+    d_->pass_.clear(); // done with this;
     return STATE_PRE_XMPP_LOGIN;
   }
   else {
     d_->engine_->SetSaslHandler(new PlainSaslHandler(
               d_->engine_->GetUser(), d_->pass_, d_->allow_plain_));
-    d_->pass_.Clear(); // done with this;
+    d_->pass_.clear(); // done with this;
     return STATE_START_XMPP_LOGIN;
   }
 }

diff --git a/third_party/libjingle_xmpp/xmpp/xmppclientsettings.h b/third_party/libjingle_xmpp/xmpp/xmppclientsettings.h
@@ -13,7 +13,6 @@
 
 #include "third_party/webrtc/p2p/base/port.h"
 #include "third_party/libjingle_xmpp/xmpp/xmppengine.h"
-#include "third_party/webrtc/base/cryptstring.h"
 
 namespace buzz {
 
@@ -26,7 +25,7 @@ class XmppUserSettings {
 
   void set_user(const std::string& user) { user_ = user; }
   void set_host(const std::string& host) { host_ = host; }
-  void set_pass(const rtc::CryptString& pass) { pass_ = pass; }
+  void set_pass(const std::string& pass) { pass_ = pass; }
   void set_auth_token(const std::string& mechanism,
                       const std::string& token) {
     auth_mechanism_ = mechanism;
@@ -44,7 +43,7 @@ class XmppUserSettings {
 
   const std::string& user() const { return user_; }
   const std::string& host() const { return host_; }
-  const rtc::CryptString& pass() const { return pass_; }
+  const std::string& pass() const { return pass_; }
   const std::string& auth_mechanism() const { return auth_mechanism_; }
   const std::string& auth_token() const { return auth_token_; }
   const std::string& resource() const { return resource_; }
@@ -56,7 +55,7 @@ class XmppUserSettings {
  private:
   std::string user_;
   std::string host_;
-  rtc::CryptString pass_;
+  std::string pass_;
   std::string auth_mechanism_;
   std::string auth_token_;
   std::string resource_;
@@ -84,7 +83,7 @@ class XmppClientSettings : public XmppUserSettings {
   void set_proxy_port(int port) { proxy_port_ = port; };
   void set_use_proxy_auth(bool f) { use_proxy_auth_ = f; }
   void set_proxy_user(const std::string& user) { proxy_user_ = user; }
-  void set_proxy_pass(const rtc::CryptString& pass) { proxy_pass_ = pass; }
+  void set_proxy_pass(const std::string& pass) { proxy_pass_ = pass; }
 
   const rtc::SocketAddress& server() const { return server_; }
   cricket::ProtocolType protocol() const { return protocol_; }
@@ -93,7 +92,7 @@ class XmppClientSettings : public XmppUserSettings {
   int proxy_port() const { return proxy_port_; }
   bool use_proxy_auth() const { return use_proxy_auth_; }
   const std::string& proxy_user() const { return proxy_user_; }
-  const rtc::CryptString& proxy_pass() const { return proxy_pass_; }
+  const std::string& proxy_pass() const { return proxy_pass_; }
 
  private:
   rtc::SocketAddress server_;
@@ -103,7 +102,7 @@ class XmppClientSettings : public XmppUserSettings {
   int proxy_port_;
   bool use_proxy_auth_;
   std::string proxy_user_;
-  rtc::CryptString proxy_pass_;
+  std::string proxy_pass_;
 };
 
 }

diff --git a/third_party/libjingle_xmpp/xmpp/xmppengine_unittest.cc b/third_party/libjingle_xmpp/xmpp/xmppengine_unittest.cc
@@ -62,14 +62,13 @@ class XmppEngineTest : public testing::Test {
     handler_.reset(new XmppTestHandler(engine_.get()));
 
     Jid jid("david@my-server");
-    rtc::InsecureCryptStringImpl pass;
-    pass.password() = "david";
+    std::string pass("david");
     engine_->SetSessionHandler(handler_.get());
     engine_->SetOutputHandler(handler_.get());
     engine_->AddStanzaHandler(handler_.get());
     engine_->SetUser(jid);
     engine_->SetSaslHandler(
-        new buzz::PlainSaslHandler(jid, rtc::CryptString(pass), true));
+        new buzz::PlainSaslHandler(jid, pass, true));
   }
   virtual void TearDown() {
     handler_.reset();

diff --git a/third_party/libjingle_xmpp/xmpp/xmpplogintask_unittest.cc b/third_party/libjingle_xmpp/xmpp/xmpplogintask_unittest.cc
@@ -19,7 +19,6 @@
 #include "third_party/libjingle_xmpp/xmpp/saslplainmechanism.h"
 #include "third_party/libjingle_xmpp/xmpp/util_unittest.h"
 #include "third_party/libjingle_xmpp/xmpp/xmppengine.h"
-#include "third_party/webrtc/base/cryptstring.h"
 #include "third_party/webrtc/base/gunit.h"
 #include "third_party/webrtc/typedefs.h"
 
@@ -52,14 +51,13 @@ class XmppLoginTaskTest : public testing::Test {
     handler_.reset(new XmppTestHandler(engine_.get()));
 
     Jid jid("david@my-server");
-    rtc::InsecureCryptStringImpl pass;
-    pass.password() = "david";
+    std::string pass("david");
     engine_->SetSessionHandler(handler_.get());
     engine_->SetOutputHandler(handler_.get());
     engine_->AddStanzaHandler(handler_.get());
     engine_->SetUser(jid);
     engine_->SetSaslHandler(
-        new buzz::PlainSaslHandler(jid, rtc::CryptString(pass), true));
+        new buzz::PlainSaslHandler(jid, pass, true));
   }
   virtual void TearDown() {
     handler_.reset();