Whazzup,
I just released the beta of my plugin that implements OpenPGP into Roundcube through JavaScript by using the OpenPGP.js library. Github repo available here https://github.com/qnrq/rc_openpgpjs and announcement here http://qnrq.se/rc_openpgpjs_ending_seven_years_of_roundcube_insecurity/.
Expect a stable release as soon as I have regulated my dopamine levels; or rather, as soon as I have fixed these https://github.com/qnrq/rc_openpgpjs/issues?milestone=1&page=1&state=open minor issues. I'm estimating it'll take /at most/ one or two weeks to finalize the stable version. In the meantime we can handle the political discussions that this may stir up.
Please ennjoy it, mah homiez!
/qnrq
Hi Niklas,
On Monday 07 January 2013 10:52:35 Niklas Femerstrand wrote:
I just released the beta of my plugin that implements OpenPGP into Roundcube through JavaScript by using the OpenPGP.js library.
This is awesome news :)
Thanks for your work! I've been waiting for this for quite some time.
Regards, Torsten
Hi,
nice work. Thank u for that. :)
On 07.01.2013 15:56, Torsten Grote wrote:
Hi Niklas,
On Monday 07 January 2013 10:52:35 Niklas Femerstrand wrote:
I just released the beta of my plugin that implements OpenPGP into Roundcube through JavaScript by using the OpenPGP.js library.
This is awesome news :)
Thanks for your work! I've been waiting for this for quite some time.
Regards, Torsten
Roundcube Development discussion mailing list dev@lists.roundcube.net http://lists.roundcube.net/mailman/listinfo/dev
Niklas Femerstrand wrote:
Whazzup,
I just released the beta of my plugin that implements OpenPGP into Roundcube through JavaScript by using the OpenPGP.js library. Github repo available here https://github.com/qnrq/rc_openpgpjs and announcement here http://qnrq.se/rc_openpgpjs_ending_seven_years_of_roundcube_insecurity/.
Expect a stable release as soon as I have regulated my dopamine levels; or rather, as soon as I have fixed these https://github.com/qnrq/rc_openpgpjs/issues?milestone=1&page=1&state=open minor issues. I'm estimating it'll take /at most/ one or two weeks to finalize the stable version. In the meantime we can handle the political discussions that this may stir up.
Hi Niklas
Thanks for the hard work you put into this! It's good to have somebody finally taking care about the whole PGP topic as we're too busy with other stuff.
After a first sight at your work, I'd like to drop some comments about what I discovered. From both, a developer's as well as a user's point of view, there are some issues which should be addressed:
Development
- You bind events to UI buttons by using their object IDs. This doesn't
work reliably because these are internal IDs and they can vary with different skins. With the new Larry skin, the plugin doesn't work because #rcmbtn114 isn't the send button (but the CC+ button of the address book widget). Solution: there are generic events triggered by the Roundcube core script, which you can bind on [1]. In your case, binding to the 'send' command would look like this:
rcmail.addEventListener('beforesend', function(e){ /* do your magic */ });
- Pubkey selection: $("#_to").val().split(",") doesn't always work. There
might be recipient addresses like this: "Brüderli, Thomas" thomas@roundcube.net. Solution: grep for email address patterns in the recipients list instead. BTW: there are also CC and BCC fields to consider when encrypting.
- Storing the passphrase as cookie: even if the cookie has a short
expiration period, please be aware that cookies might be sent to the server on every HTTP request. I'd suggest to use local storage functions to store the passphrase.
- There's no encryption for attachments. I know, this is hard or almost
impossible to implement on the client but it's a serious lack of feature. We might consider to implement hooks that'll let you read attachment contents back to the client in order to encrypt them.
- Decryption of encrypted messages is only possible on messages sent with
text/plain but not with multipart/encrypted. This is mainly because Roundcube doesn't even send the encrypted body parts to the client. Solution: there are plugin hooks in PHP available [2] to grab encrypted parts from the message structure and make sure they're sent to the client. Attachment decryption is also consider here.
- No decryption in message preview! One has to open the message in full
screen to have the openpgpjs plugin kick in. Just activate the plugin on 'preview' action as well.
- UI elements such as dialogs and forms are fully created in javascript. In
order to also make them customizable using skins, you should use skins and templates and add them to the html page in PHP.
- Texts are not localized but hard-coded in javascript. The Roundcube
plugin system provides easy ways to maintain localized texts. Those strings can also be made available in javascript. See other plugins how this is done.
- Make key-server to lookup keys configurable. See other plugins how plugin
config works.
Usability
- In the message view screen, a button is added to the top tasks bar.
Besides that it doesn't fit in the Larry skin, this isn't the right place to put that button. In the compose screen, it is placed to the toolbar which is much better. However, I'd consider to find the key management in the Settings section.
- There's no progress/activity display while encrypting/decrypting action
is going on. I rekon this sometimes takes a few seconds and as a user, I don't know what's happening.
- The key management dialogs don't have a proper layout. At least in Larry
skin, the import box is aligned right of the table, forcing the table contents to break. The inputs for importing keys could even be hidden in an additional dialog or just placed below the table.
- The order of the dialog tabs (Generate keys, Private keys, Public keys)
should be reversed. Importing public keys is IMO the most common task while generating keys is rarely used.
- Create individual CSS files for different skins in order to better
integrate with the Roundcube UI.
Please don't take the above points as a rant but as a friendly and constructive feedback. We're happy to see progress on this and we're willing to help you making things nice and shiny.
Best regards, Thomas
[1] http://trac.roundcube.net/wiki/Plugin_Events#beforeandafter [2] http://trac.roundcube.net/wiki/Plugin_Hooks#message_part_structure
Heylo!
Don't worry about me taking this personally, I absolutely don't and rather really appreciate the feedback. I am already aware of a lot of the points you raise, my goal was to put in all base crypto stuff before bothering about the RC specifics. A lot of the things are just my quick way of doing them versus spending time referencing RC docs. Doesn't mean that the points shouldn't be repeated and highlighted though :) Also I really appreciate that you link directly to things and give examples, makes my life so much easier!
- Will definitely replace the hard coded triggers. A lot of the things
are very specific currently. I have assigned this as #16 https://github.com/qnrq/rc_openpgpjs/issues/162. Oops! Good eye catching the separator issue, that one could've turned into a nasty future bug. I split this into #17 https://github.com/qnrq/rc_openpgpjs/issues/17 and #18 https://github.com/qnrq/rc_openpgpjs/issues/18. 3. 100% agree, the cookie was just a quick way to make it persistent to spark devt results; assigned #19 https://github.com/qnrq/rc_openpgpjs/issues/19. 4. The OpenPGP.js library has encrypted file attachments on their list https://github.com/openpgpjs/openpgpjs/issues/12, if they're too slow I'll chop something nice up in the future :) 5. I'll have to look closer into multipart/encrypted, assigned #20 https://github.com/qnrq/rc_openpgpjs/issues/20. 6. Hehe, I've added the preview action now. Didn't validate yet, but I assume that it works and if it doesn't the blame is on you ;-) 7. I just merged a fix for the skin template thingy made by lazlolazlolazlo https://github.com/qnrq/rc_openpgpjs/pull/15 8. Localized text also merged in as a fix made by lazlolazlolazlo https://github.com/qnrq/rc_openpgpjs/pull/14 9. I have thought a lot about the key server layout: my problem with PKS is that I don't like it. My plan is to build a PKS into the plugin itself; I want to use HKP http://tools.ietf.org/html/draft-shaw-openpgp-hkp-00 but I don't trust it fully (due to HTTP). There is some code prepared https://github.com/qnrq/rc_openpgpjs/blob/master/rc_openpgpjs.php to extend into a full HKP PKS over HTTPS instead. The missing TLS in HKP isn't my only concern, I'm medidating on the classical issues such as how key expirations would be handled through a third party which replicates. I'm trying to design a decentralized HKP API currently, but I need some more time to contemplate regarding this. 10. I will fix the button so it plays nicely with Roundcube, Alec assigned it as #13 https://github.com/qnrq/rc_openpgpjs/issues/13. 11. Progress activity, or at least a rotator, will be put in place as soon as HTML5 web workers (#1 https://github.com/qnrq/rc_openpgpjs/issues/1) are put in place. 12. I'll check the key manager dialogs, and also re-order the tabs :) 13. lazlolazlolazlo fixed https://github.com/qnrq/rc_openpgpjs/commit/6f55cf75038019d3b2b334c2069281b7eb91fc3c the individual CSS file issue, and it has been merged :)
Thanks a lot for your feedback, hints and pointers. Stay cool!
/N
On 2013-01-12 00:01, Thomas Bruederli wrote:
Niklas Femerstrand wrote:
Whazzup,
I just released the beta of my plugin that implements OpenPGP into Roundcube through JavaScript by using the OpenPGP.js library. Github repo available here https://github.com/qnrq/rc_openpgpjs and announcement here http://qnrq.se/rc_openpgpjs_ending_seven_years_of_roundcube_insecurity/.
Expect a stable release as soon as I have regulated my dopamine levels; or rather, as soon as I have fixed these https://github.com/qnrq/rc_openpgpjs/issues?milestone=1&page=1&state=open minor issues. I'm estimating it'll take /at most/ one or two weeks to finalize the stable version. In the meantime we can handle the political discussions that this may stir up.
Hi Niklas
Thanks for the hard work you put into this! It's good to have somebody finally taking care about the whole PGP topic as we're too busy with other stuff.
After a first sight at your work, I'd like to drop some comments about what I discovered. From both, a developer's as well as a user's point of view, there are some issues which should be addressed:
Development
- You bind events to UI buttons by using their object IDs. This doesn't
work reliably because these are internal IDs and they can vary with different skins. With the new Larry skin, the plugin doesn't work because #rcmbtn114 isn't the send button (but the CC+ button of the address book widget). Solution: there are generic events triggered by the Roundcube core script, which you can bind on [1]. In your case, binding to the 'send' command would look like this:
rcmail.addEventListener('beforesend', function(e){ /* do your magic */ });
- Pubkey selection: $("#_to").val().split(",") doesn't always work. There
might be recipient addresses like this: "Brüderli, Thomas" thomas@roundcube.net. Solution: grep for email address patterns in the recipients list instead. BTW: there are also CC and BCC fields to consider when encrypting.
- Storing the passphrase as cookie: even if the cookie has a short
expiration period, please be aware that cookies might be sent to the server on every HTTP request. I'd suggest to use local storage functions to store the passphrase.
- There's no encryption for attachments. I know, this is hard or almost
impossible to implement on the client but it's a serious lack of feature. We might consider to implement hooks that'll let you read attachment contents back to the client in order to encrypt them.
- Decryption of encrypted messages is only possible on messages sent with
text/plain but not with multipart/encrypted. This is mainly because Roundcube doesn't even send the encrypted body parts to the client. Solution: there are plugin hooks in PHP available [2] to grab encrypted parts from the message structure and make sure they're sent to the client. Attachment decryption is also consider here.
- No decryption in message preview! One has to open the message in full
screen to have the openpgpjs plugin kick in. Just activate the plugin on 'preview' action as well.
- UI elements such as dialogs and forms are fully created in javascript. In
order to also make them customizable using skins, you should use skins and templates and add them to the html page in PHP.
- Texts are not localized but hard-coded in javascript. The Roundcube
plugin system provides easy ways to maintain localized texts. Those strings can also be made available in javascript. See other plugins how this is done.
- Make key-server to lookup keys configurable. See other plugins how plugin
config works.
Usability
- In the message view screen, a button is added to the top tasks bar.
Besides that it doesn't fit in the Larry skin, this isn't the right place to put that button. In the compose screen, it is placed to the toolbar which is much better. However, I'd consider to find the key management in the Settings section.
- There's no progress/activity display while encrypting/decrypting action
is going on. I rekon this sometimes takes a few seconds and as a user, I don't know what's happening.
- The key management dialogs don't have a proper layout. At least in Larry
skin, the import box is aligned right of the table, forcing the table contents to break. The inputs for importing keys could even be hidden in an additional dialog or just placed below the table.
- The order of the dialog tabs (Generate keys, Private keys, Public keys)
should be reversed. Importing public keys is IMO the most common task while generating keys is rarely used.
- Create individual CSS files for different skins in order to better
integrate with the Roundcube UI.
Please don't take the above points as a rant but as a friendly and constructive feedback. We're happy to see progress on this and we're willing to help you making things nice and shiny.
Best regards, Thomas
[1] http://trac.roundcube.net/wiki/Plugin_Events#beforeandafter [2] http://trac.roundcube.net/wiki/Plugin_Hooks#message_part_structure _______________________________________________ Roundcube Development discussion mailing list dev@lists.roundcube.net http://lists.roundcube.net/mailman/listinfo/dev
Niklas Femerstrand wrote:
Heylo!
Don't worry about me taking this personally, I absolutely don't and rather really appreciate the feedback. I am already aware of a lot of the points you raise, my goal was to put in all base crypto stuff before bothering about the RC specifics. A lot of the things are just my quick way of doing them versus spending time referencing RC docs. Doesn't mean that the points shouldn't be repeated and highlighted though :)
All right! I just was concerned about your announcement to soon release a 'stable' version of the plugin which I somehow assumed to be final.
So keep up the good work, I'll probably look into the multipart/encrypted issue since I'm pretty familiar with the server-side mime structure and the available plugin hooks.
Best, Thomas
-
Niklas Femerstrand -
Patrick Heinson -
Thomas Bruederli -
Torsten Grote