simon-git: putty (master): Simon Tatham

Commits to Tartarus hosted VCS tartarus-commits at lists.tartarus.org
Thu Jan 3 14:41:48 GMT 2019


TL;DR:
  f081885b Move standalone parts of misc.c into utils.c.
  f2174536 Switch sresize to using TYPECHECK.
  4397016a Fix use-after-free when using ChaCha20 + Poly1305.
  c02031ff New marshalling function put_datapl().
  d169b04d New helper function ptrlen_strcmp().
  febef916 Make ssh2_mac_setkey take the key as a ptrlen.
  a2d1c211 Replace more (pointer, length) arg pairs with ptrlen.
  2bd76ed8 Tidy up the API for RSA key exchange.
  0d9ab2f1 Fix aliasing bug in mp_lshift_fixed_into.
  34d78286 modsqrt: return success if taking square root of 0.
  df1ed3ba Fix goof in mp_reduce_mod_2to.
  425a119a Fix special case when mp_modsub returns zero.

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-01-03 14:41:48

commit f081885bc070f3293667f91864287d88a4bbecd4
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f081885bc070f3293667f91864287d88a4bbecd4;hp=0112936ef7bc7bf4de39a17ce8b4cc37a9825f83
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 08:44:11 2019 +0000

    Move standalone parts of misc.c into utils.c.
    
    misc.c has always contained a combination of things that are tied
    tightly into the PuTTY code base (e.g. they use the conf system, or
    work with our sockets abstraction) and things that are pure standalone
    utility functions like nullstrcmp() which could quite happily be
    dropped into any C program without causing a link failure.
    
    Now the latter kind of standalone utility code lives in the new source
    file utils.c, whose only external dependency is on memory.c (for snew,
    sfree etc), which in turn requires the user to provide an
    out_of_memory() function. So it should now be much easier to link test
    programs that use PuTTY's low-level functions without also pulling in
    half its bulky infrastructure.
    
    In the process, I came across a memory allocation logging system
    enabled by -DMALLOC_LOG that looks long since bit-rotted; in any case
    we have much more advanced tools for that kind of thing these days,
    like valgrind and Leak Sanitiser, so I've just removed it rather than
    trying to transplant it somewhere sensible. (We can always pull it
    back out of the version control history if really necessary, but I
    haven't used it in at least a decade.)
    
    The other slightly silly thing I did was to give bufchain a function
    pointer field that points to queue_idempotent_callback(), and disallow
    direct setting of the 'ic' field in favour of calling
    bufchain_set_callback which will fill that pointer in too. That allows
    the bufchain system to live in utils.c rather than misc.c, so that
    programs can use it without also having to link in the callback system
    or provide an annoying stub of that function. In fact that's just
    allowed me to remove stubs of that kind from PuTTYgen and Pageant!

 Recipe            |  16 +-
 cmdgen.c          |   4 -
 memory.c          |  51 +--
 misc.c            | 974 +-----------------------------------------------------
 misc.h            |  15 +
 mpint.c           |   3 +-
 puttymem.h        |  20 +-
 ssh.c             |   2 +-
 sshserver.c       |   2 +-
 testzlib.c        |   9 +-
 utils.c           | 961 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 windows/winpgen.c |   5 -
 windows/winpgnt.c |   5 -
 13 files changed, 1025 insertions(+), 1042 deletions(-)

commit f2174536a8a48524a334ab6d760d57afdf751aea
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=f2174536a8a48524a334ab6d760d57afdf751aea;hp=f081885bc070f3293667f91864287d88a4bbecd4
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 10:51:52 2019 +0000

    Switch sresize to using TYPECHECK.
    
    Now that that facility is centralised in defs.h, there's no reason to
    have a special ad-hoc version in sresize and separately comment it.

 puttymem.h | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

commit 4397016a519aadcf7bd3936e02fd4c96d68616a7
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=4397016a519aadcf7bd3936e02fd4c96d68616a7;hp=f2174536a8a48524a334ab6d760d57afdf751aea
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 13:57:54 2019 +0000

    Fix use-after-free when using ChaCha20 + Poly1305.
    
    When we're running with that cipher/MAC combination, what ssh2bpp.c
    sees as separate cipher and MAC objects are actually just different
    interfaces of the same object: the Poly1305 constructor function just
    returns an alternate view of the cipher object it's passed, and the
    free function does nothing.
    
    But Address Sanitiser points out that if we first call
    ssh2_cipher_free and then ssh2_mac_free, then although poly1305_free
    would do nothing once we reached it, we can't actually reach it
    without looking up the vtable in the object we've just thrown away.
    
    Easily fixed by reversing the order of mac_free and cipher_free in the
    BPP. Also I've centralised that freeing so that it doesn't have to be
    repeated with the same careful ordering in the BPP cleanup function
    and the functions that replace a direction of crypto.

 ssh2bpp.c | 48 +++++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 19 deletions(-)

commit c02031ffd6a23aba11b931f1f9e2f1eb01fa44b8
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=c02031ffd6a23aba11b931f1f9e2f1eb01fa44b8;hp=4397016a519aadcf7bd3936e02fd4c96d68616a7
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Jan 1 19:00:19 2019 +0000

    New marshalling function put_datapl().
    
    Just like put_data(), but takes a ptrlen rather than separate ptr and
    len arguments, so it saves a bit of repetition at call sites. I
    probably should have written this ages ago, but better late than
    never; I've also converted every call site I can find that needed it.

 import.c              |  2 +-
 marshal.c             |  5 +++++
 marshal.h             |  3 +++
 pageant.c             |  4 ++--
 scpserver.c           |  5 ++---
 ssh2userauth-server.c |  2 +-
 ssh2userauth.c        |  6 +++---
 sshecc.c              | 10 +++++-----
 sshrsa.c              |  2 +-
 x11fwd.c              |  2 +-
 10 files changed, 24 insertions(+), 17 deletions(-)

commit d169b04dba065a32b6f8e3df1ab0f2a7ca5444d0
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=d169b04dba065a32b6f8e3df1ab0f2a7ca5444d0;hp=c02031ffd6a23aba11b931f1f9e2f1eb01fa44b8
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Jan 1 19:07:00 2019 +0000

    New helper function ptrlen_strcmp().
    
    Compares strings with the same semantics as strcmp, but uses ptrlens
    as input instead of zero-terminated strings.

 misc.h  |  1 +
 utils.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

commit febef916a51463f628737efc394f02d051895704
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=febef916a51463f628737efc394f02d051895704;hp=d169b04dba065a32b6f8e3df1ab0f2a7ca5444d0
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 13:49:02 2019 +0000

    Make ssh2_mac_setkey take the key as a ptrlen.
    
    This makes the API more flexible, so that it's not restricted to
    taking a key of precisely the length specified in the ssh2_macalg
    structure. Instead, ssh2bpp looks up that length to construct the
    MAC's key.
    
    Some MACs (e.g. Poly1305) will only _work_ with a single key length.
    But this way, I can run standard test vectors against MACs that can
    take a variable length (e.g. everything in the HMAC family).

 ssh.h      |  2 +-
 ssh2bpp.c  |  4 ++--
 sshccp.c   | 15 ++++++++-------
 sshmd5.c   |  4 ++--
 sshsh256.c |  4 ++--
 sshsha.c   |  7 ++-----
 6 files changed, 17 insertions(+), 19 deletions(-)

commit a2d1c211a719bcc3f06a30a8f1c0292db3fa0929
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=a2d1c211a719bcc3f06a30a8f1c0292db3fa0929;hp=febef916a51463f628737efc394f02d051895704
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Jan 1 21:07:48 2019 +0000

    Replace more (pointer, length) arg pairs with ptrlen.
    
    The abstract method ssh_key_sign(), and the concrete functions
    ssh_rsakex_newkey() and rsa_ssh1_public_blob_len(), now each take a
    ptrlen argument in place of a separate pointer and length pair.
    
    Partly that's because I'm generally preferring ptrlens these days and
    it keeps argument lists short and tidy-looking, but mostly it's
    because it will make those functions easier to wrap in my upcoming
    test system.

 pageant.c        |  5 +++--
 ssh.h            | 11 +++++------
 ssh2kex-client.c |  2 +-
 ssh2kex-server.c |  5 +++--
 ssh2userauth.c   |  2 +-
 sshdss.c         |  5 ++---
 sshecc.c         | 13 ++++++-------
 sshrsa.c         | 13 ++++++-------
 8 files changed, 27 insertions(+), 29 deletions(-)

commit 2bd76ed88c41eecee06e00878dca97a74db92e6d
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=2bd76ed88c41eecee06e00878dca97a74db92e6d;hp=a2d1c211a719bcc3f06a30a8f1c0292db3fa0929
Author: Simon Tatham <anakin at pobox.com>
Date:   Wed Jan 2 08:39:16 2019 +0000

    Tidy up the API for RSA key exchange.
    
    ssh_rsakex_encrypt took an input (pointer, length) pair, which I've
    replaced with a ptrlen; it also took an _output_ (pointer, length)
    pair, and then re-computed the right length internally and enforced by
    assertion that the one passed in matched it. Now it just returns a
    strbuf of whatever length it computed, which saves the caller having
    to compute the length at all.
    
    Also, both ssh_rsakex_encrypt and ssh_rsakex_decrypt took their
    arguments in a weird order; I think it looks more sensible to put the
    RSA key first rather than last, so now they both have the common order
    (key, hash, input data).

 ssh.h            |  9 ++++-----
 ssh2kex-client.c | 15 +++++----------
 ssh2kex-server.c |  2 +-
 sshrsa.c         | 20 +++++++++++---------
 4 files changed, 21 insertions(+), 25 deletions(-)

commit 0d9ab2f14bb73ebaf7566c6a073d6971c917eee7
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=0d9ab2f14bb73ebaf7566c6a073d6971c917eee7;hp=2bd76ed88c41eecee06e00878dca97a74db92e6d
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 13:08:44 2019 +0000

    Fix aliasing bug in mp_lshift_fixed_into.
    
    I had not considered what would happen if the input and output mp_ints
    aliased each other. What happens is that because I wrote the output
    starting from the low end, input words would get corrupted before I'd
    finished with them. Easily fixed by reversing the order of the loop.

 mpint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

commit 34d78286e6d096695d731428022bd42ea46df05d
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=34d78286e6d096695d731428022bd42ea46df05d;hp=0d9ab2f14bb73ebaf7566c6a073d6971c917eee7
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 13:10:26 2019 +0000

    modsqrt: return success if taking square root of 0.
    
    My test for whether x has a square root was based on testing whether a
    large power of x was congruent to 1 mod p, which is a fine test
    provided x is in the multiplicative group of p, but would give a false
    negative on the one possible input value that _isn't_ - namely zero.
    
    The actual number returned from the function is fine (because that too
    is a large power of the input, and when the input is 0 that's
    foolproof). So I just needed to add a special case for the returned
    'success' flag.

 mpint.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

commit df1ed3ba6e7b683fedf1de17b8cbd951937f7196
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=df1ed3ba6e7b683fedf1de17b8cbd951937f7196;hp=34d78286e6d096695d731428022bd42ea46df05d
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 10:37:19 2019 +0000

    Fix goof in mp_reduce_mod_2to.
    
    It correctly masked off bits in the partial word, but then left all
    higher words _unchanged_ rather than zeroing them.
    
    Apparently its use in mp_invert_mod_2to was in restricted enough
    circumstances not to cause a failure there!

 mpint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

commit 425a119ae83910367bdba7343c43ceb631289872
web diff https://git.tartarus.org/?p=simon/putty.git;a=commitdiff;h=425a119ae83910367bdba7343c43ceb631289872;hp=df1ed3ba6e7b683fedf1de17b8cbd951937f7196
Author: Simon Tatham <anakin at pobox.com>
Date:   Thu Jan 3 11:53:38 2019 +0000

    Fix special case when mp_modsub returns zero.
    
    If it had to negate x-y to make it positive for mp_mod, but the answer
    comes out as zero after that, then after re-negating it this is the
    one case where we _shouldn't_ add the modulus afterwards. Result was
    that, for example, mp_modsub(0, 0, 5) would return 5 instead of the
    obvious 0.

 mpint.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)



More information about the tartarus-commits mailing list