Counter: Deprecate disable_shortcut; Remove __PCT_CTR_SHORTCUT__ entirely

The `disable_shortcut` option served as a workaround in case
`__PCT_CTR_SHORTCUT__` leaked through a wrapper object, but I don't
think anyone actually used it, and it was a bad idea to expose it as
part of the public API.

Now that we do strong type checking inside block_template.c, there
shoujld be no need to ever use this option.  It's now a no-op, retained
for backward compatibility only.  It will be removed in some future
version of PyCrypto.

4 files changed, 56 insertions, 30 deletions

diff --git a/lib/Crypto/SelfTest/Cipher/common.py b/lib/Crypto/SelfTest/Cipher/common.py
index 26fabed..4d6edf0 100644
--- a/lib/Crypto/SelfTest/Cipher/common.py
+++ b/lib/Crypto/SelfTest/Cipher/common.py
@@ -24,6 +24,8 @@
 
 """Self-testing for PyCrypto hash modules"""
 
+from __future__ import nested_scopes
+
 __revision__ = "$Id$"
 
 import sys
@@ -195,18 +197,43 @@ class CTRWraparoundTest(unittest.TestCase):
         self.module_name = params.get('module_name', None)
 
     def shortDescription(self):
-        return """Regression test: %s with MODE_CTR should raise OverflowError on wraparound when shortcut used""" % (self.module_name,)
+        return """Regression test: %s with MODE_CTR raising OverflowError on wraparound""" % (self.module_name,)
 
     def runTest(self):
         from Crypto.Util import Counter
 
-        for disable_shortcut in (0, 1): # (False, True) Test CTR-mode shortcut and PyObject_CallObject code paths
-            for little_endian in (0, 1): # (False, True) Test both endiannesses
-                ctr = Counter.new(8*self.module.block_size, initial_value=2L**(8*self.module.block_size)-1, little_endian=little_endian, disable_shortcut=disable_shortcut)
-                cipher = self.module.new(a2b_hex(self.key), self.module.MODE_CTR, counter=ctr)
-                block = b("\x00") * self.module.block_size
-                cipher.encrypt(block)
-                self.assertRaises(OverflowError, cipher.encrypt, block)
+        def pythonCounter():
+            state = [0]
+            def ctr():
+                # First block succeeds; Second and subsequent blocks raise OverflowError
+                if state[0] == 0:
+                    state[0] = 1
+                    return b("\xff") * self.module.block_size
+                else:
+                    raise OverflowError
+            return ctr
+
+        for little_endian in (0, 1): # (False, True) Test both endiannesses
+            block = b("\x00") * self.module.block_size
+
+            # Test PyObject_CallObject code path: if the counter raises OverflowError
+            cipher = self.module.new(a2b_hex(self.key), self.module.MODE_CTR, counter=pythonCounter())
+            cipher.encrypt(block)
+            self.assertRaises(OverflowError, cipher.encrypt, block)
+            self.assertRaises(OverflowError, cipher.encrypt, block)
+
+            # Test PyObject_CallObject code path: counter object should raise OverflowError
+            ctr = Counter.new(8*self.module.block_size, initial_value=2L**(8*self.module.block_size)-1, little_endian=little_endian)
+            ctr()
+            self.assertRaises(OverflowError, ctr)
+            self.assertRaises(OverflowError, ctr)
+
+            # Test the CTR-mode shortcut
+            ctr = Counter.new(8*self.module.block_size, initial_value=2L**(8*self.module.block_size)-1, little_endian=little_endian)
+            cipher = self.module.new(a2b_hex(self.key), self.module.MODE_CTR, counter=ctr)
+            cipher.encrypt(block)
+            self.assertRaises(OverflowError, cipher.encrypt, block)
+            self.assertRaises(OverflowError, cipher.encrypt, block)
 
 class CFBSegmentSizeTest(unittest.TestCase):
 
diff --git a/lib/Crypto/SelfTest/Util/test_Counter.py b/lib/Crypto/SelfTest/Util/test_Counter.py
index 33c9bd7..7dbfc97 100644
--- a/lib/Crypto/SelfTest/Util/test_Counter.py
+++ b/lib/Crypto/SelfTest/Util/test_Counter.py
@@ -39,34 +39,24 @@ class CounterTests(unittest.TestCase):
         from Crypto.Util import Counter
 
     def test_BE_shortcut(self):
-        """Big endian, shortcut enabled"""
+        """Big endian"""
         c = Counter.new(128)
-        self.assertEqual(c.__PCT_CTR_SHORTCUT__,True) # assert_
         c = Counter.new(128, little_endian=False)
-        self.assertEqual(c.__PCT_CTR_SHORTCUT__,True) # assert_
-        c = Counter.new(128, disable_shortcut=False)
-        self.assertEqual(c.__PCT_CTR_SHORTCUT__,True) # assert_
-        c = Counter.new(128, little_endian=False, disable_shortcut=False)
-        self.assertEqual(c.__PCT_CTR_SHORTCUT__,True) # assert_
 
     def test_LE_shortcut(self):
-        """Little endian, shortcut enabled"""
+        """Little endian"""
         c = Counter.new(128, little_endian=True)
-        self.assertEqual(c.__PCT_CTR_SHORTCUT__,True) # assert_
-        c = Counter.new(128, little_endian=True, disable_shortcut=False)
-        self.assertEqual(c.__PCT_CTR_SHORTCUT__,True) # assert_
 
     def test_BE_no_shortcut(self):
-        """Big endian, shortcut disabled"""
+        """Big endian, with disable_shortcut"""
+        # Just testing API backward-compatibility.  disable_shortcut is now a no-op.
         c = Counter.new(128, disable_shortcut=True)
-        self.assertRaises(AttributeError, getattr, c, '__PCT_CTR_SHORTCUT__')
         c = Counter.new(128, little_endian=False, disable_shortcut=True)
-        self.assertRaises(AttributeError, getattr, c, '__PCT_CTR_SHORTCUT__')
 
     def test_LE_no_shortcut(self):
         """Little endian, shortcut disabled"""
+        # Just testing API backward-compatibility.  disable_shortcut is now a no-op.
         c = Counter.new(128, little_endian=True, disable_shortcut=True)
-        self.assertRaises(AttributeError, getattr, c, '__PCT_CTR_SHORTCUT__')
 
     def test_BE_defaults(self):
         """128-bit, Big endian, defaults"""
diff --git a/lib/Crypto/Util/Counter.py b/lib/Crypto/Util/Counter.py
index 60e281c..2068fb3 100644
--- a/lib/Crypto/Util/Counter.py
+++ b/lib/Crypto/Util/Counter.py
@@ -57,11 +57,15 @@ if sys.version_info[0] == 2 and sys.version_info[1] == 1:
     from Crypto.Util.py21compat import *
 from Crypto.Util.py3compat import *
 
+from Crypto.pct_warnings import DisableShortcut_DeprecationWarning
 from Crypto.Util import _counter
 import struct
+import warnings
+
 
 # Factory function
-def new(nbits, prefix=b(""), suffix=b(""), initial_value=1, overflow=0, little_endian=False, allow_wraparound=False, disable_shortcut=False):
+_deprecated = "deprecated"
+def new(nbits, prefix=b(""), suffix=b(""), initial_value=1, overflow=0, little_endian=False, allow_wraparound=False, disable_shortcut=_deprecated):
     """Create a stateful counter block function suitable for CTR encryption modes.
 
     Each call to the function returns the next counter block.
@@ -89,10 +93,9 @@ def new(nbits, prefix=b(""), suffix=b(""), initial_value=1, overflow=0, little_e
         If True, the function will raise an *OverflowError* exception as soon
         as the counter wraps around. If False (default), the counter will
         simply restart from zero.
-      disable_shortcut : boolean
-        If True, do not make ciphers from `Crypto.Cipher` bypass the Python
-        layer when invoking the counter block function.
-        If False (default), bypass the Python layer.
+      disable_shortcut : deprecated
+        This option is a no-op for backward compatibility.  It will be removed
+        in a future version.  Don't use it.
     :Returns:
       The counter block function.
     """
@@ -109,10 +112,13 @@ def new(nbits, prefix=b(""), suffix=b(""), initial_value=1, overflow=0, little_e
 
     initval = _encode(initial_value, nbytes, little_endian)
 
+    if disable_shortcut is not _deprecated:  # exact object comparison
+        warnings.warn("disable_shortcut has no effect and is deprecated", DisableShortcut_DeprecationWarning)
+
     if little_endian:
-        return _counter._newLE(bstr(prefix), bstr(suffix), initval, allow_wraparound=allow_wraparound, disable_shortcut=disable_shortcut)
+        return _counter._newLE(bstr(prefix), bstr(suffix), initval, allow_wraparound=allow_wraparound)
     else:
-        return _counter._newBE(bstr(prefix), bstr(suffix), initval, allow_wraparound=allow_wraparound, disable_shortcut=disable_shortcut)
+        return _counter._newBE(bstr(prefix), bstr(suffix), initval, allow_wraparound=allow_wraparound)
 
 def _encode(n, nbytes, little_endian=False):
     retval = []
diff --git a/lib/Crypto/pct_warnings.py b/lib/Crypto/pct_warnings.py
index 9b4361e..d6adc5b 100644
--- a/lib/Crypto/pct_warnings.py
+++ b/lib/Crypto/pct_warnings.py
@@ -49,6 +49,9 @@ class ClockRewindWarning(CryptoRuntimeWarning):
 class GetRandomNumber_DeprecationWarning(CryptoDeprecationWarning):
     """Issued when Crypto.Util.number.getRandomNumber is invoked."""
 
+class DisableShortcut_DeprecationWarning(CryptoDeprecationWarning):
+    """Issued when Counter.new(disable_shortcut=...) is invoked."""
+
 class PowmInsecureWarning(CryptoRuntimeWarning):
     """Warning for when _fastmath is built without mpz_powm_sec"""