From 336d0426dbd83dd8b1be6e0757a45307a0d4e526 Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 2 Jul 2017 14:59:56 +0200 Subject: [PATCH 1/2] crypto low_level: don't mutate local bytes() CPython kind-of permits this (even by the docs), other implementations don't. Allocate the result on the stack, then copy to fresh bytes. --- src/borg/crypto/low_level.pyx | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/borg/crypto/low_level.pyx b/src/borg/crypto/low_level.pyx index d98228ef7..4351a4d27 100644 --- a/src/borg/crypto/low_level.pyx +++ b/src/borg/crypto/low_level.pyx @@ -6,6 +6,7 @@ from math import ceil from libc.stdlib cimport malloc, free from cpython.buffer cimport PyBUF_SIMPLE, PyObject_GetBuffer, PyBuffer_Release +from cpython.bytes cimport PyBytes_FromStringAndSize API_VERSION = '1.1_01' @@ -201,19 +202,18 @@ cdef class AES: def hmac_sha256(key, data): - md = bytes(32) cdef Py_buffer data_buf = ro_buffer(data) cdef const unsigned char *key_ptr = key cdef int key_len = len(key) - cdef unsigned char *md_ptr = md + cdef unsigned char md[32] try: with nogil: - rc = HMAC(EVP_sha256(), key_ptr, key_len, data_buf.buf, data_buf.len, md_ptr, NULL) - if rc != md_ptr: + rc = HMAC(EVP_sha256(), key_ptr, key_len, data_buf.buf, data_buf.len, md, NULL) + if rc != md: raise Exception('HMAC(EVP_sha256) failed') finally: PyBuffer_Release(&data_buf) - return md + return PyBytes_FromStringAndSize( &md[0], 32) cdef blake2b_update_from_buffer(blake2b_state *state, obj): @@ -232,8 +232,7 @@ def blake2b_256(key, data): if blake2b_init(&state, 32) == -1: raise Exception('blake2b_init() failed') - md = bytes(32) - cdef unsigned char *md_ptr = md + cdef unsigned char md[32] cdef unsigned char *key_ptr = key # This is secure, because BLAKE2 is not vulnerable to length-extension attacks (unlike SHA-1/2, MD-5 and others). @@ -246,11 +245,11 @@ def blake2b_256(key, data): raise Exception('blake2b_update() failed') blake2b_update_from_buffer(&state, data) - rc = blake2b_final(&state, md_ptr, 32) + rc = blake2b_final(&state, &md[0], 32) if rc == -1: raise Exception('blake2b_final() failed') - return md + return PyBytes_FromStringAndSize( &md[0], 32) def hkdf_hmac_sha512(ikm, salt, info, output_length): From 9827578df5057598d0ffe1735524ff80796ead7f Mon Sep 17 00:00:00 2001 From: Marian Beermann Date: Sun, 2 Jul 2017 15:07:09 +0200 Subject: [PATCH 2/2] hashindex: don't pass side effect into macro Py_XDECREF and friends are explicitly written to use op only once in CPython (and other code relies on this, Py_XDECREF(something()) is fairly common), but other implementations don't guarantee this. So, let's make a rule: don't pass side effects into macros, full stop. --- src/borg/_hashindex.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/borg/_hashindex.c b/src/borg/_hashindex.c index d49574bc8..e62e8875d 100644 --- a/src/borg/_hashindex.c +++ b/src/borg/_hashindex.c @@ -280,7 +280,7 @@ hashindex_read(PyObject *file_py, int permit_compact) { Py_ssize_t length, buckets_length, bytes_read; Py_buffer header_buffer; - PyObject *header_bytes, *length_object, *bucket_bytes; + PyObject *header_bytes, *length_object, *bucket_bytes, *tmp; HashHeader *header; HashIndex *index = NULL; @@ -307,7 +307,8 @@ hashindex_read(PyObject *file_py, int permit_compact) * Hash the header * If the header is corrupted this bails before doing something stupid (like allocating 3.8 TB of memory) */ - Py_XDECREF(PyObject_CallMethod(file_py, "hash_part", "s", "HashHeader")); + tmp = PyObject_CallMethod(file_py, "hash_part", "s", "HashHeader"); + Py_XDECREF(tmp); if(PyErr_Occurred()) { if(PyErr_ExceptionMatches(PyExc_AttributeError)) { /* Be able to work with regular file objects which do not have a hash_part method. */ @@ -329,7 +330,8 @@ hashindex_read(PyObject *file_py, int permit_compact) goto fail_decref_header; } - Py_XDECREF(PyObject_CallMethod(file_py, "seek", "ni", (Py_ssize_t)sizeof(HashHeader), SEEK_SET)); + tmp = PyObject_CallMethod(file_py, "seek", "ni", (Py_ssize_t)sizeof(HashHeader), SEEK_SET); + Py_XDECREF(tmp); if(PyErr_Occurred()) { goto fail_decref_header; } @@ -479,7 +481,7 @@ hashindex_free(HashIndex *index) static void hashindex_write(HashIndex *index, PyObject *file_py) { - PyObject *length_object, *buckets_view; + PyObject *length_object, *buckets_view, *tmp; Py_ssize_t length; Py_ssize_t buckets_length = (Py_ssize_t)index->num_buckets * index->bucket_size; HashHeader header = { @@ -507,7 +509,8 @@ hashindex_write(HashIndex *index, PyObject *file_py) /* * Hash the header */ - Py_XDECREF(PyObject_CallMethod(file_py, "hash_part", "s", "HashHeader")); + tmp = PyObject_CallMethod(file_py, "hash_part", "s", "HashHeader"); + Py_XDECREF(tmp); if(PyErr_Occurred()) { if(PyErr_ExceptionMatches(PyExc_AttributeError)) { /* Be able to work with regular file objects which do not have a hash_part method. */