Importing with ctypes in Python: preventing overflows

Introduction

On some chilly winter evening, we’ve determined to refactor a number of examples and assessments for Python wrapper in Themis, as a result of issues should be not solely environment friendly and helpful, however elegant as properly. One factor after one other, and we ended up revamping Themis error codes a bit.

Inside error and standing flags generally get much less consideration than crypto-related code: they’re internals for inside use. Drawback is, once they fail, they could break one thing extra essential in a totally invisible method.

Since greatest errors are errors which aren’t simply mounted, however correctly analyzed, mirrored and recorded, we wrote this small report on a totally boring matter: each edge and connection is a problem. This story is a mirrored image on a typical subject: completely different folks engaged on completely different layers of 1 giant product, after which go searching to wipe out the technical debt.

Unusual assessments conduct

Anytime we contact Themis wrapper code, we contact the assessments as a result of pesticide paradox in software program improvement is not any small drawback.

It began with Safe Comparator assessments:

# take a look at.py

from pythemis.scomparator import scomparator, SCOMPARATOR_CODES

secret = b'some secret'

alice = scomparator(secret)
bob = scomparator(secret)
knowledge = alice.begin_compare()

whereas (alice.end result() == SCOMPARATOR_CODES.NOT_READY and
               bob.end result() == SCOMPARATOR_CODES.NOT_READY):
    knowledge = alice.proceed_compare(bob.proceed_compare(knowledge))

assert alice.end result() != SCOMPARATOR_CODES.NOT_MATCH
assert bob.end result() != SCOMPARATOR_CODES.NOT_MATCH

This take a look at makes an attempt to run Safe Comparator with a relentless secret, this fashion ensuring that comparability ends in a constructive end result (flag is named SCOMPARATOR_CODES.MATCH). If the key is matched, assessments ought to end with success.

Safe Comparator can lead to neither SCOMPARATOR_CODES.NOT_MATCH or SCOMPARATOR_CODES.MATCH.

However why the assert needs to be only a unfavourable comparability if we’re testing a characteristic with boolean state? Checking towards non-equality of NOT_MATCH doesn’t mechanically imply it matches.

The primary response is clearly to see if it even works (through instance code). It did.

Right here, the verification code assessments for equality, fortunately:

if comparator.is_equal():
    print("match")
else:
    print("not match")

Tremendous, so the issue touches solely assessments. Let’s rewrite assert in order that it compares scomparator.end result() towards SCOMPARATOR_CODES.MATCH right anticipated state:

# take a look at.py

...

assert alice.end result() == SCOMPARATOR_CODES.MATCH
assert bob.end result() == SCOMPARATOR_CODES.MATCH

… and stumble upon surprising error:

# python take a look at.py

Traceback (most up-to-date name final):
  File "take a look at.py", line 23, in 
    assert alice.end result() == SCOMPARATOR_CODES.MATCH
AssertionError

A routine repair of testing of completely working characteristic shortly turns into an attention-grabbing riddle. We’ve added variable output for debugging to see what’s actually occurring inside:

# take a look at.py

...

print('alice.end result(): {}nNOT_MATCH: {}nMATCH: {}'.format(
    alice.end result(),
    SCOMPARATOR_CODES.NOT_MATCH,
    SCOMPARATOR_CODES.MATCH
))
assert alice.end result() == SCOMPARATOR_CODES.MATCH
assert bob.end result() == SCOMPARATOR_CODES.MATCH

… and get the fully surprising:

# python take a look at.py

alice.end result(): -252645136
NOT_MATCH: -1
MATCH: 4042322160
Traceback (most up-to-date name final):
  File "take a look at.py", line 23, in 
    assert alice.end result() == SCOMPARATOR_CODES.MATCH
AssertionError

How come?

>>> import sys
>>> sys.int_info
sys.int_info(bits_per_digit=30, sizeof_digit=4)
...
>>> import ctypes
>>> print(ctypes.sizeof(ctypes.c_int))
4   

Regardless that OS, Python and Themis are 64bit, PyThemis wrapper is made utilizing ctypes, which has 32-bit int sort.

Accordingly, receiving 0xf0f0f0f0 from C Themis, ctypes expects a 32-bit quantity however 0xf0f0f0f0 is a unfavourable quantity. Then Python makes an attempt to transform it to an integer with none bit size restrict, and literal 0xf0f0f0f0 (from SCOMPARATOR_CODES) turns into 4042322160.

That is unusual. Let’s dive a bit into Themis:

src/soter/error.h:

// 

...
/** @transient return sort */
typedef int soter_status_t;

/**
 * @addtogroup SOTER
 * @{
 * @defgroup SOTER_ERROR_CODES standing codes
 * @{
 */

#outline SOTER_SUCCESS 0
#outline SOTER_FAIL   -1
#outline SOTER_INVALID_PARAMETER -2
#outline SOTER_NO_MEMORY -3
#outline SOTER_BUFFER_TOO_SMALL -4
#outline SOTER_DATA_CORRUPT -5
#outline SOTER_INVALID_SIGNATURE -6
#outline SOTER_NOT_SUPPORTED -7
#outline SOTER_ENGINE_FAIL -8

...

typedef int themis_status_t;

/**
 * @addtogroup THEMIS
 * @{
 * @defgroup SOTER_ERROR_CODES standing codes
 * @{
 */
#outline THEMIS_SSESSION_SEND_OUTPUT_TO_PEER 1
#outline THEMIS_SUCCESS SOTER_SUCCESS
#outline THEMIS_FAIL   SOTER_FAIL
#outline THEMIS_INVALID_PARAMETER SOTER_INVALID_PARAMETER
#outline THEMIS_NO_MEMORY SOTER_NO_MEMORY
#outline THEMIS_BUFFER_TOO_SMALL SOTER_BUFFER_TOO_SMALL
#outline THEMIS_DATA_CORRUPT SOTER_DATA_CORRUPT
#outline THEMIS_INVALID_SIGNATURE SOTER_INVALID_SIGNATURE
#outline THEMIS_NOT_SUPPORTED SOTER_NOT_SUPPORTED
#outline THEMIS_SSESSION_KA_NOT_FINISHED -8
#outline THEMIS_SSESSION_TRANSPORT_ERROR -9
#outline THEMIS_SSESSION_GET_PUB_FOR_ID_CALLBACK_ERROR -10

src/themis/secure_comparator.h:

...

#outline THEMIS_SCOMPARE_MATCH 0xf0f0f0f0
#outline THEMIS_SCOMPARE_NO_MATCH THEMIS_FAIL
#outline THEMIS_SCOMPARE_NOT_READY 0

...

themis_status_t secure_comparator_destroy(secure_comparator_t *comp_ctx);

themis_status_t secure_comparator_append_secret(secure_comparator_t *comp_ctx, const void *secret_data, size_t secret_data_length);

themis_status_t secure_comparator_begin_compare(secure_comparator_t *comp_ctx, void *compare_data, size_t *compare_data_length);
themis_status_t secure_comparator_proceed_compare(secure_comparator_t *comp_ctx, const void *peer_compare_data, size_t peer_compare_data_length, void *compare_data, size_t *compare_data_length);

themis_status_t secure_comparator_get_result(const secure_comparator_t *comp_ctx);

Now let’s see PyThemis facet at src/wrappers/themis/python/pythemis/exception.py.

All values right here correspond to C code, numbers are small and match any bit size limits:

from enum import IntEnum

class THEMIS_CODES(IntEnum):
    NETWORK_ERROR = -2222
    BUFFER_TOO_SMALL = -4
    FAIL = -1
    SUCCESS = 0
    SEND_AS_IS = 1

...

What about Safe Comparator half? Trying on the src/wrappers/themis/python/pythemis/scomparator.py, we see that general values are high quality, however Comparator’s worth for SCOMPARATOR_CODES.MATCH is problematic and turns into unfavourable in 32-bit int:

...

class SCOMPARATOR_CODES(IntEnum):
    MATCH = 0xf0f0f0f0
    NOT_MATCH = THEMIS_CODES.FAIL
    NOT_READY = 0

... 

If we forged it to signed 4 byte quantity, we obtain -252645136 the place we anticipate 4042322160.

So the issue is on the seams between C and Python, the place our code 0xf0f0f0f0 will get misinterpreted.

Attainable options

The entire drawback is a minor offense, straightforward to repair with a clutch, however the entire endeavor was to get rid of technical debt, not create extra of it.

• Possibility 1. Add sturdy sort casting when importing variables through ctypes

Very simple clutch. Since we all know how ctypes acts on this case, we will explicitly make code understand it as unsigned, then 0xf0f0f0f0 as int64_t might be equal to the interpretation of uint32_t. To try this, we might merely:

Add both

themis.secure_comparator_get_result.restype = ctypes.c_int64

or

themis.secure_comparator_get_result.restype = ctypes.c_uint

into src/wrappers/themis/python/pythemis/scomparator.py.

However that appears a bit like an unpleasant clutch, which moreover requires verifying the correctness of ctypes conduct on 32-bit machine with 32-bit Python.

• Possibility 2. Change from one byte illustration to a different

Hack quantity two. Take away implicit interpretation of hex literal 0xf0f0f0f0 and simply give it the proper worth, on this context -252645136. This can repair the issue in Python wrapper, however we nonetheless will want further verification on a 32bit system and control it in future.

Not an choice should you can keep away from it.

• Possibility 3. Refactor all statuses in C library, by no means use unfavourable numbers or values close to sort maximums to keep away from overflows.

The best could be the second choice: because it’s one such error in a single wrapper, why even hassle? Repair it immediately and overlook about it. However having issues even as soon as is usually sufficient to see a necessity for sure standardisation.

We took the third path, and re-thought the precept behind standing flags a bit:

• By no means use unfavourable numbers, as a result of -1 in 32bit is 0xffffffff, in 64bit is 0xffffffffffffffff and one can simply hit into overflow fairly quickly.
• Use small constructive numbers for error codes and statuses. Since Themis is meant to work throughout many architectures and (theoretically), there is likely to be a bizarre 9bit kitchen sink processor (they do want extra robots to hitch DoS armies, so have our phrase, it’s going to occur ultimately), we determined to restrict flag size with (0..127).
• In Themis half, which is immediately dealing with the wrappers, we’ve modified ints to express int32_t.

Since altering error code system in C library impacts all wrappers, and their error codes must be adjusted accordingly, we’ve determined to get error codes from C code immediately through variable export the place attainable (Go, NodeJS, Java, PHP).

After refactoring, error codes in Themis began to seem like:

src/soter/error.h:

...

/** @transient return sort */
typedef int soter_status_t;

/**
 * @addtogroup SOTER
 * @{
 * @defgroup SOTER_ERROR_CODES standing codes
 * @{
 */

#outline SOTER_SUCCESS 0//success code

//error codes
#outline SOTER_FAIL          11
#outline SOTER_INVALID_PARAMETER     12
#outline SOTER_NO_MEMORY         13
#outline SOTER_BUFFER_TOO_SMALL      14
#outline SOTER_DATA_CORRUPT      15
#outline SOTER_INVALID_SIGNATURE     16
#outline SOTER_NOT_SUPPORTED         17
#outline SOTER_ENGINE_FAIL       18

...

/** @transient return sort */
typedef int32_t themis_status_t;

/**
 * @addtogroup THEMIS
 * @{
 * @defgroup SOTER_ERROR_CODES standing codes
 * @{
 */

//
#outline THEMIS_SUCCESS              SOTER_SUCCESS
#outline THEMIS_SSESSION_SEND_OUTPUT_TO_PEER     1

//errors
#outline THEMIS_FAIL                     SOTER_FAIL
#outline THEMIS_INVALID_PARAMETER            SOTER_INVALID_PARAMETER
#outline THEMIS_NO_MEMORY                SOTER_NO_MEMORY
#outline THEMIS_BUFFER_TOO_SMALL             SOTER_BUFFER_TOO_SMALL
#outline THEMIS_DATA_CORRUPT                 SOTER_DATA_CORRUPT
#outline THEMIS_INVALID_SIGNATURE            SOTER_INVALID_SIGNATURE
#outline THEMIS_NOT_SUPPORTED                SOTER_NOT_SUPPORTED
#outline THEMIS_SSESSION_KA_NOT_FINISHED         19
#outline THEMIS_SSESSION_TRANSPORT_ERROR         20
#outline THEMIS_SSESSION_GET_PUB_FOR_ID_CALLBACK_ERROR   21

#outline THEMIS_SCOMPARE_SEND_OUTPUT_TO_PEER         THEMIS_SSESSION_SEND_OUTPUT_TO_PEER

...

src/themis/secure_comparator.h:

...

#outline THEMIS_SCOMPARE_MATCH       21
#outline THEMIS_SCOMPARE_NO_MATCH    22
#outline THEMIS_SCOMPARE_NOT_READY   0

...

… and, accordingly, in PyThemis:

...

class THEMIS_CODES(IntEnum):
    NETWORK_ERROR = 2222
    BUFFER_TOO_SMALL = 14
    FAIL = 11
    SUCCESS = 0
    SEND_AS_IS = 1

...

Notice: NETWORK_ERROR is PyThemis particular and isn’t utilized in C half, so we saved it the way in which it was.

src/wrappers/themis/python/pythemis/scomparator.py:

...

class SCOMPARATOR_CODES(IntEnum):
    MATCH = 21
    NOT_MATCH = 22
    NOT_READY = 0

...

For instance, that is how direct importing of those flags in Go works:

gothemis/examine/examine.go:

bundle examine

/*
#cgo LDFLAGS: -lthemis -lsoter

...

const int GOTHEMIS_SCOMPARE_MATCH = THEMIS_SCOMPARE_MATCH;
const int GOTHEMIS_SCOMPARE_NO_MATCH = THEMIS_SCOMPARE_NO_MATCH;
const int GOTHEMIS_SCOMPARE_NOT_READY = THEMIS_SCOMPARE_NOT_READY;
*/
import "C"
import (
    "github.com/cossacklabs/themis/gothemis/errors"
    "runtime"
    "unsafe"
)

var (
    COMPARE_MATCH = int(C.GOTHEMIS_SCOMPARE_MATCH)
    COMPARE_NO_MATCH = int(C.GOTHEMIS_SCOMPARE_NO_MATCH)
    COMPARE_NOT_READY = int(C.GOTHEMIS_SCOMPARE_NOT_READY)
)

...

Outcomes

After fixing and refactoring, the brand new scomparator class seems to be like:

class SComparator(object):
# the identical
....

    def is_compared(self):
        return not (themis.secure_comparator_get_result(self.comparator_ctx) ==
                    SCOMPARATOR_CODES.NOT_READY)

    def is_equal(self):
        return (themis.secure_comparator_get_result(self.comparator_ctx) ==
                SCOMPARATOR_CODES.MATCH)

And the brand new take a look at code, lastly refactored to a good look:

import unittest

from pythemis import scomparator

class SComparatorTest(unittest.TestCase):
    def setUp(self):
        self.message = b"That is take a look at message"
        self.message1 = b"That is take a look at message2"

    def testComparation(self):
        alice = scomparator.SComparator(self.message)
        bob = scomparator.SComparator(self.message)
        knowledge = alice.begin_compare()
        whereas not (alice.is_compared() and bob.is_compared()):
            knowledge = alice.proceed_compare(bob.proceed_compare(knowledge))
        self.assertTrue(alice.is_equal())
        self.assertTrue(bob.is_equal())

    def testComparation2(self):
        alice = scomparator.SComparator(self.message)
        bob = scomparator.SComparator(self.message1)
        knowledge = alice.begin_compare()
        whereas not (alice.is_compared() and bob.is_compared()):
            knowledge = alice.proceed_compare(bob.proceed_compare(knowledge))
        self.assertFalse(alice.is_equal())
        self.assertFalse(bob.is_equal())    

# python scomparator_test.py 
..
----------------------------------------------------------------------
Ran 2 assessments in 0.064s

OK

Conclusions

We love taking the time exploring minor, boring, trivial issues. Other than prepared to present all people a greater Themis expertise, we use it every single day to construct completely different instruments and want to be extraordinarily assured that behind a pleasant API, which isolates all implementation particulars we’d accidently break, the implementations are right.

As with every bug, many of the conclusions sound like coming from the gods of copybook headings, as soon as you understand them:

• Use kinds of express sizes (int16_t, int32_t, int8_t) to be much less dependent of person architectures.
• Look ahead to sort overflows in signed varieties.
• Attempt to explicitly take a look at all attainable return standing flags in assessments.
• !false is true solely in boolean illustration. When you encode it in numbers, don’t depend on one-sided analysis. Should you’re evaluating ints, which characterize the 2 states,- there could be a million explanation why !false is definitely kittens, not true. Two mutually unique states don’t imply your system is not going to generate N-2 extra states due to some error.

Notice: This publish was written by the folks at Cossack Labs. The unique publish is out there right here.