simon-git: putty (master): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Sun Feb 10 13:19:51 GMT 2019
TL;DR:
30117bff Add primegen() to the testcrypt API.
f6596142 ecc.[ch]: add elliptic-curve point_copy_into functions.
40843b43 dss_sign(): fix a theoretically possible overflow.
f133abe5 Give a sensible error when using a too-short RSA key.
bc11f74c Stop aborting the connection if Pageant won't sign.
fa4a7dd3 Stop falling back to 16-bit BignumInt on VS Arm builds.
83db341e New test system to detect side channels in crypto code.
9cb8c4bc mp_cond_swap: add a brute-force 'volatile'.
Repository: https://git.tartarus.org/simon/putty.git
On the web: https://git.tartarus.org/?p=simon/putty.git
Branch updated: master
Committer: Simon Tatham <anakin at pobox.com>
Date: 2019-02-10 13:19:51
commit 30117bff5536a9a9cb1612c10b3fdcf4eb0bacd7
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=30117bff5536a9a9cb1612c10b3fdcf4eb0bacd7;hp=03492ab59369dc8347d8eb8693da548b5e27cf0b
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Feb 9 15:35:02 2019 +0000
Add primegen() to the testcrypt API.
I just found I wanted to generate a prime with particular properties,
and I knew PuTTY's prime generator could manage it, so it was easier
to add this function to testcrypt for occasional manual use than to
look for another prime-generator with the same feature set!
I've wrapped the function so as to remove the three progress-
reporting parameters.
Recipe | 6 ++++--
testcrypt.c | 23 +++++++++++++++++++++++
testcrypt.h | 1 +
3 files changed, 28 insertions(+), 2 deletions(-)
commit f6596142726ed1104877398126a4e0b3824c82a6
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f6596142726ed1104877398126a4e0b3824c82a6;hp=30117bff5536a9a9cb1612c10b3fdcf4eb0bacd7
Author: Simon Tatham <anakin at pobox.com>
Date: Sat Feb 9 17:50:27 2019 +0000
ecc.[ch]: add elliptic-curve point_copy_into functions.
This will let my upcoming new test of memory access patterns run a
sequence of tests on different elliptic-curve data which is stored at
the same address each time.
ecc.c | 23 +++++++++++++++++++++++
ecc.h | 5 +++++
2 files changed, 28 insertions(+)
commit 40843b432ae8975c171d78aa2ecc68b8977ed189
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=40843b432ae8975c171d78aa2ecc68b8977ed189;hp=f6596142726ed1104877398126a4e0b3824c82a6
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Feb 10 08:08:50 2019 +0000
dss_sign(): fix a theoretically possible overflow.
I computed hash + x*r by first computing x*r, and then using
mp_add_into to add the hash to it in the same bignum. But if the
result of x*r had been allocated an mp_int only just large enough to
contain it, then the addition of the hash might have made it overflow
and generated a bogus signature.
I've never seen that happen, and for all I know word sizes may make it
completely impossible. But it's a theoretical possibility, and easy to
fix now that I've happened to spot it in passing.
sshdss.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
commit f133abe5218a1a0ff8b4640c6bd7100a50446ae7
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f133abe5218a1a0ff8b4640c6bd7100a50446ae7;hp=40843b432ae8975c171d78aa2ecc68b8977ed189
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Feb 10 08:44:59 2019 +0000
Give a sensible error when using a too-short RSA key.
The ssh_signkey vtable has grown a new method ssh_key_invalid(), which
checks whether the key is going to be usable for constructing a
signature at all. Currently the only way this can fail is if it's an
RSA key so short that there isn't room to put all the PKCS#1
formatting in the signature preimage integer, but the return value is
an arbitrary error message just in case more reasons are needed later.
This is tested separately rather than at key-creation time because of
the signature flags system: an RSA key of intermediate length could be
valid for SHA-1 signing but not for SHA-512. So really this method
should be called at the point where you've decided what sig flags you
want to use, and you're checking if _those flags_ are OK.
On the verification side, there's no need for a separate check. If
someone presents us with an RSA key so short that it's impossible to
encode a valid signature using it, then we simply regard all
signatures as invalid.
misc.h | 2 +
pageant.c | 9 +++
ssh.h | 2 +
ssh2userauth.c | 17 +++++
sshdss.c | 7 ++
sshecc.c | 11 ++++
sshrsa.c | 197 +++++++++++++++++++++++++++++++++++---------------------
testcrypt.c | 8 +++
testcrypt.h | 1 +
unix/uxserver.c | 6 ++
10 files changed, 186 insertions(+), 74 deletions(-)
commit bc11f74c7482f369c920222836584acd6d99e029
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=bc11f74c7482f369c920222836584acd6d99e029;hp=f133abe5218a1a0ff8b4640c6bd7100a50446ae7
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Feb 10 08:51:36 2019 +0000
Stop aborting the connection if Pageant won't sign.
There's been a FIXME comment in there for ages saying we should do
something less drastic than ssh_sw_abort(). This actually came up in
the course of testing Pageant's support for the new RSA validity
check, so I've fixed it: if Pageant won't deliver us a signature from
the private key we'd like, then we treat it the same as any other auth
method failure: shrug and move on to the next method on our list (or
even just the next key in Pageant).
ssh2userauth.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
commit fa4a7dd3d544105d2ad2026cecd9e16b9ce633c7
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=fa4a7dd3d544105d2ad2026cecd9e16b9ce633c7;hp=bc11f74c7482f369c920222836584acd6d99e029
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Feb 10 09:02:00 2019 +0000
Stop falling back to 16-bit BignumInt on VS Arm builds.
The case previously conditioned on _M_IX86, where we use __int64 as
the BignumDblInt type, is actually valid on any Visual Studio target
platform at all, so it's safe to remove that condition and let it
apply to _M_ARM and _M_ARM64 as well. The only situation in which we
_shouldn't_ use that case for Visual Studio builds is when we have
something even better available, such as the x86-64 intrinsics for
add-with-carry and double-width multiply.
mpint_i.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
commit 83db341e8aa17694646b68071ed7d230a1abf9bd
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=83db341e8aa17694646b68071ed7d230a1abf9bd;hp=fa4a7dd3d544105d2ad2026cecd9e16b9ce633c7
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Feb 10 13:09:53 2019 +0000
New test system to detect side channels in crypto code.
All the work I've put in in the last few months to eliminate timing
and cache side channels from PuTTY's mp_int and cipher implementations
has been on a seat-of-the-pants basis: just thinking very hard about
what kinds of language construction I think would be safe to use, and
trying not to absentmindedly leave a conditional branch or a cast to
bool somewhere vital.
Now I've got a test suite! The basic idea is that you run the same
crypto primitive multiple times, with inputs differing only in ways
that are supposed to avoid being leaked by timing or leaving evidence
in the cache; then you instrument the code so that it logs all the
control flow, memory access and a couple of other relevant things in
each of those runs, and finally, compare the logs and expect them to
be identical.
The instrumentation is done using DynamoRIO, which I found to be well
suited to this kind of work: it lets you define custom modifications
of the code in a reasonably low-effort way, and it lets you work at
both the low level of examining single instructions _and_ the higher
level of the function call ABI (so you can give things like malloc
special treatment, not to mention intercepting communications from the
program being instrumented). Build instructions are all in the comment
at the top of testsc.c.
At present, I've found this test to give a 100% pass rate using gcc
-O0 and -O3 (Ubuntu 18.10). With clang, there are a couple of
failures, which I'll fix in the next commit.
.gitignore | 1 +
Recipe | 14 +-
test/sclog/.gitignore | 7 +
test/sclog/CMakeLists.txt | 16 +
test/sclog/sclog.c | 592 +++++++++++++++++
testsc.c | 1558 +++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 2182 insertions(+), 6 deletions(-)
commit 9cb8c4bcb78fd3388f10ed626e853a225c33faed
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=9cb8c4bcb78fd3388f10ed626e853a225c33faed;hp=83db341e8aa17694646b68071ed7d230a1abf9bd
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Feb 10 13:16:12 2019 +0000
mp_cond_swap: add a brute-force 'volatile'.
With this change, my new side-channel test system gets a 100% pass
rate when compiled with clang -O3 on Ubuntu 18.10. Previously, it had
three failing tests (namely the three ECC multiply functions), all due
to inconsistent control flow inside mp_cond_swap.
I admit I don't really understand whether this is really necessary or
not, so I'm playing it safe. The problem _seems_ to be that clang has
generated one version of the cond_swap loop using integer arithmetic
and another using MMX vectors, so the obvious suspect is alignment -
probably mp_cond_swap is processing an iteration of the loop up front
until its pointer is 16-byte aligned and then switching over to the
vectorised version. But on the other hand, when I experimentally tried
forcing allocations to be 16- or even 32-byte aligned, it didn't make
a difference. And I don't speak x86 vector instructions very well (in
fact barely at all), so I'm not even completely sure of whether the
code I was reading did what I thought it did; so I'm more comfortable
with simply applying brute force to get some code generation that the
automated test is genuinely happy with.
mpint.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
More information about the tartarus-commits
mailing list