-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Safari does not save e2e keys #12207
Comments
@vit9696, thanks for the report. Are you using Riot inside a regular or private window with desktop Safari? Please submit debug logs (top left menu -> Settings -> Help & About -> Submit debug logs) just after verifying and reference this issue so we can investigate further. |
Hi @jryans, this is a regular tab. Debug logs may contain sensitive data, so I may not be able to provide them. At least unless I can directly control what is sent. If you believe there is some particular algorithm to get the relevant information for you, I should try to follow. |
The debug logs contain all messages logged to the browser's console, so you can use that to preview what would be sent. It's a bit hard for me to guess how to proceed without seeing the logs... If you don't want to send them the normal way, you could review the browser console to sanitise them first. |
@jryans, thank you for understanding. Sure, I will include the messages printed to debug console when I reach the computer. |
Ok, here you are: Logs
The issue seems to be related to UPD: Put the logs into details tag. |
@vit9696 Thanks for the logs, it looks like indeed the browser is blocking writes to IndexedDB. Some browsers limit web storage based on free disk space you have left, are you low on disk space? If you are able to backup your keys either using key backup or key export, the fastest fix might be to login in a separate session where you should start fresh. |
@jryans I have 240 and 840 gigabytes on two affected machines, so it is definitely not the case. I get it that I can "hack" it this way, but it will be nicer if there is a proper fix. Let me know whether I can help you anyhow and/or you plan to continue exploring this. |
Just in case, I googled whether I can indexDB current size. With the help of the script from here I was able to get the following:
I do not think this is too much. |
Hmm, I agree that's pretty small. We'll need to construct some new investigation theories here. |
@jryans while I am very far from web development, I was able to find this mention of the WebKit behaviour we were able to observe in the logs. It says that WebKit can terminate IndexDB transaction at idle state, and this can apparently be confirmed by the implementation. From what I understood from your code, the client initially performs a connection, and then sends data through a promise, which can therefore happen much later. In case I understood this properly, this seems to be similar to the error described on stackoverflow. Hope that helps. |
@turt2live could you please provide an insight why does the issue have lower priority now? Is Safari not a supported browser for Riot and I should therefore either constantly reset data storage (which in fact may not even work) or switch to another browser to use Riot? I do not want to sound noisy, but according to your policy this looks like a release-blocking issue. It is a regression and it makes the client in Safari basically unusable. |
So far, there is no evidence of a regression in Riot causing this issue, and you appear to be the only one affected, so we'll need to discover somehow what has happened on your system to cause this. I agree it is quite a bad experience for you. Browsers are known to break IndexedDB storage in a variety of hard to detect ways. At the moment, no one on the Riot core team can reproduce this issue, so that makes it quite hard to confidently investigate and fix. |
Hrm, that is a problem, as it happened on two separate machines independently at about the same time for myself, and for one machine for my friend. I think it may have to do with the amount of messages, so it is possible that the core team does not use Safari actively enough to trigger this. In any case, if the information I provided above does not make much sense, I guess we may have to wait for somebody from the core team to reproduce this. |
Aha, I am able to reproduce now! 🎉 I'll see what can be done here. |
This ensures we wait until after the device list writes to the crypto store before marking thing as clean. This is particularly important for the error path, as the write to the crypto store can fail. Part of element-hq/element-web#12207
At least on Safari but perhaps other browsers as well, you must perform IndexedDB operations in the same JS task as you start the transaction. As a concrete example, you cannot open the transaction and await some promise before actually using it. This fixes the crypto store to meet this requirement. Fixes element-hq/element-web#12207
At least on Safari but perhaps other browsers as well, you must perform IndexedDB operations in the same JS task as you start the transaction. As a concrete example, you cannot open the transaction and await some promise before actually using it. This fixes the crypto store to meet this requirement. Fixes element-hq/element-web#12207
@vit9696 This should now be fixed on https://riot.im/develop. It would be great if you could test and verify whether it works for you. Please note that in my own testing, even after updating Riot, Safari remained "confused" about storage until I signed out of Riot (which clears local data) and signed back in. Please ensure you have a backup of your keys before doing this. After a possible one-time sign out and in cycle, it should continue to work from there into the future. |
Thanks a lot for the fix, currently I am trying to login in develop, but it seems to be quite a problem. I set up key backup, and then was prompted with a weird message right after login, which took quite a lot of time in the first place. After clicking
it started to tiny XHR queries to the server which take 30+ seconds to complete most of the cases. It was way over 10 minutes, so I reloaded the page and pressed skip. Afterwards it once again insisted on the verification:
And basically the story repeats, dozens of minutes of XHR requests. Is it normal? I am not sure I can verify the bugfix, if it takes infinite time to verify. I also cannot understand where legacy verification has gone now, as I normally verify all my clients by recording their e2e keys, and now it suggests me to initiate a handshake with all the partners, which obviously is unacceptable due to different timezones, devices, and the amount of users. |
Ah hmm, I see. Yes, at the moment https://riot.im/develop has force-enabled cross-signing, which will replace the device-by-device verification process, but it is still a work in progress with various bugs to fix before it's deployed to everyone. If you're interested, I could create an ad-hoc testing location tomorrow to try out this fix without the confusion of cross-signing also appearing, or else you could wait until our next release in ~2 weeks. As for the various issues you encountered, so do sound like bugs with the new cross-signing path, and we'd be grateful for your help in reporting them as new issues and submitting / pasting debug logs for analysis.
That certainly does not sound expected, but we'd need to see log data to work what's going on.
Cross-signing (the system of verifying users once instead of each of their devices) is meant to replace device verification for most users. There's no requirement to verify each person, but for those who do wish to verify people in a room, they can do so by verifying each person only once, and then each person is responsible for verifying each of their own devices. A chain of signature verifications ensures that new devices are from the right person. There will be more content explaining the background, details, etc. as we get closer to releasing this. |
@jryans, I am mostly aware of what cross-signing is, but thanks for extra clarification. Regarding the 2 hour "verification" process, I realised that the issue with it is that "Next" button for some reason is consistently broken for me (despite pressing multiple times with the mouse). Once I pressed Enter on the keyboard, it performed the upgrade with no visible issues. Currently my issue is that the suggested test entirely broke my ability to communicate with the desktop clients. Previously I could have just avoided reloading the page and compare few of my contacts' e2e keys if I accidentally did that (not a big issue). After this test I got the following:
Hopefully this provides you some feedback of what develop version actually is. My only option to make this at least somewhat useable was to remove all four Safari desktop keys from my account and restrict my communication to mobile devices for the time being. To continue to use riot on desktop devices I will unavoidably have to create new keys and verify these keys with all the devices of all my contacts. Since my primary reason for trying to fix Safari was primarily preserving my keys and verifications, and the bugfix required a logout with a permanent breakage of the identification, right now I could just install the Electron client. Still thank you for being ready to setup an adhoc instance of the fixed web version. Putting this aside, I want to ask whether the future of client verification will involve the requirement to verify anything at the same time (like comparing session emojis) between two users or two clients of the same user? For me it is unacceptable even if I ignore the need to somehow convert images to some kind of unambiguous text. |
Ah okay, this was #12560 which should be fixed now on develop.
If you expand the user's session list in an encrypted room, you should be able to attempt a legacy session-specific verification by clicking an unverified session. However, in my testing, it's not quite working yet, so I filed #12586.
That's expected, as the cross-signing work for Riot iOS is only present in TestFlight builds. Sorry for all of this pain, but thanks in any case for your testing and feedback. 😅
At the moment, we believe most people will prefer the approach of verifying each user one time via emojis or QR codes, but it's sounding like that won't work for you. I would encourage you to file a new issue explaining your use case in more detail to help us understand your needs. |
Thanks for addressing my issues and filing bugs. QR codes or similar things do not work for me for sure, so I filed #12589 as requested. |
Description
Since recently (most likely within a month) Safari stopped saving other contacts' keys after a successful legacy verification. I.e. refreshing a tab button shows one of the contact's devices unverified right after it was verified. All previously verified devices show fine.
Steps to reproduce
I expect the verified device to stay as verified. To keep in mind, another user had his e2e own key reset every time he relogined in riot web, also Safari and also since recently, this might be the same issue of select settings not saving in Safari anymore. iOS version has no such issue. Resetting caches in preferences does not affect the problem.
Logs being sent: yes/no
Version information
For the web app:
The text was updated successfully, but these errors were encountered: