Manifest: use limited unpacker

This commit is contained in:
Thomas Waldmann 2017-06-23 05:56:41 +02:00
parent c83c4d633a
commit 6c2c51939d
5 changed files with 25 additions and 5 deletions

View File

@ -31,6 +31,11 @@ DEFAULT_MAX_SEGMENT_SIZE = 500 * 1024 * 1024
# the header, and the total size was set to 20 MiB).
MAX_DATA_SIZE = 20971479
# to use a safe, limited unpacker, we need to set a upper limit to the archive count in the manifest.
# this does not mean that you can always really reach that number, because it also needs to be less than
# MAX_DATA_SIZE or it will trigger the check for that.
MAX_ARCHIVES = 400000
DEFAULT_SEGMENTS_PER_DIR = 1000
CHUNK_MIN_EXP = 19 # 2**19 == 512kiB

View File

@ -24,6 +24,7 @@ from ..helpers import get_keys_dir, get_security_dir
from ..helpers import bin_to_hex
from ..item import Key, EncryptedKey
from ..platform import SaveFile
from .. import remote
from .nonces import NonceManager
from .low_level import AES, bytes_to_long, bytes_to_int, num_aes_blocks, hmac_sha256, blake2b_256, hkdf_hmac_sha512
@ -216,9 +217,9 @@ class KeyBase:
logger.warning('Manifest authentication DISABLED.')
tam_required = False
data = bytearray(data)
# Since we don't trust these bytes we use the slower Python unpacker,
# which is assumed to have a lower probability of security issues.
unpacked = msgpack.fallback.unpackb(data, object_hook=StableDict, unicode_errors='surrogateescape')
unpacker = remote.get_limited_unpacker('manifest')
unpacker.feed(data)
unpacked = unpacker.unpack()
if b'tam' not in unpacked:
if tam_required:
raise TAMRequiredError(self.repository._location.canonical_path())

View File

@ -351,6 +351,10 @@ class Manifest:
prev_ts = self.last_timestamp
incremented = (prev_ts + timedelta(microseconds=1)).isoformat()
self.timestamp = max(incremented, datetime.utcnow().isoformat())
# include checks for limits as enforced by limited unpacker (used by load())
assert len(self.archives) <= MAX_ARCHIVES
assert all(len(name) <= 255 for name in self.archives)
assert len(self.item_keys) <= 100
manifest = ManifestItem(
version=1,
archives=StableDict(self.archives.get_raw_dict()),

View File

@ -20,6 +20,7 @@ import msgpack
from . import __version__
from .compress import LZ4
from .constants import * # NOQA
from .helpers import Error, IntegrityError
from .helpers import bin_to_hex
from .helpers import get_home_dir
@ -28,6 +29,7 @@ from .helpers import replace_placeholders
from .helpers import sysinfo
from .helpers import format_file_size
from .helpers import truncate_and_unlink
from .helpers import StableDict
from .logger import create_logger, setup_logging
from .repository import Repository, MAX_OBJECT_SIZE, LIST_SCAN_LIMIT
from .version import parse_version, format_version
@ -81,8 +83,16 @@ def get_limited_unpacker(kind):
args.update(dict(max_array_len=LIST_SCAN_LIMIT, # result list from repo.list() / .scan()
max_map_len=100, # misc. result dicts
))
elif kind == 'manifest':
args.update(dict(use_list=True, # default value
max_array_len=100, # ITEM_KEYS ~= 22
max_map_len=MAX_ARCHIVES, # list of archives
max_str_len=255, # archive name
object_hook=StableDict,
unicode_errors='surrogateescape',
))
else:
raise ValueError('kind must be "server" or "client"')
raise ValueError('kind must be "server", "client" or "manifest"')
return msgpack.Unpacker(**args)

View File

@ -331,7 +331,7 @@ class TestTAM:
key.unpack_and_verify_manifest(blob)
blob = b'\xc1\xc1\xc1'
with pytest.raises(msgpack.UnpackException):
with pytest.raises((ValueError, msgpack.UnpackException)):
key.unpack_and_verify_manifest(blob)
def test_missing_when_required(self, key):