Enigma smime signed messages verification #6043

duk3luk3 · 2017-11-14T07:29:36Z

This is the first step to address #4977.

I made this PR to familiarise myself with the enigma plugin and show a first proof-of-concept.

Description

It does the following things:

Hooks up SMIME signature verification logic that already mostly existed in enigma
Allows the server administrator to specify trusted CA files in enigma plugin config
Introduces new "server-verified" verification level: If trusted CAs are specified in enigma plugin config, full verification trust (Green bar) is reserved for messages signed with a certificate trusted by one of those CAs; messages signed by a certificate trusted by the default CAs are displayed as "server-verified" with a yellow bar.

What it doesn't do:

No decryption - so also no verification of encrypted messages
No encryption
No user CAs / certificates

What I am finding out here is that the php openssl module is severely limited in what it can do. Things I would like to do but seemingly can't using php openssl:

Use intermediate certificates as trust anchors
Retrieve the whole certificate chain used to verify a signature

It may be necessary to invoke an openssl executable directly to actually implement all the features users would expect.

How to test

Test basic verification:

Enable the enigma plugin using the code in this branch
Open a signed (unencrypted!) S/MIME message
If the root ca for the signature of that message is in your server's cert store, you should see a green signature verification bar

Test verification of configured CAs:

Set $config['enigma_smime_ca'] = array('/'); (a directory without certificates)
The message should now have a yellow signature verification bar
Set $config['enigma_smime_ca'] = array('/path/to/ca.cert'); referencing the correct root ca
The message should now have a green bar again

taalas · 2017-11-14T10:19:11Z

Looks very promising. Looking forward to this being merged.

alecpl · 2017-11-20T08:47:22Z

Looks good. I'm not sure it should be merged with master. Maybe a separate branch would be better. Or we implement an option that will disable the whole S/MIME functionality by default.

duk3luk3 · 2017-11-22T03:39:54Z

The S/MIME functionality (as it is implemented so far) is completely unobtrusive.

Once I add more UI for it (e.g. Certificate Management and a column to enable/disable S/MIME functions in the main Enigma config panel), the existing config override system can be used to disable S/MIME entirely and it's fine to set it disabled by default.

Is that agreeable?

renne · 2018-02-08T11:49:08Z

Does it support retrieving S/MIME certificates from DNS according to IETF RFC 8162?

G00sfraba · 2019-04-07T14:23:49Z

Can you share a screenshot of how this should look?
I've applied the changes from this PR manually on my local copy, but I, can't see this "green bar" you are referring to. Any advice or guidance?

PS, I also tried direct install from the branch, but still with 0 success on any indication for valid s/mime signed email on the UI.

m-ueberall · 2019-06-08T12:52:02Z

@G00sfraba It looks as follows after I've applied the (corrected) resulting patch against v1.3.9 today:

tborychowski · 2020-07-01T11:08:55Z

Any update on this? Would love to see that feature :-)

G00sfraba · 2020-07-02T04:56:10Z

Any update on this? Would love to see that feature :-)

I ended up customising rc_smime.php as I need. have to say that no other changes were needed since then (more than an year ago) so go for it.

tborychowski · 2020-07-02T09:27:49Z

@G00sfraba, would you have a fork/branch/repo with these changes?
Or could you share them in any way? :-)

dereks · 2020-08-20T22:48:40Z

Thank you for submitting a Pull Request (PR) to the Roundcube GitHub project.

You are receiving this message because your PR has conflicts which need to be resolved. We are trying to catch up on our backlog of old PRs and get them merged in (where appropriate). Therefor, we request the following:

Step 1. Rebase from the latest Roundcube branch master into your PR.

git fetch upstream
git checkout your_feature_branch_name
git rebase upstream/master
git push -f origin your_feature_branch_name

Step 2. Re-test to make sure your new code still works as expected

Step 3. Comment back here once it has been tested and will merge cleanly.

Once this has been done we will treat it like a new Pull Request and consider it for acceptance.

Apologies for the inconvenience. Thank you for contributing to Roundcube!

alecpl · 2021-01-16T14:07:24Z

plugins/enigma/lib/enigma_driver_phpssl.php

@@ -37,6 +38,7 @@ function __construct($user)
    function init()
    {
        $homedir = $this->rc->config->get('enigma_smime_homedir', INSTALL_PATH . '/plugins/enigma/home');
+        $this->cainfo = $this->rc->config->get('enigma_smime_ca', array());


We're using short array syntax now.

alecpl · 2021-01-16T14:07:44Z

plugins/enigma/lib/enigma_driver_phpssl.php

@@ -65,6 +67,15 @@ function init()

        $this->homedir = $homedir;

+        #XXX: Workaround for https://bugs.php.net/bug.php?id=75494
+        if (count($this->cainfo) > 0) {


This will be a warning if the option in config is not an array.

alecpl · 2021-01-16T14:08:34Z

plugins/enigma/lib/enigma_driver_phpssl.php

+        if ($sig !== true) {
+            // try with server trusted certificate verification
+            $sig      = openssl_pkcs7_verify($msg_file, 0, $cert_file);
+            $validity = enigma_error::SERVER_VERIFIED;


Why would that be an error? Find another way to pass this info, e.g. involving enigma_signature::$partial.

alecpl · 2021-01-16T14:08:53Z

plugins/enigma/lib/enigma_driver_phpssl.php

@@ -229,6 +245,8 @@ private function parse_sig_cert($file, $validity)
 //        $data->comment     = '';
        $data->email       = $cert['subject']['emailAddress'];

+        rcube::write_log('errors', 'Decrypted sig: ' . $data->name);


This is not needed.

alecpl · 2021-01-16T14:11:36Z

plugins/enigma/lib/enigma_error.php

@@ -30,6 +30,7 @@ class enigma_error
    const BADPASS     = 5;
    const EXPIRED     = 6;
    const UNVERIFIED  = 7;
+    const SERVER_VERIFIED  = 8;


This is conflict that needs to be resolved.

alecpl · 2021-01-16T14:12:15Z

plugins/enigma/lib/enigma_mime_message.php

@@ -19,6 +19,8 @@ class enigma_mime_message extends Mail_mime
 {
    const PGP_SIGNED    = 1;
    const PGP_ENCRYPTED = 2;
+    const SMIME_SIGNED  = 3;


These new const aren't used/implemented yet. I would remove them.

duk3luk3 added 4 commits November 21, 2017 11:15

SMIME sig verification stub test

ee27a7a

Add configured certificate verification

338cebe

Add php bug workaround

e9d95ca

Add smime CA config option

d803cdb

duk3luk3 force-pushed the enigma-smime branch from 20d1ee0 to d803cdb Compare November 21, 2017 00:15

duk3luk3 mentioned this pull request Mar 18, 2018

Add S/MIME-support to Roundcube #4977

Open

alecpl requested changes Jan 16, 2021

View reviewed changes

huberste mentioned this pull request Apr 22, 2022

Enigma smime signed messages verification #8513

Open

ChristophWurst mentioned this pull request Aug 25, 2022

S/MIME support nextcloud/mail#7085

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enigma smime signed messages verification #6043

Enigma smime signed messages verification #6043

duk3luk3 commented Nov 14, 2017

taalas commented Nov 14, 2017

alecpl commented Nov 20, 2017

duk3luk3 commented Nov 22, 2017 •

edited

Loading

renne commented Feb 8, 2018

G00sfraba commented Apr 7, 2019

m-ueberall commented Jun 8, 2019 •

edited

Loading

tborychowski commented Jul 1, 2020

G00sfraba commented Jul 2, 2020

tborychowski commented Jul 2, 2020

dereks commented Aug 20, 2020

alecpl Jan 16, 2021

alecpl Jan 16, 2021

alecpl Jan 16, 2021

alecpl Jan 16, 2021

alecpl Jan 16, 2021

alecpl Jan 16, 2021

Enigma smime signed messages verification #6043

Are you sure you want to change the base?

Enigma smime signed messages verification #6043

Conversation

duk3luk3 commented Nov 14, 2017

Description

How to test

taalas commented Nov 14, 2017

alecpl commented Nov 20, 2017

duk3luk3 commented Nov 22, 2017 • edited Loading

renne commented Feb 8, 2018

G00sfraba commented Apr 7, 2019

m-ueberall commented Jun 8, 2019 • edited Loading

tborychowski commented Jul 1, 2020

G00sfraba commented Jul 2, 2020

tborychowski commented Jul 2, 2020

dereks commented Aug 20, 2020

alecpl Jan 16, 2021

Choose a reason for hiding this comment

alecpl Jan 16, 2021

Choose a reason for hiding this comment

alecpl Jan 16, 2021

Choose a reason for hiding this comment

alecpl Jan 16, 2021

Choose a reason for hiding this comment

alecpl Jan 16, 2021

Choose a reason for hiding this comment

alecpl Jan 16, 2021

Choose a reason for hiding this comment

duk3luk3 commented Nov 22, 2017 •

edited

Loading

m-ueberall commented Jun 8, 2019 •

edited

Loading