Skip to content

Commit

Permalink
CLI: Improve message, displayed when signature verification is failed.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ni4 @ronaldtse
ni4 authored and ronaldtse committed May 7, 2022
1 parent 37b5322 commit 2bd2597
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 32 deletions.
28 changes: 21 additions & 7 deletions src/rnp/fficli.cpp
View file
Expand Up @@ -2900,15 +2900,29 @@ cli_rnp_print_signatures(cli_rnp_t *rnp, const std::vector<rnp_op_verify_signatu
rnp_signature_handle_destroy(handle);
}

if (sigs.size() == 0) {
if (!sigs.size()) {
ERR_MSG("No signature(s) found - is this a signed file?");
} else if (invalidc > 0 || unknownc > 0) {
ERR_MSG(
"Signature verification failure: %u invalid signature(s), %u unknown signature(s)",
invalidc,
unknownc);
} else {
return;
}
if (!invalidc && !unknownc) {
ERR_MSG("Signature(s) verified successfully");
return;
}
/* Show a proper error message if there are invalid/unknown signatures */
auto si = invalidc > 1 ? "s" : "";
auto su = unknownc > 1 ? "s" : "";
auto fail = "Signature verification failure: ";
if (invalidc && !unknownc) {
ERR_MSG("%s%u invalid signature%s", fail, invalidc, si);
} else if (!invalidc && unknownc) {
ERR_MSG("%s%u unknown signature%s", fail, unknownc, su);
} else {
ERR_MSG("%s%u invalid signature%s, %u unknown signature%s",
fail,
invalidc,
si,
unknownc,
su);
}
}

Expand Down
54 changes: 29 additions & 25 deletions src/tests/cli_tests.py
View file
Expand Up @@ -87,6 +87,7 @@ def escape_regex(str):
PUBRING_1 = 'keyrings/1/pubring.gpg'
SECRING_G10 = 'test_stream_key_load/g10'
KEY_ALICE_PUB = 'test_key_validity/alice-pub.asc'
KEY_ALICE_SUB_PUB = 'test_key_validity/alice-sub-pub.pgp'
KEY_ALICE_SEC = 'test_key_validity/alice-sec.asc'
KEY_ALICE_SUB_SEC = 'test_key_validity/alice-sub-sec.pgp'
KEY_25519_NOTWEAK_SEC = 'test_key_edge_cases/key-25519-non-tweaked-sec.asc'
Expand Down Expand Up @@ -1376,7 +1377,7 @@ def test_export_keys(self):

clear_keyrings()
# Import Alice's public key
ret, _, _ = run_proc(RNPK, ['--homedir', RNPDIR, '--import', data_path('test_key_validity/alice-sub-pub.pgp')])
ret, _, _ = run_proc(RNPK, ['--homedir', RNPDIR, '--import', data_path(KEY_ALICE_SUB_PUB)])
self.assertEqual(ret, 0)
# Attempt to export no key
ret, _, err = run_proc(RNPK, ['--homedir', RNPDIR, '--export-key'])
Expand Down Expand Up @@ -2126,14 +2127,14 @@ def test_rnp_list_packets_edge_cases(self):
# List empty key packets
params = ['--list-packets', data_path('test_key_edge_cases/key-empty-packets.pgp')]
ret, out, _ = run_proc(RNP, params)
self.assertEqual(ret, 0, 'packet listing failed')
self.assertEqual(ret, 0, PKT_LIST_FAILED)
compare_file_ex(data_path('test_key_edge_cases/key-empty-packets.txt'), out,
'key-empty-packets listing mismatch')

# List empty key packets json
params = ['--list-packets', '--json', data_path('test_key_edge_cases/key-empty-packets.pgp')]
ret, _, _ = run_proc(RNP, params)
self.assertEqual(ret, 0, 'packet listing failed')
self.assertEqual(ret, 0, PKT_LIST_FAILED)

# List empty uid
params = ['--list-packets', KEY_EMPTY_UID]
Expand Down Expand Up @@ -2736,10 +2737,10 @@ def test_text_sig_crcr(self):
srcsig = data_path('test_messages/message.text-sig-crcr.sig')
srctxt = data_path('test_messages/message.text-sig-crcr')
# Verify with RNP
ret, _, _ = run_proc(RNP, ['--homedir', RNPDIR, '--keyfile', data_path('test_key_validity/alice-sub-pub.pgp'), '-v', srcsig])
ret, _, _ = run_proc(RNP, ['--homedir', RNPDIR, '--keyfile', data_path(KEY_ALICE_SUB_PUB), '-v', srcsig])
self.assertEqual(ret, 0)
# Verify with GPG
ret, _, _ = run_proc(GPG, ['--batch', '--homedir', GPGHOME, '--import', data_path('test_key_validity/alice-sub-pub.pgp')])
ret, _, _ = run_proc(GPG, ['--batch', '--homedir', GPGHOME, '--import', data_path(KEY_ALICE_SUB_PUB)])
self.assertEqual(ret, 0, 'gpg key import failed')
gpg_verify_detached(srctxt, srcsig, 'Alice <alice@rnp>')

Expand All @@ -2758,14 +2759,14 @@ def test_clearsign_long_lines(self):
[sig] = reg_workfiles('cleartext', '.sig')
srctxt = data_path('test_messages/message.4k-long-lines')
srcsig = data_path('test_messages/message.4k-long-lines.asc')
pubkey = data_path('test_key_validity/alice-sub-pub.pgp')
pubkey = data_path(KEY_ALICE_SUB_PUB)
seckey = data_path('test_key_validity/alice-sub-sec.pgp')
# Verify already existing file
ret, _, err = run_proc(RNP, ['--homedir', RNPDIR, '--keyfile', pubkey, '-v', srcsig])
self.assertEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Good signature made.*73edcc9119afc8e2dbbdcde50451409669ffde3c.*')
# Verify with gnupg
ret, _, _ = run_proc(GPG, ['--batch', '--homedir', GPGHOME, '--import', data_path('test_key_validity/alice-sub-pub.pgp')])
ret, _, _ = run_proc(GPG, ['--batch', '--homedir', GPGHOME, '--import', pubkey])
self.assertEqual(ret, 0, 'gpg key import failed')
gpg_verify_cleartext(srcsig, 'Alice <alice@rnp>')
# Sign again with RNP
Expand All @@ -2784,13 +2785,13 @@ def test_eddsa_sig_lead_zero(self):
srcs = data_path('test_messages/eddsa-zero-s.txt.sig')
srcr = data_path('test_messages/eddsa-zero-r.txt.sig')
# Verify with RNP
ret, _, _ = run_proc(RNP, ['--homedir', RNPDIR, '--keyfile', data_path('test_key_validity/alice-sub-pub.pgp'), '-v', srcs])
ret, _, _ = run_proc(RNP, ['--homedir', RNPDIR, '--keyfile', data_path(KEY_ALICE_SUB_PUB), '-v', srcs])
self.assertEqual(ret, 0)
ret, _, _ = run_proc(RNP, ['--homedir', RNPDIR, '--keyfile', data_path('test_key_validity/alice-sub-pub.pgp'), '-v', srcr])
ret, _, _ = run_proc(RNP, ['--homedir', RNPDIR, '--keyfile', data_path(KEY_ALICE_SUB_PUB), '-v', srcr])
self.assertEqual(ret, 0)
# Verify with GPG
[dst] = reg_workfiles('eddsa-zero', '.txt')
ret, _, _ = run_proc(GPG, ['--batch', '--homedir', GPGHOME, '--import', data_path('test_key_validity/alice-sub-pub.pgp')])
ret, _, _ = run_proc(GPG, ['--batch', '--homedir', GPGHOME, '--import', data_path(KEY_ALICE_SUB_PUB)])
self.assertEqual(ret, 0, 'gpg key import failed')
gpg_verify_file(srcs, dst, 'Alice <alice@rnp>')
os.remove(dst)
Expand All @@ -2806,19 +2807,20 @@ def test_verify_detached_source(self):
# Just verify
ret, _, err = run_proc(RNP, ['--homedir', keys, '-v', sig])
self.assertEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Good signature made.*e95a3cbf583aa80a2ccc53aa7bc6709b15c23a4a.*')
R_GOOD = r'(?s)^.*Good signature made.*e95a3cbf583aa80a2ccc53aa7bc6709b15c23a4a.*'
self.assertRegex(err, R_GOOD)
# Verify .asc
ret, _, err = run_proc(RNP, ['--homedir', keys, '-v', sigasc])
self.assertEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Good signature made.*e95a3cbf583aa80a2ccc53aa7bc6709b15c23a4a.*')
self.assertRegex(err, R_GOOD)
# Do not provide source
ret, _, err = run_proc(RNP, ['--homedir', keys, '-v', sig, '--source'])
self.assertEqual(ret, 1)
self.assertRegex(err, r'(?s)^.*rnp(|\.exe): option( .--source.|) requires an argument.*Usage: rnp --command \[options\] \[files\].*')
# Verify by specifying the correct path
ret, _, err = run_proc(RNP, ['--homedir', keys, '--source', src, '-v', sig])
self.assertEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Good signature made.*e95a3cbf583aa80a2ccc53aa7bc6709b15c23a4a.*')
self.assertRegex(err, R_GOOD)
# Verify by specifying the incorrect path
ret, _, err = run_proc(RNP, ['--homedir', keys, '--source', src + '.wrong', '-v', sig])
self.assertNotEqual(ret, 0)
Expand All @@ -2835,12 +2837,12 @@ def test_verify_detached_source(self):
srcdata = srcf.read().decode('utf-8')
ret, _, err = run_proc(RNP, ['--homedir', keys, '--source', '-', '-v', csig], srcdata)
self.assertEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Good signature made.*e95a3cbf583aa80a2ccc53aa7bc6709b15c23a4a.*')
self.assertRegex(err, R_GOOD)
# Verify by reading data from env
os.environ["SIGNED_DATA"] = srcdata
ret, _, err = run_proc(RNP, ['--homedir', keys, '--source', 'env:SIGNED_DATA', '-v', csig])
self.assertEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Good signature made.*e95a3cbf583aa80a2ccc53aa7bc6709b15c23a4a.*')
self.assertRegex(err, R_GOOD)
del os.environ["SIGNED_DATA"]
# Attempt to verify by specifying bot sig and data from stdin
sigtext = file_text(sigasc)
Expand Down Expand Up @@ -2890,33 +2892,35 @@ def test_onepass_edge_cases(self):
# Two one-passes and sig of the unknown version
ret, _, err = run_proc(RNP, ['--keyfile', key, '-v', data_path('test_messages/message.txt.signed-2-2-sig-v10')])
self.assertEqual(ret, 1)
self.assertRegex(err, r'(?s)^.*unknown signature version: 10.*failed to parse signature.*UNKNOWN signature.*Good signature made.*0451409669ffde3c.*')
self.assertRegex(err, r'(?s)^.*Signature verification failure: 0 invalid signature\(s\), 1 unknown signature\(s\).*')
R_VER_10 = r'(?s)^.*unknown signature version: 10.*failed to parse signature.*UNKNOWN signature.*Good signature made.*0451409669ffde3c.*'
R_1_UNK = r'(?s)^.*Signature verification failure: 1 unknown signature.*'
self.assertRegex(err, R_VER_10)
self.assertRegex(err, R_1_UNK)
# Two one-passes and sig of the unknown version (second)
ret, _, err = run_proc(RNP, ['--keyfile', key, '-v', data_path('test_messages/message.txt.signed-2-2-sig-v10-2')])
self.assertEqual(ret, 1)
self.assertRegex(err, r'(?s)^.*unknown signature version: 10.*failed to parse signature.*Good signature made.*0451409669ffde3c.*UNKNOWN signature.*')
self.assertRegex(err, r'(?s)^.*Signature verification failure: 0 invalid signature\(s\), 1 unknown signature\(s\).*')
self.assertRegex(err, R_1_UNK)
# 2 detached signatures, first is of version 10
ret, _, err = run_proc(RNP, ['--keyfile', key, '-v', data_path('test_messages/message.txt.2sigs'), '--source', data_path('test_messages/message.txt')])
self.assertEqual(ret, 1)
self.assertRegex(err, r'(?s)^.*unknown signature version: 10.*failed to parse signature.*UNKNOWN signature.*Good signature made.*0451409669ffde3c.*')
self.assertRegex(err, r'(?s)^.*Signature verification failure: 0 invalid signature\(s\), 1 unknown signature\(s\).*')
self.assertRegex(err, R_VER_10)
self.assertRegex(err, R_1_UNK)
# 2 detached signatures, second is of version 10
ret, _, err = run_proc(RNP, ['--keyfile', key, '-v', data_path('test_messages/message.txt.2sigs-2'), '--source', data_path('test_messages/message.txt')])
self.assertEqual(ret, 1)
self.assertRegex(err, r'(?s)^.*unknown signature version: 10.*failed to parse signature.*Good signature made.*0451409669ffde3c.*UNKNOWN signature.*')
self.assertRegex(err, r'(?s)^.*Signature verification failure: 0 invalid signature\(s\), 1 unknown signature\(s\).*')
self.assertRegex(err, R_1_UNK)
# Two cleartext signatures, first is of unknown version
ret, _, err = run_proc(RNP, ['--keyfile', key, '-v', data_path('test_messages/message.txt.clear-2-sigs')])
self.assertEqual(ret, 1)
self.assertRegex(err, r'(?s)^.*unknown signature version: 10.*failed to parse signature.*UNKNOWN signature.*Good signature made.*0451409669ffde3c.*')
self.assertRegex(err, r'(?s)^.*Signature verification failure: 0 invalid signature\(s\), 1 unknown signature\(s\).*')
self.assertRegex(err, R_VER_10)
self.assertRegex(err, R_1_UNK)
# Two cleartext signatures, second is of unknown version
ret, _, err = run_proc(RNP, ['--keyfile', key, '-v', data_path('test_messages/message.txt.clear-2-sigs-2')])
self.assertEqual(ret, 1)
self.assertRegex(err, r'(?s)^.*unknown signature version: 11.*failed to parse signature.*Good signature made.*0451409669ffde3c.*UNKNOWN signature.*')
self.assertRegex(err, r'(?s)^.*Signature verification failure: 0 invalid signature\(s\), 1 unknown signature\(s\).*')
self.assertRegex(err, R_1_UNK)

def test_pkesk_skesk_wrong_version(self):
key = data_path('test_stream_key_load/ecc-p256-sec.asc')
Expand Down Expand Up @@ -3386,7 +3390,7 @@ def test_encryption_aead_defs(self):
if not RNP_AEAD:
return
# Encrypt with RNP
pubkey = data_path('test_key_validity/alice-sub-pub.pgp')
pubkey = data_path(KEY_ALICE_SUB_PUB)
src, enc, dec = reg_workfiles('cleartext', '.txt', '.enc', '.dec')
random_text(src, 120000)
ret, _, _ = run_proc(RNP, ['--keyfile', pubkey, '-z', '0', '-r', 'alice', '--aead', '-e', src, '--output', enc])
Expand Down

0 comments on commit 2bd2597

Please sign in to comment.