#4245 closed defect (fixed)
Browse: show a error if ssl certificate is wrong
Reported by: | godiard | Owned by: | humitos |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | Browse | Version: | Unspecified |
Severity: | Unspecified | Keywords: | r+, olpc-test-passed |
Cc: | humitos, greenfeld, dsd | Distribution/OS: | Unspecified |
Bug Status: | Unconfirmed |
Description
See more info in techteam
Attachments (3)
Change History (20)
comment:1 Changed 10 years ago by humitos
- Owner changed from manuq to humitos
- Status changed from new to accepted
comment:2 follow-up: ↓ 3 Changed 10 years ago by dsd
- Cc dsd added
comment:3 in reply to: ↑ 2 Changed 10 years ago by humitos
I vote for the 2nd option. We have often applied a "if in doubt, do what epiphany does" mindset to Browse, and I think this is again valid here.
Well, Epiphany shows the broken lock but when you click on it Epiphany shows a dialogue with all the information of the certificate. I understand that we don't want to show that dialogue at this time on the cycle.
comment:4 follow-up: ↓ 5 Changed 10 years ago by dsd
Is that how epiphany behaves on the XO? Or did you test on some other setup?
comment:5 in reply to: ↑ 4 Changed 10 years ago by humitos
Replying to dsd:
Is that how epiphany behaves on the XO? Or did you test on some other setup?
Oh, sorry. I forgot to mention that. It's on the XO-4 13.1.0 build 14.
comment:6 Changed 10 years ago by greenfeld
- Cc greenfeld added; Cerlyn removed
comment:7 Changed 10 years ago by manuq
The attached patch tries to do the second option, in the same way Epiphany does here. However from Python soup_message_get_https_status is always returning False for the first output, which means is not using https. I have left the logging calls to show it. My test URLs were:
- https://bugzilla.gnome.org/ -> should show a lock icon
- https://mail.google.com -> should show a lock icon too
- https://activation.laptop.org -> should show a broken lock icon
- Browse homepage -> should not show any icon on the left side of the URL entry
comment:8 Changed 10 years ago by manuq
Oh and the change I did in the Soup session is based on this:
http://git.gnome.org/browse/epiphany/tree/embed/ephy-embed-single.c#n457
comment:9 Changed 10 years ago by manuq
- Keywords r? added
comment:10 Changed 10 years ago by manuq
Sorry the patch is working on os15, please test.
comment:11 Changed 10 years ago by manuq
Or maybe wait until I update it to work when switching tabs.
comment:12 Changed 10 years ago by humitos
- Keywords r- added; r? removed
Well, the patch looks good and works like it says. I checked the sites that are mentioned on this ticket and some others as well, like Facebook, PayPal and some SSL cert sellers. It shows a broken lock when the SSL is invalid for the site visited and show a lock when the SSL is valid.
There is just a little issue in the log file when a PDF tab is opened:
Traceback (most recent call last): File "/home/olpc/Activities/Browse.activity/webtoolbar.py", line 352, in __switch_page_cb self._connect_to_browser(tabbed_view.props.current_browser) File "/home/olpc/Activities/Browse.activity/webtoolbar.py", line 369, in _connect_to_browser self._set_security_status(self._browser.security_status) AttributeError: 'DummyBrowser' object has no attribute 'security_status' /home/olpc/Activities/Browse.activity/webtoolbar.py:356: Warning: gsignal.c:2576: instance `0x12fa730' has no handler with id `15343' self._browser.disconnect(self._uri_changed_hid)
I know that we discussed about this yesterday and we want a simple solution similar how Epiphany works but I'd like to add two comments on this patch to consider them in the future:
- ssl-use-system-ca-file: setting this property to True we are trusting on the ca-files that we have on our system. In some way this could be dangerous if the user has an invalid CA file on his system.
- widget.get_main_frame(): we are checking the certificate just for the main frame. So, if the main frame has a valid certificate we are saying that you are in a trusted site when that is not totally true because the "real website" could be inside an iframe or doing something weird via javascript.
I'd say, fix the traceback issue, push this patch and go back later in the future with the best solution considering all the possibilities.
comment:13 Changed 10 years ago by manuq
- Keywords r? added; r- removed
Thanks for the review, updated PDF DummyBrowser and updated the patch.
comment:14 Changed 10 years ago by humitos
- Keywords r+ added; r? removed
Good work manuq! It's everything ok. Go ahead pushing it! :D
comment:15 Changed 10 years ago by manuq
- Keywords olpc-test-pending added
- Resolution set to fixed
- Status changed from accepted to closed
Thanks, pushed dcb5f998 .
comment:16 Changed 10 years ago by greenfeld
- Keywords olpc-test-passed added; olpc-test-pending removed
We show a lock icon in Browse-148 indicating the status of the currently shown main frame, but are no better than Epiphany and at least a few other WebKitGTK browsers in a bunch of not-so-edge cases. Bug reports about these various issues are already filed elsewhere, and have been for a while.
We are waiting to hear back to see if there is consensus about how to universally fix this across all the various WebKitGTK browsers.
After some discussion it seems like webkit/webkitgtk does not present the API to allow us to do a really complete job here (pop up warning messages, allow users to confirm certificate exceptions, apply these considerations to embedded content as well as the main page, and whatever else).
The two immediate options we have here are:
If I missed any options that don't involve writing loads of code and modifying webkit, etc, please post them here.
I vote for the 2nd option. We have often applied a "if in doubt, do what epiphany does" mindset to Browse, and I think this is again valid here.