GNOME Bugzilla – Bug 771888
mask nickserv identify passwords
Last modified: 2021-06-10 11:05:56 UTC
Its nice that polari remembers the conversation with each user. This is also a problem when it saves the chats with nickserv. I see identify mypass in the history of Nickserv chat. It shall be nice to mask those passwords. Say if I do "/msg nickserv identify mypass" What I see in Nickserv private chat window should be "identify ******" (of course replacing * with the usual char for passwords, which is a smaller filled circle or a bigger \cdot (what ever it is called)). Thanks
Makes sense to me
Created attachment 336176 [details] [review] util: Split out match_identify_message() method Matching an identify command is currently done by the room to emit the ::identify-sent signal when the user entered the command, so we can offer to save the password. We now want to mask the sent password in the chat log, so split out the matching code into a utility method.
Created attachment 336177 [details] [review] chatView: Mask identify passwords in private chats We now send automatic identify messages when connecting, so the sensitive password regularly appears in the NickServ conversation where it is potentially subject to shoulder surfing. Litigate this risk by masking the password part of identify messages.
Created attachment 336178 [details] [review] chatView: Be a bit silly on April 1st Happy Easter everyone! Who is saying that passwords need to be masked with boring dots all the time?
Review of attachment 336176 [details] [review]: looks good to me, one question: ::: src/lib/polari-util.c @@ +60,3 @@ + static GRegex *identify_message_regex = NULL; + GMatchInfo *match; + char *text, *stripped_text; i see match and text is freed at the end of this function, where is stripped_text freed?
Review of attachment 336177 [details] [review]: looks good to me.
Review of attachment 336178 [details] [review]: i did this in a game once and got my account stolen :x
(In reply to Bastian Ilsø from comment #5) > i see match and text is freed at the end of this function, where is > stripped_text freed? It's not. Or rather: The function (macro actually) doesn't (re)allocate any memory, so text and stripped_text refer to the same memory that only needs to be freed once.
Review of attachment 336176 [details] [review]: (In reply to Florian Müllner from comment #8) > (In reply to Bastian Ilsø from comment #5) > > i see match and text is freed at the end of this function, where is > > stripped_text freed? > > It's not. Or rather: The function (macro actually) doesn't (re)allocate any > memory, so text and stripped_text refer to the same memory that only needs > to be freed once. looks good to me then
Attachment 336176 [details] pushed as 06dafa1 - util: Split out match_identify_message() method Attachment 336177 [details] pushed as 3c7cb62 - chatView: Mask identify passwords in private chats
The messages from nickserv in notification still reveals the real password. Hope that this is already known and that's the reason why this bug isn't yet marked as closed. Thanks.
(In reply to Mohammed Sadiq from comment #11) > Hope that this is already known and that's the reason why this bug isn't yet > marked as closed. It's known, but it's not the reason the bug hasn't been closed - the notification is not implemented by Polari at all. Those are provided by gnome-shell's built-in chat integration, which is generic for all telepathy one-to-one conversations. I'm afraid there's no intention of special-casing IRC/NickServ in gnome-shell.
(In reply to Florian Müllner from comment #12) > (In reply to Mohammed Sadiq from comment #11) > > Hope that this is already known and that's the reason why this bug isn't yet > > marked as closed. > > It's known, but it's not the reason the bug hasn't been closed - the > notification is not implemented by Polari at all. Those are provided by > gnome-shell's built-in chat integration, which is generic for all telepathy > one-to-one conversations. I'm afraid there's no intention of special-casing > IRC/NickServ in gnome-shell. Well, that's bad then. Passwords should never be visible as plain text, inside a system notification, that just pops up. Is there nothing that can be made to hide or blacklist it?
This patch is not working for me, my passwords are not masked and doing some research it is due to your regex pattern: `"^identify (?:(\\w+) )?(\\S+)$"` You are guessing the user name must match `\w+` but my user name is `alexander-alzate` which doesn't match. so I suggest using this pattern instead: `"^identify (?:(\\S+) )?(\\S+)$"` We know for sure that a user name should not contain spaces.
Created attachment 347210 [details] [review] Fix patch proposed for bug 771888
Review of attachment 347210 [details] [review]: Thanks for the patch! Please provide a proper commit message, following https://chris.beams.io/posts/git-commit/. Also note that the patch will require rebasing after landing bug 778841. ::: src/lib/polari-util.c @@ +68,2 @@ if (G_UNLIKELY (identify_message_regex == NULL)) + identify_message_regex = g_regex_new ("^identify (?:(\\S+) )?(\\S+)$", Not sure it really matters, but the new regex matches strings like $%&"?, which isn't a valid nickname. Still, unlikely to cause any false positives in practice ...
Created attachment 347247 [details] [review] util: Match any nickname in identify command
Comment on attachment 347247 [details] [review] util: Match any nickname in identify command Rebasing bug 778841 and commit message improved.
Comment on attachment 347247 [details] [review] util: Match any nickname in identify command Rebasing bug 778841 commits and commit message improved.
Review of attachment 347247 [details] [review]: "A wide variety" is a bit of a stretch - the legal characters we are currently missing are []\`-^{|} - but OK. Keeping the regex readable certainly beats pointlessly following the spec to the letter.
Comment on attachment 347247 [details] [review] util: Match any nickname in identify command Attachment 347247 [details] pushed as e1d28fa - util: Match any nickname in identify command
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version of Polari, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/polari/-/issues/ Thank you for your understanding and your help.