Page MenuHomePhabricator

SecurePoll is not compatible with GPG 2.1+
Open, HighPublic

Description

As discussed on T209802, GPG 2.1+ automatically starts a gpg-agent child process and leaves it running after the parent gpg exits. This does not suit SecurePoll's pattern of use of gpg. At best, it is a resource leak. If MediaWiki is configured to use cgroup-based memory limiting, ($wgShellCgroup) a background monitor process will wait forever, expecting gpg-agent to exit so that the cgroup can be cleaned up. If HHVM LightProcess is used, the PHP script also hangs waiting for the background processes to exit. Perhaps there are also security implications, if the gpg-agent inappropriately persists the private signing key.

The workaround for now is to install GPG 1.x and to configure SecurePoll to use it with $wgSecurePollGPGCommand.

@faidon suggests migrating to a GPGME wrapper such as http://php.net/manual/en/book.gnupg.php

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptNov 19 2018, 10:49 PM
tstarling added subscribers: STran, phuedx.

Bumping this because we can't just use GPG 1 forever.

GpgCrypt is the main reason tallying takes a long time -- if it was faster, maybe we wouldn't even need to do tallying via the job queue. If we can avoid tallying via the job queue, we can avoid storing the tally key in the database, we can just have it in request memory, which improves privacy.

The gnupg extension in PECL still stores its keys in a keyring file on disk, so it has a lot of the same drawbacks as the current code.

Maybe we should use OpenSSL instead. What SecurePoll needs is basic asymmetric crypto primitives, preferably with some sort of standard ASCII armor, it doesn't need keyrings or a web of trust or anything like that.

What SecurePoll needs is basic asymmetric crypto primitives, preferably with some sort of standard ASCII armor, it doesn't need keyrings or a web of trust or anything like that.

php-libsodium is bundled with PHP 7.2 onwards and can be enabled at compile time โ€“ IIRC it needs to be installed via PECL for existing PHP installations. However, I don't see any mention of ASCII armour in the extension's documentation.

Maybe we should use OpenSSL instead. What SecurePoll needs is basic asymmetric crypto primitives, preferably with some sort of standard ASCII armor, it doesn't need keyrings or a web of trust or anything like that.

+1, as this also nicely avoids needing to shell out to GPG, which would've required figuring out how to do that with Shellbox.

php-libsodium is bundled with PHP 7.2 onwards and can be enabled at compile time โ€“ IIRC it needs to be installed via PECL for existing PHP installations. However, I don't see any mention of ASCII armour in the extension's documentation.

php-sodium should be available on all appservers:

legoktm@mwmaint2002:~$ php -i | grep sodium
sodium
sodium support => enabled
libsodium headers version => 1.0.11
libsodium library version => 1.0.11
jrbs raised the priority of this task from Low to High.Sep 23 2022, 10:35 PM

To a certain extent its debatable whether we really need asymmetric crypto here. We're trusting the server to accurately record everyone's vote. Users get a receipt but have no way of verifying it. The scheme of asymmetric crypto here prevents an attacker from finding out the results of the election early. It does not stop an attacker with server or DB access from casting false ballots or replacing existing ones. I'm not sure that really makes sense as a threat model, although perhaps its easier for an attacker to morally justify an early peak as just being slightly evil, where it is harder to lie to yourself once you start replacing ballots.

The scheme of asymmetric crypto here prevents an attacker from finding out the results of the election early.

This happened in a past WMF Board election (the results being leaked midway through the election). No opinion on whether it still makes sense as part of the threat model.

I would like to see a way forward for this, as securepoll currently is a blocker for the kubernetes transition.

@tstarling what do you think is the best option:

  • Migrate calls to gpg to shellbox, figure out the way forward for this task later?
  • Actually migrate to use libsodium?

Also: who is going to take on this work?

Reviewing the libsodium manual: I'm concerned about the lack of support for standard message formats. In fact there are very few references to standards of any kind. It's like a research project. At least 93% of commits are by a single contributor (Frank Denis). There's a risk that if he gets bored with it, the library will stop being maintained and we won't be able to read our data anymore.

OpenSSL has openssl_pkey_get_public() and openssl_pkey_get_private(), which take a PEM formatted public/private key as a parameter. Then there is openssl_seal(), openssl_sign(), openssl_verify() and openssl_open() to use the key pair. It doesn't support OpenPGP ASCII armor and message formats, but neither does libsodium. So I don't understand the desire to use libsodium. It seems like OpenSSL can do the job with less risk.

I think a project to migrate to OpenSSL would be pretty straightforward. A lot of the code in GpgCrypt.php is just to support file management and to manage the slowness of gpg shell commands. An OpenSSL subclass of Crypt would be simpler.

Also: who is going to take on this work?

@jrbs and @drochford may have thoughts on that, since they've been organising maintenance work on SecurePoll recently.

Also: who is going to take on this work?

@jrbs and @drochford may have thoughts on that, since they've been organising maintenance work on SecurePoll recently.

I can prioritise this with the contractors we have at the moment particularly if this is blocking other work.

Also: who is going to take on this work?

@jrbs and @drochford may have thoughts on that, since they've been organising maintenance work on SecurePoll recently.

I can prioritise this with the contractors we have at the moment particularly if this is blocking other work.

Hi @jrbs, sorry I missed this comment! Yes, moving away from gnupg (or moving its execution to shellbox, which doesn't seem to be viable though) is a blocker for the migration to mediawiki on kubernetes.

We have now learned that only the votewiki install of SecurePoll makes use of GPG. This is good news for the shellbox move but is less good news for the local elections piece, since it sounds like there would not presently be a way to encrypt a local election.

Therefore I'm formally blocking T301180 on this.

From what I have read, I also think the best solution would be to implement encryption with PHP's OpenSSL extensions [1], rather than using libsodium or a "shellboxed" gpg. I guess this extension is enabled on all wikis on the Wikimedia Cluster, right?

What needs to be done?

  1. Implement a new Crypt class OpenSSLCrypt
  2. Implement tests for this new class (old implementation unfortunately does not have any)
  3. Optional: Remove GpgCrypt, $wgSecurePollGpgSignKey and $wgSecurePollGpgSignKey completely

What about a migration path? I don't really think this will be required, but there may be reasons that I am currently not aware of.

[1] https://www.php.net/manual/en/openssl.installation.php

Change 1003091 had a related patch set uploaded (by Skizzerz; author: Skizzerz):

[mediawiki/extensions/SecurePoll@master] Add encryption method using openssl

https://gerrit.wikimedia.org/r/1003091

Hey @Skizzerz, thanks for submitting a patch. This task was listed in our contractors' work package and I was just wondering what the status of the CR is?

Hey @Skizzerz, thanks for submitting a patch. This task was listed in our contractors' work package and I was just wondering what the status of the CR is?

Just submitted a new patch set with fixes for the most recent set of review comments, however if it still fails Code Review then your contractors can pick it up and take it to the finish line as other projects have been competing for my time recently.

What is the ETA to complete this as this is a blocker for SRE Service Ops team with Mediawiki migration to Kubernetes?

Change #1003091 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] Add encryption method using openssl

https://gerrit.wikimedia.org/r/1003091

tstarling added a subscriber: dom_walden.

Following the merge of https://gerrit.wikimedia.org/r/1003091 it would be good if @dom_walden could do a test election using the new OpenSSL encryption method. See my local test log comment on PS5 for information about generating keys and creating an election.

This is blocking mw-on-k8s migration. Is there an ETA for this to be resolved?

@tstarling On beta and locally, when I try to create an OpenSSL election I get:

[ZjyFH@yv@w1-3tE-EC@1CAAAAAA] /wiki/Special:SecurePoll/create Error: Call to private method MediaWiki\Extension\SecurePoll\Crypt\OpenSslCrypt::checkPublicKey() from context 'MediaWiki\HTMLForm\HTMLFormField'

Backtrace:

from /srv/mediawiki/php-master/includes/htmlform/HTMLFormField.php(441)
#0 /srv/mediawiki/php-master/includes/htmlform/HTMLForm.php(759): MediaWiki\HTMLForm\HTMLFormField->validate(string, array)
#1 /srv/mediawiki/php-master/includes/htmlform/HTMLForm.php(673): MediaWiki\HTMLForm\HTMLForm->trySubmit()
#2 /srv/mediawiki/php-master/extensions/SecurePoll/includes/Pages/CreatePage.php(541): MediaWiki\HTMLForm\HTMLForm->tryAuthorizedSubmit()
#3 /srv/mediawiki/php-master/extensions/SecurePoll/includes/SpecialSecurePoll.php(65): MediaWiki\Extension\SecurePoll\Pages\CreatePage->execute(array)
#4 /srv/mediawiki/php-master/includes/specialpage/SpecialPage.php(719): MediaWiki\Extension\SecurePoll\SpecialSecurePoll->execute(string)
#5 /srv/mediawiki/php-master/includes/specialpage/SpecialPageFactory.php(1672): MediaWiki\SpecialPage\SpecialPage->run(string)
#6 /srv/mediawiki/php-master/includes/actions/ActionEntryPoint.php(502): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, MediaWiki\Context\RequestContext)
#7 /srv/mediawiki/php-master/includes/actions/ActionEntryPoint.php(145): MediaWiki\Actions\ActionEntryPoint->performRequest()
#8 /srv/mediawiki/php-master/includes/MediaWikiEntryPoint.php(199): MediaWiki\Actions\ActionEntryPoint->execute()
#9 /srv/mediawiki/php-master/index.php(58): MediaWiki\MediaWikiEntryPoint->run()
#10 /srv/mediawiki/w/index.php(3): require(string)
#11 {main}

I have successfully been able to create a GPG election.

Change #1029910 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/SecurePoll@master] Fix exception when creating an election with the OpenSSL encryption type

https://gerrit.wikimedia.org/r/1029910

After checking out T209892#9785114, when trying to create an OpenSSL election, below the Signing key input I see the error:

One of the keys provided was invalid. An encryption key must be an RSA public key of at least 2048 bits and a decryption or signing key must be an RSA private key of at least 2048 bits.

I think this line should be checkPrivateKey.

Change #1029910 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] Fix exception when creating an election with the OpenSSL encryption type

https://gerrit.wikimedia.org/r/1029910

Change #1031070 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/SecurePoll@wmf/1.43.0-wmf.4] Fix exception when creating an election with the OpenSSL encryption type

https://gerrit.wikimedia.org/r/1031070

Change #1031070 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@wmf/1.43.0-wmf.4] Fix exception when creating an election with the OpenSSL encryption type

https://gerrit.wikimedia.org/r/1031070

Mentioned in SAL (#wikimedia-operations) [2024-05-14T00:20:37Z] <tstarling@deploy1002> Started scap: Fix SecurePoll exception T209892 and CodeMirror 5 RTL T363752

Mentioned in SAL (#wikimedia-operations) [2024-05-14T00:35:34Z] <tstarling@deploy1002> Finished scap: Fix SecurePoll exception T209892 and CodeMirror 5 RTL T363752 (duration: 14m 56s)