Debian Bug report logs - #850025
python-crypto: regression: 2.6-4+deb7u4 breaks python-paramiko

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Thomas <thomasdn@gmail.com>

To: submit@bugs.debian.org

Subject: python-crypto: regression: 2.6-4+deb7u4 breaks python-paramiko

Date: Tue, 3 Jan 2017 10:45:02 +0100

[Message part 1 (text/plain, inline)]

Package: python-crypto
Version: 2.6-4+deb7u4
Severity: normal

Dear Maintainer,

After installing the recent security update for python-crypto
(2.6-4+deb7u4), the backup tool duplicity stopped working with SSH backend.
It reported the following errors:

----
Warning: Duplicity cleanup failed.
Warning: Duplicity remove-older-than failed.
Error: ssh: Unknown exception: CTR mode needs counter parameter, not IV
Error: ssh: Traceback (most recent call last):
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/transport.py",
line 1542, in run
Error: ssh: self.kex_engine.parse_next(ptype, m)
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/kex_group1.py",
line 68, in parse_next
Error: ssh: return self._parse_kexdh_reply(m)
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/kex_group1.py",
line 107, in _parse_kexdh_reply
Error: ssh: self.transport._activate_outbound()
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/transport.py",
line 1831, in _activate_outbound
Error: ssh: engine = self._get_cipher(self.local_cipher, key_out, IV_out)
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/transport.py",
line 1480, in _get_cipher
Error: ssh: return self._cipher_info[name]['class'].new(key,
self._cipher_info[name]['mode'], iv, counter)
Error: ssh: File "/usr/lib/python2.7/dist-packages/Crypto/Cipher/AES.py",
line 94, in new
Error: ssh: return AESCipher(key, *args, **kwargs)
Error: ssh: File "/usr/lib/python2.7/dist-packages/Crypto/Cipher/AES.py",
line 59, in __init__
Error: ssh: blockalgo.BlockAlgo.__init__(self, _AES, key, *args, **kwargs)
Error: ssh: File
"/usr/lib/python2.7/dist-packages/Crypto/Cipher/blockalgo.py", line 141, in
__init__
Error: ssh: self._cipher = factory.new(key, *args, **kwargs)
Error: ssh: ValueError: CTR mode needs counter parameter, not IV
Error: ssh:
Error: BackendException: ssh connection to backupserver.foonet:22 failed:
CTR mode needs counter parameter, not IV
Fatal: Duplicity failed.
----


It seems that python-crypto breaks python-paramiko. I am not sure if the
fault is at python-crypto or python-paramiko. I am reporting it here as it
was the update to python-crypto that seemed to have caused the problem.



A temporary workaround to this problem is to downgrade python-crypto to
version 2.6-4+deb7u3.


-- System Information:
Debian Release: 7.11
  APT prefers oldstable
  APT policy: (500, 'oldstable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-4-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_DK.UTF-8, LC_CTYPE=en_DK.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages python-crypto depends on:
ii  libc6     2.13-38+deb7u11
ii  libgmp10  2:5.0.5+dfsg-2
ii  python    2.7.3-4+deb7u1

python-crypto recommends no packages.

Versions of packages python-crypto suggests:
pn  python-crypto-dbg  <none>
pn  python-crypto-doc  <none>

-- no debconf information




Med venlig hilsen/Kind regards
Thomas

[Message part 2 (text/html, inline)]

Message #10 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Zed Pobre <zed.pobre@nasa.gov>

To: 850025@bugs.debian.org

Subject: Problem replicated (+ proposed patch)

Date: Tue, 3 Jan 2017 13:07:38 -0500

I'm also having this problem.  Some searching shows that the pycrypto
folks believe that this should be fixed in paramiko:

  https://github.com/dlitz/pycrypto/issues/149

A fellow who found a workaround on the paramiko side notes that the
pycrypto comments in AES.py are wrong now:

  http://uucode.com/blog/2015/02/20/workaround-for-ctr-mode-needs-counter-parameter-not-iv/

Despite that, I think I agree that paramiko needs to change.  The
problem is that this is a stable distribution, and the patch that
causes this problem, used to fix #849495, is really just attempting to
prevent bad usage by other programs, not inherently fixing a security
flaw.  In addition, the CTR component isn't actually dangerous, just
"confusing".

I propose that you remove the following from src/block_template.c:

++      if (IVlen != 0 && mode == MODE_CTR)
++      {
++              PyErr_Format(PyExc_ValueError,
++                      "CTR mode needs counter parameter, not IV");
++              return NULL;
++      }

Leave the rest.  That will still force it to die on the more dangerous
ECB misuse, but doesn't cause unexpected breakage in other packages
that are relying on being able to take shortcuts sending an IV string
even where one isn't needed.

Regards,
Zed

Message #15 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Sebastian Ramacher <sramacher@debian.org>

To: Zed Pobre <zed.pobre@nasa.gov>, 850025@bugs.debian.org

Cc: lamby@debian.org

Subject: Re: Bug#850025: Problem replicated (+ proposed patch)

Date: Wed, 4 Jan 2017 13:14:19 +0100

[Message part 1 (text/plain, inline)]

On 2017-01-03 13:07:38, Zed Pobre wrote:
> I'm also having this problem.  Some searching shows that the pycrypto
> folks believe that this should be fixed in paramiko:
> 
>   https://github.com/dlitz/pycrypto/issues/149
> 
> A fellow who found a workaround on the paramiko side notes that the
> pycrypto comments in AES.py are wrong now:
> 
>   http://uucode.com/blog/2015/02/20/workaround-for-ctr-mode-needs-counter-parameter-not-iv/
> 
> Despite that, I think I agree that paramiko needs to change.  The
> problem is that this is a stable distribution, and the patch that
> causes this problem, used to fix #849495, is really just attempting to
> prevent bad usage by other programs, not inherently fixing a security
> flaw.  In addition, the CTR component isn't actually dangerous, just
> "confusing".
> 
> I propose that you remove the following from src/block_template.c:
> 
> ++      if (IVlen != 0 && mode == MODE_CTR)
> ++      {
> ++              PyErr_Format(PyExc_ValueError,
> ++                      "CTR mode needs counter parameter, not IV");
> ++              return NULL;
> ++      }

No, dropping thas would open up the vulnerability again. For jessie the
exception was downgraded to a warning and IVlen set to 0.

For wheezy LTS I sent the updated patch to Chris Lamb (CCed). I'd expect an
update there soon.

Regards

> Leave the rest.  That will still force it to die on the more dangerous
> ECB misuse, but doesn't cause unexpected breakage in other packages
> that are relying on being able to take shortcuts sending an IV string
> even where one isn't needed.
> 
> Regards,
> Zed

-- 
Sebastian Ramacher

[signature.asc (application/pgp-signature, inline)]

Message #20 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 850025@bugs.debian.org, 850077@bugs.debian.org

Subject: Re: python-crypto: regression: 2.6-4+deb7u4 breaks python-paramiko

Date: Wed, 04 Jan 2017 17:11:06 +0000

tags 850025 + pending
tags 850077 + pending
thanks

Just uploaded.


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #27 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Thomas <thomasdn@gmail.com>

To: 850025@bugs.debian.org

Subject: Still a problem in 2.6-4+deb7u5

Date: Wed, 4 Jan 2017 20:07:09 +0100

[Message part 1 (text/plain, inline)]

Hi,

I just installed the newly updated 2.6-4+deb7u5 version of python-crypto.
Unfortunately, the problem persists.


Output from duplicity with most recent version:


-----------
Error: ssh: Unknown exception: CTR mode needs counter parameter, not IV
Error: ssh: Traceback (most recent call last):
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/transport.py",
line 1542, in run
Error: ssh: self.kex_engine.parse_next(ptype, m)
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/kex_group1.py",
line 68, in parse_next
Error: ssh: return self._parse_kexdh_reply(m)
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/kex_group1.py",
line 107, in _parse_kexdh_reply
Error: ssh: self.transport._activate_outbound()
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/transport.py",
line 1831, in _activate_outbound
Error: ssh: engine = self._get_cipher(self.local_cipher, key_out, IV_out)
Error: ssh: File "/usr/lib/python2.7/dist-packages/paramiko/transport.py",
line 1480, in _get_cipher
Error: ssh: return self._cipher_info[name]['class'].new(key,
self._cipher_info[name]['mode'], iv, counter)
Error: ssh: File "/usr/lib/python2.7/dist-packages/Crypto/Cipher/AES.py",
line 94, in new
Error: ssh: return AESCipher(key, *args, **kwargs)
Error: ssh: File "/usr/lib/python2.7/dist-packages/Crypto/Cipher/AES.py",
line 59, in __init__
Error: ssh: blockalgo.BlockAlgo.__init__(self, _AES, key, *args, **kwargs)
Error: ssh: File
"/usr/lib/python2.7/dist-packages/Crypto/Cipher/blockalgo.py", line 141, in
__init__
Error: ssh: self._cipher = factory.new(key, *args, **kwargs)
Error: ssh: ValueError: CTR mode needs counter parameter, not IV
Error: ssh:
Error: BackendException: ssh connection to backup.foo:22 failed: CTR mode
needs counter parameter, not IV

-----------


# dpkg -l python-crypto
Desired=Unknown/Install/Remove/Purge/Hold
|
Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                                                           Version
                             Architecture
Description
+++-==============================================================-====================================-====================================-==================================================================================================================================
ii  python-crypto
 2.6-4+deb7u5                         amd64
 cryptographic algorithms and protocols for Python






Med venlig hilsen/Kind regards
Thomas

[Message part 2 (text/html, inline)]

Message #32 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Chris Lamb <lamby@debian.org>

To: 850025@bugs.debian.org

Cc: Thomas <thomasdn@gmail.com>

Subject: Re: python-crypto: regression: 2.6-4+deb7u4 breaks python-paramiko

Date: Thu, 05 Jan 2017 10:38:39 +0000

tags 850025 + pending
tags 850077 + pending
thanks

Thomas wrote:

> I just installed the newly updated 2.6-4+deb7u5 version of python-crypto.
> Unfortunately, the problem persists.

Fix pending upload…


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

Message #37 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Zed Pobre <zed.pobre@nasa.gov>

To: Sebastian Ramacher <sramacher@debian.org>

Cc: 850025@bugs.debian.org, lamby@debian.org

Subject: Re: Bug#850025: Problem replicated (+ proposed patch)

Date: Thu, 5 Jan 2017 16:53:44 -0500

On Wed, Jan 04, 2017 at 01:14:19PM +0100, Sebastian Ramacher wrote:
> On 2017-01-03 13:07:38, Zed Pobre wrote:
> > Despite that, I think I agree that paramiko needs to change.  The
> > problem is that this is a stable distribution, and the patch that
> > causes this problem, used to fix #849495, is really just attempting to
> > prevent bad usage by other programs, not inherently fixing a security
> > flaw.  In addition, the CTR component isn't actually dangerous, just
> > "confusing".
> >
> > I propose that you remove the following from src/block_template.c:
> >
> > ++      if (IVlen != 0 && mode == MODE_CTR)
> > ++      {
> > ++              PyErr_Format(PyExc_ValueError,
> > ++                      "CTR mode needs counter parameter, not IV");
> > ++              return NULL;
> > ++      }
>
> No, dropping thas would open up the vulnerability again. For jessie the
> exception was downgraded to a warning and IVlen set to 0.
>
> For wheezy LTS I sent the updated patch to Chris Lamb (CCed). I'd expect an
> update there soon.

Unfortunately, python-crypto 2.6-4+deb7u5 does not fix the problem.  I
would like to reiterate that as far as CTR is concerned *there is no
vulnerability* according to all the discussion I have read on it thus
far -- at worst, it could be considered "confusing" and lead to
third-party misuse of the library, no examples of which in the wild
have yet been presented.

The main threat came from ECB.  Removing the MODE_CTR IVlen check will
not affect that.

Unless the deb7u5 release is found to have incorrectly applied the
'downgraded to warning' patch and can be easily fixed, please
reconsider reverting just the MODE_CTR component to prevent
regression, especially considering that the alternative that most
sites will have to take is to run:

  apt-get install python-crypto=2.6-4+deb7u3
  echo "python-crypto hold" | dpkg --set-selections

and go about their business, which is even worse from a security
standpoint (now they don't even get protection from ECB misuse).

Regards,
Zed

Message #42 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Sebastian Ramacher <sramacher@debian.org>

To: Zed Pobre <zed.pobre@nasa.gov>

Cc: 850025@bugs.debian.org, lamby@debian.org

Subject: Re: Bug#850025: Problem replicated (+ proposed patch)

Date: Fri, 6 Jan 2017 13:53:47 +0100

[Message part 1 (text/plain, inline)]

Hi Zed

On 2017-01-05 16:53:44, Zed Pobre wrote:
> On Wed, Jan 04, 2017 at 01:14:19PM +0100, Sebastian Ramacher wrote:
> > On 2017-01-03 13:07:38, Zed Pobre wrote:
> > > Despite that, I think I agree that paramiko needs to change.  The
> > > problem is that this is a stable distribution, and the patch that
> > > causes this problem, used to fix #849495, is really just attempting to
> > > prevent bad usage by other programs, not inherently fixing a security
> > > flaw.  In addition, the CTR component isn't actually dangerous, just
> > > "confusing".
> > >
> > > I propose that you remove the following from src/block_template.c:
> > >
> > > ++      if (IVlen != 0 && mode == MODE_CTR)
> > > ++      {
> > > ++              PyErr_Format(PyExc_ValueError,
> > > ++                      "CTR mode needs counter parameter, not IV");
> > > ++              return NULL;
> > > ++      }
> >
> > No, dropping thas would open up the vulnerability again. For jessie the
> > exception was downgraded to a warning and IVlen set to 0.
> >
> > For wheezy LTS I sent the updated patch to Chris Lamb (CCed). I'd expect an
> > update there soon.
> 
> Unfortunately, python-crypto 2.6-4+deb7u5 does not fix the problem.

It was fixed in 2-6-4+deb7u6.

> I
> would like to reiterate that as far as CTR is concerned *there is no
> vulnerability* according to all the discussion I have read on it thus
> far -- at worst, it could be considered "confusing" and lead to
> third-party misuse of the library, no examples of which in the wild
> have yet been presented.

Then perhaps the discussions you read are wrong or incomplete. Maybe there is no
exploitable code out there using CTR mode, but it can be used to trigger the
very same buffer overflow as with ECB. Here is the interesting part of AES.new
before applying IVlen checks for ECB and CTR:

	if (IVlen != BLOCK_SIZE && mode != MODE_ECB && mode != MODE_CTR)
	{
		PyErr_Format(PyExc_ValueError,
			     "IV must be %i bytes long", BLOCK_SIZE);
		return NULL;
	}

This is the only IVlen check and does not have any effect in ECB or CTR mode.

	if (mode == MODE_CTR) {
		if (counter == NULL) {
			PyErr_SetString(PyExc_TypeError,
					"'counter' keyword parameter is required with CTR mode");
			return NULL;
#ifdef IS_PY3K
		} else if (PyObject_HasAttr(counter, PyUnicode_FromString("__PCT_CTR_SHORTCUT__"))) {
#else
		} else if (PyObject_HasAttrString(counter, "__PCT_CTR_SHORTCUT__")) {
#endif
			counter_shortcut = 1;
		} else if (!PyCallable_Check(counter)) {
			PyErr_SetString(PyExc_ValueError, 
					"'counter' parameter must be a callable object");
			return NULL;
		}
	} else {
		if (counter != NULL) {
			PyErr_SetString(PyExc_ValueError, 
					"'counter' parameter only useful with CTR mode");
			return NULL;
		}
	}

In CTR mode, only the presence of counter is checked.

	memset(new->IV, 0, BLOCK_SIZE);
	memset(new->oldCipher, 0, BLOCK_SIZE);
	memcpy(new->IV, IV, IVlen);

The last memcpy now causes a buffer overflow in case of MODE_CTR and MODE_ECB
since IVlen is never checked and completely controlled by a potential adversary.

Cheers
-- 
Sebastian Ramacher

[signature.asc (application/pgp-signature, inline)]

Message #47 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Zed Pobre <zed.pobre@nasa.gov>

To: Sebastian Ramacher <sramacher@debian.org>

Cc: 850025@bugs.debian.org, lamby@debian.org

Subject: Re: Bug#850025: Problem replicated (+ proposed patch)

Date: Fri, 6 Jan 2017 11:37:24 -0500

On Fri, Jan 06, 2017 at 01:53:47PM +0100, Sebastian Ramacher wrote:
> It was fixed in 2-6-4+deb7u6.

Thanks, I can confirm that this fixes the problem.


> Then perhaps the discussions you read are wrong or incomplete. Maybe there is no
> exploitable code out there using CTR mode, but it can be used to trigger the
> very same buffer overflow as with ECB. Here is the interesting part of AES.new
> before applying IVlen checks for ECB and CTR:

You are correct, the discussion I was reading was apparently quite
incomplete.  Thank you for the thorough explanation, and my apologies
for my error.

Regards,
Zed

Message #52 received at 850025@bugs.debian.org (full text, mbox, reply):

From: Thomas <thomasdn@gmail.com>

To: 850025@bugs.debian.org

Subject: Works

Date: Fri, 6 Jan 2017 20:45:50 +0100

[Message part 1 (text/plain, inline)]

Version 2.6-4+deb7u6 works!


Med venlig hilsen/Kind regards
Thomas

[Message part 2 (text/html, inline)]

Send a report that this bug log contains spam.