Commit c52b970d authored by ng's avatar ng

Fix #360 - do not allow setting an empty fingerprint through `SET-FINGERPRINT`

Check for a valid fingerprint as argument and do not accept an empty
fingerprint. This will not anymore allow to unset a fingerprint
through the `SET-FINGERPRINT`. This functionality will be superseeded
by another keyword.

As part of that fix, wie centralize checking for a valid fingerpint
and constrain the check to be either 32 (v3) or 40 (v4) characters
long.
parent f6b61a08
......@@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
* Send replies to keyword-usage and notices to admins regardless of the delivery-flag of their subscription. (#354)
* X-UNSUBSCRIBE will refuse to unsubscribe the last admin of a list. (#357)
* Handle "protected subjects" in a way that Thunderbird/Enigmail recognize. (#74)
* X-SET-FINGERPRINT will not anymore allow setting an empty fingerprint. (#360)
### Changed
......
......@@ -5,7 +5,8 @@ module Schleuder
include Singleton
EMAIL_REGEXP = /\A.+@[[:alnum:]_.-]+\z/i
FINGERPRINT_REGEXP = /\A(0x)?[a-f0-9]{32,}\z/i
# TODO: drop v3 keys and only accept length of 40
FINGERPRINT_REGEXP = /\A(0x)?[a-f0-9]{32}([a-f0-9]{8})?\z/i
DEFAULTS = {
'lists_dir' => '/var/lib/schleuder/lists',
......
......@@ -208,5 +208,9 @@ module GPGME
pinentry = File.join(ENV['SCHLEUDER_ROOT'], 'bin', 'pinentry-clearpassphrase')
GPGME::Ctx.spawn_daemon('gpg-agent', "--use-standard-socket --pinentry-program #{pinentry}")
end
def self.valid_fingerprint?(fp)
fp =~ Schleuder::Conf::FINGERPRINT_REGEXP
end
end
end
......@@ -137,8 +137,15 @@ module Schleuder
)
end
sub.fingerprint = arguments.join
fingerprint = arguments.join
unless GPGME::Key.valid_fingerprint?(fingerprint)
return I18n.t(
"plugins.subscription_management.set_fingerprint_requires_valid_fingerprint",
fingerprint: fingerprint
)
end
sub.fingerprint = fingerprint
if sub.save
I18n.t(
"plugins.subscription_management.fingerprint_set",
......@@ -149,7 +156,7 @@ module Schleuder
I18n.t(
"plugins.subscription_management.setting_fingerprint_failed",
email: email,
fingerprint: arguments.last,
fingerprint: sub.fingerprint,
errors: sub.errors.to_a.join("\n")
)
end
......
class FingerprintValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
unless value =~ /\A[A-F0-9]{32,}\z/
unless GPGME::Key.valid_fingerprint?(value)
record.errors[attribute] << (options[:message] || I18n.t("errors.invalid_fingerprint"))
end
end
......
......@@ -150,6 +150,18 @@ de:
setting_fingerprint_failed: |
Fingerabdruck für %{email} konnte nicht auf %{fingerprint} gesetzt werden:
%{errors}.
set_fingerprint_requires_valid_fingerprint: |
Du hast zu dem Schlüsselwort 'SET-FINGERPRINT' keinen gültigen Wert angegeben.
Es wurde der folgende Wert erkannt: %{fingerprint}
Benötigt werden ein oder zwei Werte, bspw.:
X-SET-FINGERPRINT: 0xB3D190D5235C74E1907EACFE898F2C91E2E6E1F3
oder (als admin):
X-SET-FINGERPRINT: subscription2@hostname 0xB3D190D5235C74E1907EACFE898F2C91E2E6E1F3
Wobei der Fingerprint in der gesamten Länge (40 Zeichen) angegeben werden muss. Optional mit 0x als Präfix.
set_fingerprint_requires_arguments: |
Du hast zu dem Schlüsselwort 'SET-FINGERPRINT' keinen Wert angegeben.
......
......@@ -154,6 +154,18 @@ en:
setting_fingerprint_failed: |
Setting fingerprint for %{email} to %{fingerprint} failed:
%{errors}.
set_fingerprint_requires_valid_fingerprint: |
You did not send a valid fingerprint for the keyword 'SET-FINGERPRINT'
The following value was detected: %{fingerprint}
One or two are required, e.g.:
X-SET-FINGERPRINT: 0xB3D190D5235C74E1907EACFE898F2C91E2E6E1F3
or (as an admin):
X-SET-FINGERPRINT: subscription2@hostname 0xB3D190D5235C74E1907EACFE898F2C91E2E6E1F3
While the fingerprint must be passed in the full length (40 characters). Optionally prefixed with 0x.
set_fingerprint_requires_arguments: |
Error: You did not send any arguments for the keyword 'SET-FINGERPRINT'.
......
......@@ -672,6 +672,89 @@ describe "user sends keyword" do
teardown_list_and_mailer(list)
end
it "x-set-fingerprint with email-address but without fingerprint" do
list = create(:list)
list.subscribe("schleuder@example.org", '59C71FB38AEE22E091C78259D06350440F759BD3', true)
list.import_key(File.read('spec/fixtures/example_key.txt'))
ENV['GNUPGHOME'] = list.listdir
mail = Mail.new
mail.to = list.request_address
mail.from = list.admins.first.email
gpg_opts = {
encrypt: true,
keys: {list.request_address => list.fingerprint},
sign: true,
sign_as: list.admins.first.fingerprint
}
mail.gpg(gpg_opts)
mail.body = "x-list-name: #{list.email}\nX-set-fingerprint: schleuder@example.org "
mail.deliver
encrypted_mail = Mail::TestMailer.deliveries.first
Mail::TestMailer.deliveries.clear
begin
Schleuder::Runner.new().run(encrypted_mail.to_s, list.request_address)
rescue SystemExit
end
raw = Mail::TestMailer.deliveries.first
message = Mail.create_message_to_list(raw.to_s, list.request_address, list).setup
subscription = list.subscriptions.where(email: 'schleuder@example.org').first
expect(message.to).to eql(['schleuder@example.org'])
expect(message.first_plaintext_part.body.to_s).to eql(I18n.t(
"plugins.subscription_management.set_fingerprint_requires_valid_fingerprint",
fingerprint: ''
))
expect(subscription).to be_present
expect(subscription.fingerprint).to eql('59C71FB38AEE22E091C78259D06350440F759BD3')
teardown_list_and_mailer(list)
end
it "x-set-fingerprint with email-address but without valid fingerprint" do
list = create(:list)
list.subscribe("schleuder@example.org", '59C71FB38AEE22E091C78259D06350440F759BD3', true)
list.import_key(File.read('spec/fixtures/example_key.txt'))
ENV['GNUPGHOME'] = list.listdir
mail = Mail.new
mail.to = list.request_address
mail.from = list.admins.first.email
gpg_opts = {
encrypt: true,
keys: {list.request_address => list.fingerprint},
sign: true,
sign_as: list.admins.first.fingerprint
}
mail.gpg(gpg_opts)
mail.body = "x-list-name: #{list.email}\nX-set-fingerprint: schleuder@example.org 59C71FB38AEE22E091C78259D0"
mail.deliver
encrypted_mail = Mail::TestMailer.deliveries.first
Mail::TestMailer.deliveries.clear
begin
Schleuder::Runner.new().run(encrypted_mail.to_s, list.request_address)
rescue SystemExit
end
raw = Mail::TestMailer.deliveries.first
message = Mail.create_message_to_list(raw.to_s, list.request_address, list).setup
subscription = list.subscriptions.where(email: 'schleuder@example.org').first
expect(message.to).to eql(['schleuder@example.org'])
# arguments are downcased when parsed
expect(message.first_plaintext_part.body.to_s).to eql(I18n.t(
"plugins.subscription_management.set_fingerprint_requires_valid_fingerprint",
fingerprint: '59C71FB38AEE22E091C78259D0'.downcase
))
expect(subscription).to be_present
expect(subscription.fingerprint).to eql('59C71FB38AEE22E091C78259D06350440F759BD3')
teardown_list_and_mailer(list)
end
it "x-set-fingerprint without email-address and with invalid fingerprint" do
list = create(:list)
list.subscribe("schleuder@example.org", '59C71FB38AEE22E091C78259D06350440F759BD3', true)
......@@ -686,7 +769,7 @@ describe "user sends keyword" do
sign_as: list.admins.first.fingerprint
}
mail.gpg(gpg_opts)
mail.body = "x-list-name: #{list.email}\nX-set-fingerprint: blabla"
mail.body = "x-list-name: #{list.email}\nX-set-fingerprint: blaBLA"
mail.deliver
encrypted_mail = Mail::TestMailer.deliveries.first
......@@ -701,7 +784,11 @@ describe "user sends keyword" do
subscription = list.subscriptions.where(email: 'schleuder@example.org').first
expect(message.to).to eql(['schleuder@example.org'])
expect(message.to_s).to include("Fingerprint is not a valid OpenPGP-fingerprint")
# arguments are downcased when parsed
expect(message.first_plaintext_part.body.to_s).to eql(I18n.t(
"plugins.subscription_management.set_fingerprint_requires_valid_fingerprint",
fingerprint: 'blaBLA'.downcase
))
expect(subscription.fingerprint).to eql('59C71FB38AEE22E091C78259D06350440F759BD3')
......
......@@ -47,4 +47,31 @@ describe GPGME::Key do
expect(key.oneline).to match(/0xB1CD8BB15C2673C6BFD8FA4B70B2CF29E01AD53E signonly@example.org \d{4}-\d{2}-\d{2} \[not capable of encryption\]/)
end
end
describe '.valid_fingerprint?' do
context 'valid fingerprints' do
['59C71FB38AEE22E091C78259D06350440F759BD3',
'0x59C71FB38AEE22E091C78259D06350440F759BD3',
'59C71FB38AEE22E091C78259D0635044',
'0x59C71FB38AEE22E091C78259D0635044',
].each do |fp|
it "accepts #{fp} as a valid fingerprint" do
expect(described_class.valid_fingerprint?(fp)).to be_truthy
end
end
end
context 'invalid fingerprints' do
['Z9C71FB38AEE22E091C78259D06350440F759BD3',
'59C71FB38AEE22E091C78259D06350440F759BD3A',
'59C71FB38AEE22E091C78259D06350440F759BD',
'0x59C71FB38AEE22E091C78259D06350440F759B',
'Z9C71FB38AEE22E091C78259D0635044',
'Z9C71FB38AEE22E091C78259D0635044',
].each do |fp|
it "rejects #{fp} as an invalid fingerprint" do
expect(described_class.valid_fingerprint?(fp)).to be_falsey
end
end
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment