New issue
Advanced search Search tips

Issue 707833 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Remote assistance fails with mixed-case email

Project Member Reported by jamiewa...@chromium.org, Apr 3 2017

Issue description

Reported externally:

Using Remote Assistance in Chrome Remote Desktop, I used one of  e-mail ID( like  UppeRLoweR@gmail.com) and it does not work.
Remote Assistance works well in other email IDs but not in such email IDs with uppercase-Lowercase combination.

Is this because of this code: https://cs.chromium.org/chromium/src/remoting/signaling/jid_util.cc?q=tolower+file:%5Esrc/remoting/+package:%5Echromium$&dr=C&l=17
 
Owner: ajnolley@chromium.org
AJ, can you try to repro?
From above email link,  https://cs.chromium.org/chromium/src/remoting/signaling/jid_util.cc?q=tolower+file:%5Esrc/remoting/+package:%5Echromium$&dr=C&l=17

actual code is ________________
std::string NormalizeJid(const std::string& jid) {
  std::string bare_jid;
  std::string resource;
  if (SplitJidResource(jid, &bare_jid, &resource)) {
    return base::ToLowerASCII(bare_jid) + "/" + resource;
  }
  return base::ToLowerASCII(bare_jid);
}

----------------------------- after removing base::ToLowerASCII() this method from above code. This bug is resolved. -------------------------

Is there any other consequences if we don't make bare_jid to lowerCase ? I don't see any.

std::string NormalizeJid(const std::string& jid) {
  std::string bare_jid;
  std::string resource;
  if (SplitJidResource(jid, &bare_jid, &resource)) {
    return (bare_jid) + "/" + resource;
  }
  return (bare_jid);
}

Cc: sergeyu@chromium.org
It looks like that code was added in https://codereview.chromium.org/1131653002 to fix  bug 485134 , which is basically the same as this one. I don't think removing it is the right thing to do; maybe whatever comparison this method is being applied to just needs it to be applied to both components.
Owner: ----
Status: Untriaged (was: Unconfirmed)
Was the error message "The access code is invalid. Please try again."? If so, it looks like this is reproducible when sharing from a mixed case email address.
Owner: sergeyu@chromium.org
Status: Assigned (was: Untriaged)
Looks like https://codereview.chromium.org/2577333003 fixed a related problem for mixed case scenario, but now authentication fails with that change on the client side.

Comment 7 Deleted

- In sharing options (Remoting Assitance.exe ), This upper-lower case email id does not work But why does it work with remoting_host.exe(enabling chrome remote desktop ). why do these two thing behaves differently ?
 Is my assumption of having same code(dll) for both sharing(remoting_assitance.exe ) and remote access(remoting_host.exe), Correct ?
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9fe3bc8107285979782a49dbcf0a37cfe0187696

commit 9fe3bc8107285979782a49dbcf0a37cfe0187696
Author: sergeyu <sergeyu@chromium.org>
Date: Tue Apr 11 19:57:12 2017

Use SignalingAddress in SignalStrategy insterface.

Previously SignalStrategy::GetLocalJid() was returning non-normalized
JID. The value was passed as is to the authenticator in
JingleSessionManager::OnSignalStrategyIncomingStanza(). Client used
normalized JID value from SignalingAddress for authentication. As
result authentication was failing for hosts that use mixed-case
accounts. With this change GetLocalJid() is replaced with
GetLocalAddress(), which allows to ensure that JID is normalized on
both ends of connection.

BUG= 707833 

Review-Url: https://codereview.chromium.org/2798393007
Cr-Commit-Position: refs/heads/master@{#463737}

[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/client/chromoting_client.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/client/plugin/chromoting_instance.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/gcd_state_updater.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/gcd_state_updater_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/heartbeat_sender.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/heartbeat_sender_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/host_change_notification_listener.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/host_change_notification_listener_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/host_status_logger_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/it2me/it2me_host_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/it2me/it2me_native_messaging_host.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/register_support_host_request.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/register_support_host_request_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/host/signaling_connector.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/jingle_session.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/jingle_session.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/jingle_session_manager.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/jingle_session_manager.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/jingle_session_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/protocol_mock_objects.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/protocol_mock_objects.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/protocol/session_manager.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/delegating_signal_strategy.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/delegating_signal_strategy.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/fake_signal_strategy.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/fake_signal_strategy.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/iq_sender_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/log_to_server_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/mock_signal_strategy.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/mock_signal_strategy.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/push_notification_subscriber.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/push_notification_subscriber_unittest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/signal_strategy.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/xmpp_signal_strategy.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/signaling/xmpp_signal_strategy.h
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/test/protocol_perftest.cc
[modify] https://crrev.com/9fe3bc8107285979782a49dbcf0a37cfe0187696/remoting/test/test_chromoting_client_unittest.cc

Status: Fixed (was: Assigned)
Sergey, What milestone can I verify this fix in?
Labels: M-59
It's in M59
Status: Verified (was: Fixed)
Verified fixed in  59.0.3071.47

Sign in to add a comment