Add tertiary authentication for metadata (TAM)

Signed-off-by: Thomas Waldmann <tw@waldmann-edv.de>

borg/archive.py

@@ -231,7 +231,7 @@ class Archive:
 
     def _load_meta(self, id):
         data = self.key.decrypt(id, self.repository.get(id))
-        metadata = msgpack.unpackb(data)
+        metadata = msgpack.unpackb(data, unicode_errors='surrogateescape')
         if metadata[b'version'] != 1:
             raise Exception('Unknown archive metadata version')
         return metadata
@@ -325,7 +325,7 @@ Number of files: {0.stats.nfiles}'''.format(
             'time': start.isoformat(),
             'time_end': end.isoformat(),
         })
-        data = msgpack.packb(metadata, unicode_errors='surrogateescape')
+        data = self.key.pack_and_authenticate_metadata(metadata, context=b'archive')
         self.id = self.key.id_hash(data)
         self.cache.add_chunk(self.id, data, self.stats)
         self.manifest.archives[name] = {'id': self.id, 'time': metadata['time']}

borg/archiver.py

@@ -30,7 +30,7 @@ from .compress import Compressor
 from .upgrader import AtticRepositoryUpgrader, BorgRepositoryUpgrader
 from .repository import Repository
 from .cache import Cache
-from .key import key_creator, RepoKey, PassphraseKey
+from .key import key_creator, tam_required_file, tam_required, RepoKey, PassphraseKey
 from .keymanager import KeyManager
 from .archive import backup_io, BackupOSError, Archive, ArchiveChecker, CHUNKER_PARAMS, is_special
 from .remote import RepositoryServer, RemoteRepository, cache_if_remote
@@ -51,7 +51,7 @@ def argument(args, str_or_bool):
     return str_or_bool
 
 
-def with_repository(fake=False, create=False, lock=True, exclusive=False, manifest=True, cache=False):
+def with_repository(fake=False, invert_fake=False, create=False, lock=True, exclusive=False, manifest=True, cache=False):
     """
     Method decorator for subcommand-handling methods: do_XYZ(self, args, repository, …)
 
@@ -68,7 +68,7 @@ def with_repository(fake=False, create=False, lock=True, exclusive=False, manife
         def wrapper(self, args, **kwargs):
             location = args.location  # note: 'location' must be always present in args
             append_only = getattr(args, 'append_only', False)
-            if argument(args, fake):
+            if argument(args, fake) ^ invert_fake:
                 return method(self, args, repository=None, **kwargs)
             elif location.proto == 'ssh':
                 repository = RemoteRepository(location, create=create, exclusive=argument(args, exclusive),
@@ -135,6 +135,8 @@ class Archiver:
         repository.commit()
         with Cache(repository, key, manifest, warn_if_unencrypted=False):
             pass
+        tam_file = tam_required_file(repository)
+        open(tam_file, 'w').close()
         return self.exit_code
 
     @with_repository(exclusive=True, manifest=False)
@@ -161,6 +163,7 @@ class Archiver:
     def do_change_passphrase(self, args, repository, manifest, key):
         """Change repository key file passphrase"""
         key.change_passphrase()
+        logger.info('Key updated')
         return EXIT_SUCCESS
 
     @with_repository(lock=False, exclusive=False, manifest=False, cache=False)
@@ -209,6 +212,7 @@ class Archiver:
         key_new.id_key = key_old.id_key
         key_new.chunk_seed = key_old.chunk_seed
         key_new.change_passphrase()  # option to change key protection passphrase, save
+        logger.info('Key updated')
         return EXIT_SUCCESS
 
     @with_repository(fake='dry_run', exclusive=True)
@@ -705,21 +709,43 @@ class Archiver:
                           DASHES)
         return self.exit_code
 
-    def do_upgrade(self, args):
+    @with_repository(fake='tam', invert_fake=True, manifest=False, exclusive=True)
+    def do_upgrade(self, args, repository, manifest=None, key=None):
         """upgrade a repository from a previous version"""
-        # mainly for upgrades from Attic repositories,
-        # but also supports borg 0.xx -> 1.0 upgrade.
+        if args.tam:
+            manifest, key = Manifest.load(repository, force_tam_not_required=args.force)
+
+            if not manifest.tam_verified:
+                # The standard archive listing doesn't include the archive ID like in borg 1.1.x
+                print('Manifest contents:')
+                for archive_info in manifest.list_archive_infos(sort_by='ts'):
+                    print(format_archive(archive_info), '[%s]' % bin_to_hex(archive_info.id))
+                manifest.write()
+                repository.commit()
+            if not key.tam_required:
+                key.tam_required = True
+                key.change_passphrase(key._passphrase)
+                print('Updated key')
+                if hasattr(key, 'find_key'):
+                    print('Key location:', key.find_key())
+            if not tam_required(repository):
+                tam_file = tam_required_file(repository)
+                open(tam_file, 'w').close()
+                print('Updated security database')
+        else:
+            # mainly for upgrades from Attic repositories,
+            # but also supports borg 0.xx -> 1.0 upgrade.
 
-        repo = AtticRepositoryUpgrader(args.location.path, create=False)
-        try:
-            repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress)
-        except NotImplementedError as e:
-            print("warning: %s" % e)
-        repo = BorgRepositoryUpgrader(args.location.path, create=False)
-        try:
-            repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress)
-        except NotImplementedError as e:
-            print("warning: %s" % e)
+            repo = AtticRepositoryUpgrader(args.location.path, create=False)
+            try:
+                repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress)
+            except NotImplementedError as e:
+                print("warning: %s" % e)
+            repo = BorgRepositoryUpgrader(args.location.path, create=False)
+            try:
+                repo.upgrade(args.dry_run, inplace=args.inplace, progress=args.progress)
+            except NotImplementedError as e:
+                print("warning: %s" % e)
         return self.exit_code
 
     def do_debug_info(self, args):
@@ -1613,6 +1639,28 @@ class Archiver:
 
         upgrade_epilog = textwrap.dedent("""
         Upgrade an existing Borg repository.
+
+        Borg 1.x.y upgrades
+        -------------------
+
+        Use ``borg upgrade --tam REPO`` to require manifest authentication
+        introduced with Borg 1.0.9 to address security issues. This means
+        that modifying the repository after doing this with a version prior
+        to 1.0.9 will raise a validation error, so only perform this upgrade
+        after updating all clients using the repository to 1.0.9 or newer.
+
+        This upgrade should be done on each client for safety reasons.
+
+        If a repository is accidentally modified with a pre-1.0.9 client after
+        this upgrade, use ``borg upgrade --tam --force REPO`` to remedy it.
+
+        See
+        https://borgbackup.readthedocs.io/en/stable/changes.html#pre-1-0-9-manifest-spoofing-vulnerability
+        for details.
+
+        Attic and Borg 0.xx to Borg 1.x
+        -------------------------------
+
         This currently supports converting an Attic repository to Borg and also
         helps with converting Borg 0.xx to 1.0.
 
@@ -1665,6 +1713,10 @@ class Archiver:
                                default=False, action='store_true',
                                help="""rewrite repository in place, with no chance of going back to older
                                versions of the repository.""")
+        subparser.add_argument('--force', dest='force', action='store_true',
+                               help="""Force upgrade""")
+        subparser.add_argument('--tam', dest='tam', action='store_true',
+                               help="""Enable manifest authentication (in key and cache) (Borg 1.0.9 and later)""")
         subparser.add_argument('location', metavar='REPOSITORY', nargs='?', default='',
                                type=location_validator(archive=False),
                                help='path to the repository to be upgraded')

borg/crypto.pyx

@@ -2,9 +2,13 @@
 
 This could be replaced by PyCrypto maybe?
 """
+import hashlib
+import hmac
+from math import ceil
+
 from libc.stdlib cimport malloc, free
 
-API_VERSION = 2
+API_VERSION = 3
 
 cdef extern from "openssl/rand.h":
     int  RAND_bytes(unsigned char *buf, int num)
@@ -171,3 +175,30 @@ cdef class AES:
             return out[:ptl]
         finally:
             free(out)
+
+
+def hkdf_hmac_sha512(ikm, salt, info, output_length):
+    """
+    Compute HKDF-HMAC-SHA512 with input key material *ikm*, *salt* and *info* to produce *output_length* bytes.
+
+    This is the "HMAC-based Extract-and-Expand Key Derivation Function (HKDF)" (RFC 5869)
+    instantiated with HMAC-SHA512.
+
+    *output_length* must not be greater than 64 * 255 bytes.
+    """
+    digest_length = 64
+    assert output_length <= (255 * digest_length), 'output_length must be <= 255 * 64 bytes'
+    # Step 1. HKDF-Extract (ikm, salt) -> prk
+    if salt is None:
+        salt = bytes(64)
+    prk = hmac.HMAC(salt, ikm, hashlib.sha512).digest()
+
+    # Step 2. HKDF-Expand (prk, info, output_length) -> output key
+    n = ceil(output_length / digest_length)
+    t_n = b''
+    output = b''
+    for i in range(n):
+        msg = t_n + info + (i + 1).to_bytes(1, 'little')
+        t_n = hmac.HMAC(prk, msg, hashlib.sha512).digest()
+        output += t_n
+    return output[:output_length]

borg/helpers.py

@@ -86,7 +86,7 @@ def check_extension_modules():
         raise ExtensionModuleError
     if chunker.API_VERSION != 2:
         raise ExtensionModuleError
-    if crypto.API_VERSION != 2:
+    if crypto.API_VERSION != 3:
         raise ExtensionModuleError
     if platform.API_VERSION != 3:
         raise ExtensionModuleError
@@ -103,10 +103,11 @@ class Manifest:
         self.key = key
         self.repository = repository
         self.item_keys = frozenset(item_keys) if item_keys is not None else ITEM_KEYS
+        self.tam_verified = False
 
     @classmethod
-    def load(cls, repository, key=None):
-        from .key import key_factory
+    def load(cls, repository, key=None, force_tam_not_required=False):
+        from .key import key_factory, tam_required_file, tam_required
         from .repository import Repository
         from .archive import ITEM_KEYS
         try:
@@ -117,8 +118,8 @@ class Manifest:
             key = key_factory(repository, cdata)
         manifest = cls(key, repository)
         data = key.decrypt(None, cdata)
+        m, manifest.tam_verified = key.unpack_and_verify_manifest(data, force_tam_not_required=force_tam_not_required)
         manifest.id = key.id_hash(data)
-        m = msgpack.unpackb(data)
         if not m.get(b'version') == 1:
             raise ValueError('Invalid manifest version')
         manifest.archives = dict((k.decode('utf-8'), v) for k, v in m[b'archives'].items())
@@ -128,19 +129,27 @@ class Manifest:
         manifest.config = m[b'config']
         # valid item keys are whatever is known in the repo or every key we know
         manifest.item_keys = frozenset(m.get(b'item_keys', [])) | ITEM_KEYS
+        if manifest.config.get(b'tam_required', False) and manifest.tam_verified and not tam_required(repository):
+            logger.debug('Manifest is TAM verified and says TAM is required, updating security database...')
+            file = tam_required_file(repository)
+            open(file, 'w').close()
         return manifest, key
 
     def write(self):
+        if self.key.tam_required:
+            self.config[b'tam_required'] = True
         self.timestamp = datetime.utcnow().isoformat()
-        data = msgpack.packb(StableDict({
+        m = {
             'version': 1,
-            'archives': self.archives,
+            'archives': StableDict((name, StableDict(archive)) for name, archive in self.archives.items()),
             'timestamp': self.timestamp,
-            'config': self.config,
-            'item_keys': tuple(self.item_keys),
-        }))
+            'config': StableDict(self.config),
+            'item_keys': tuple(sorted(self.item_keys)),
+        }
+        self.tam_verified = True
+        data = self.key.pack_and_authenticate_metadata(m)
         self.id = self.key.id_hash(data)
-        self.repository.put(self.MANIFEST_ID, self.key.encrypt(data))
+        self.repository.put(self.MANIFEST_ID, self.key.encrypt(data, none_compression=True))
 
     def list_archive_infos(self, sort_by=None, reverse=False):
         # inexpensive Archive.list_archives replacement if we just need .name, .id, .ts
@@ -249,6 +258,18 @@ def get_keys_dir():
     return keys_dir
 
 
+def get_security_dir(repository_id=None):
+    """Determine where to store local security information."""
+    xdg_config = os.environ.get('XDG_CONFIG_HOME', os.path.join(os.path.expanduser('~'), '.config'))
+    security_dir = os.environ.get('BORG_SECURITY_DIR', os.path.join(xdg_config, 'borg', 'security'))
+    if repository_id:
+        security_dir = os.path.join(security_dir, repository_id)
+    if not os.path.exists(security_dir):
+        os.makedirs(security_dir)
+        os.chmod(security_dir, stat.S_IRWXU)
+    return security_dir
+
+
 def get_cache_dir():
     """Determine where to repository keys and cache"""
     xdg_cache = os.environ.get('XDG_CACHE_HOME', os.path.join(os.path.expanduser('~'), '.cache'))

borg/key.py

@@ -5,15 +5,17 @@ import os
 import sys
 import textwrap
 from hmac import HMAC, compare_digest
-from hashlib import sha256, pbkdf2_hmac
+from hashlib import sha256, sha512, pbkdf2_hmac
 
-from .helpers import IntegrityError, get_keys_dir, Error, yes, bin_to_hex
+import msgpack
+
+from .helpers import StableDict, IntegrityError, get_keys_dir, get_security_dir, Error, yes, bin_to_hex
 from .logger import create_logger
 logger = create_logger()
 
 from .crypto import AES, bytes_to_long, long_to_bytes, bytes_to_int, num_aes_blocks
-from .compress import Compressor
-import msgpack
+from .crypto import hkdf_hmac_sha512
+from .compress import Compressor, CNONE
 
 PREFIX = b'\0' * 8
 
@@ -30,6 +32,10 @@ class UnsupportedPayloadError(Error):
     """Unsupported payload type {}. A newer version is required to access this repository."""
 
 
+class UnsupportedManifestError(Error):
+    """Unsupported manifest envelope. A newer version is required to access this repository."""
+
+
 class KeyfileNotFoundError(Error):
     """No key file for repository {} found in {}."""
 
@@ -38,6 +44,32 @@ class RepoKeyNotFoundError(Error):
     """No key entry found in the config of repository {}."""
 
 
+class TAMRequiredError(IntegrityError):
+    __doc__ = textwrap.dedent("""
+    Manifest is unauthenticated, but authentication is required for this repository.
+
+    This either means that you are under attack, or that you modified this repository
+    with a Borg version older than 1.0.9 after TAM authentication was enabled.
+
+    In the latter case, use "borg upgrade --tam --force '{}'" to re-authenticate the manifest.
+    """).strip()
+    traceback = False
+
+
+class TAMInvalid(IntegrityError):
+    __doc__ = IntegrityError.__doc__
+    traceback = False
+
+    def __init__(self):
+        # Error message becomes: "Data integrity error: Manifest authentication did not verify"
+        super().__init__('Manifest authentication did not verify')
+
+
+class TAMUnsupportedSuiteError(IntegrityError):
+    """Could not verify manifest: Unsupported suite {!r}; a newer version is needed."""
+    traceback = False
+
+
 def key_creator(repository, args):
     if args.encryption == 'keyfile':
         return KeyfileKey.create(repository, args)
@@ -63,6 +95,16 @@ def key_factory(repository, manifest_data):
         raise UnsupportedPayloadError(key_type)
 
 
+def tam_required_file(repository):
+    security_dir = get_security_dir(bin_to_hex(repository.id))
+    return os.path.join(security_dir, 'tam_required')
+
+
+def tam_required(repository):
+    file = tam_required_file(repository)
+    return os.path.isfile(file)
+
+
 class KeyBase:
     TYPE = None  # override in subclasses
 
@@ -71,23 +113,90 @@ class KeyBase:
         self.repository = repository
         self.target = None  # key location file path / repo obj
         self.compressor = Compressor('none')
+        self.tam_required = True
 
     def id_hash(self, data):
         """Return HMAC hash using the "id" HMAC key
         """
 
-    def encrypt(self, data):
+    def encrypt(self, data, none_compression=False):
         pass
 
     def decrypt(self, id, data):
         pass
 
+    def _tam_key(self, salt, context):
+        return hkdf_hmac_sha512(
+            ikm=self.id_key + self.enc_key + self.enc_hmac_key,
+            salt=salt,
+            info=b'borg-metadata-authentication-' + context,
+            output_length=64
+        )
+
+    def pack_and_authenticate_metadata(self, metadata_dict, context=b'manifest'):
+        metadata_dict = StableDict(metadata_dict)
+        tam = metadata_dict['tam'] = StableDict({
+            'type': 'HKDF_HMAC_SHA512',
+            'hmac': bytes(64),
+            'salt': os.urandom(64),
+        })
+        packed = msgpack.packb(metadata_dict, unicode_errors='surrogateescape')
+        tam_key = self._tam_key(tam['salt'], context)
+        tam['hmac'] = HMAC(tam_key, packed, sha512).digest()
+        return msgpack.packb(metadata_dict, unicode_errors='surrogateescape')
+
+    def unpack_and_verify_manifest(self, data, force_tam_not_required=False):
+        """Unpack msgpacked *data* and return (object, did_verify)."""
+        if data.startswith(b'\xc1' * 4):
+            # This is a manifest from the future, we can't read it.
+            raise UnsupportedManifestError()
+        tam_required = self.tam_required
+        if force_tam_not_required and tam_required:
+            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')
+        if b'tam' not in unpacked:
+            if tam_required:
+                raise TAMRequiredError(self.repository._location.canonical_path())
+            else:
+                logger.debug('TAM not found and not required')
+                return unpacked, False
+        tam = unpacked.pop(b'tam', None)
+        if not isinstance(tam, dict):
+            raise TAMInvalid()
+        tam_type = tam.get(b'type', b'<none>').decode('ascii', 'replace')
+        if tam_type != 'HKDF_HMAC_SHA512':
+            if tam_required:
+                raise TAMUnsupportedSuiteError(repr(tam_type))
+            else:
+                logger.debug('Ignoring TAM made with unsupported suite, since TAM is not required: %r', tam_type)
+                return unpacked, False
+        tam_hmac = tam.get(b'hmac')
+        tam_salt = tam.get(b'salt')
+        if not isinstance(tam_salt, bytes) or not isinstance(tam_hmac, bytes):
+            raise TAMInvalid()
+        offset = data.index(tam_hmac)
+        data[offset:offset + 64] = bytes(64)
+        tam_key = self._tam_key(tam_salt, context=b'manifest')
+        calculated_hmac = HMAC(tam_key, data, sha512).digest()
+        if not compare_digest(calculated_hmac, tam_hmac):
+            raise TAMInvalid()
+        logger.debug('TAM-verified manifest')
+        return unpacked, True
+
 
 class PlaintextKey(KeyBase):
     TYPE = 0x02
 
     chunk_seed = 0
 
+    def __init__(self, repository):
+        super().__init__(repository)
+        self.tam_required = False
+
     @classmethod
     def create(cls, repository, args):
         logger.info('Encryption NOT enabled.\nUse the "--encryption=repokey|keyfile" to enable encryption.')
@@ -100,8 +209,12 @@ class PlaintextKey(KeyBase):
     def id_hash(self, data):
         return sha256(data).digest()
 
-    def encrypt(self, data):
-        return b''.join([self.TYPE_STR, self.compressor.compress(data)])
+    def encrypt(self, data, none_compression=False):
+        if none_compression:
+            compressed = CNONE().compress(data)
+        else:
+            compressed = self.compressor.compress(data)
+        return b''.join([self.TYPE_STR, compressed])
 
     def decrypt(self, id, data):
         if data[0] != self.TYPE:
@@ -112,6 +225,9 @@ class PlaintextKey(KeyBase):
             raise IntegrityError('Chunk %s: id verification failed' % bin_to_hex(id))
         return data
 
+    def _tam_key(self, salt, context):
+        return salt + context
+
 
 class AESKeyBase(KeyBase):
     """Common base class shared by KeyfileKey and PassphraseKey
@@ -133,8 +249,11 @@ class AESKeyBase(KeyBase):
         """
         return HMAC(self.id_key, data, sha256).digest()
 
-    def encrypt(self, data):
-        data = self.compressor.compress(data)
+    def encrypt(self, data, none_compression=False):
+        if none_compression:
+            data = CNONE().compress(data)
+        else:
+            data = self.compressor.compress(data)
         self.enc_cipher.reset()
         data = b''.join((self.enc_cipher.iv[8:], self.enc_cipher.encrypt(data)))
         hmac = HMAC(self.enc_hmac_key, data, sha256).digest()
@@ -269,6 +388,7 @@ class PassphraseKey(AESKeyBase):
                 key.decrypt(None, manifest_data)
                 num_blocks = num_aes_blocks(len(manifest_data) - 41)
                 key.init_ciphers(PREFIX + long_to_bytes(key.extract_nonce(manifest_data) + num_blocks))
+                key._passphrase = passphrase
                 return key
             except IntegrityError:
                 passphrase = Passphrase.getpass(prompt)
@@ -284,6 +404,7 @@ class PassphraseKey(AESKeyBase):
     def init(self, repository, passphrase):
         self.init_from_random_data(passphrase.kdf(repository.id, self.iterations, 100))
         self.init_ciphers()
+        self.tam_required = False
 
 
 class KeyfileKeyBase(AESKeyBase):
@@ -307,6 +428,7 @@ class KeyfileKeyBase(AESKeyBase):
                 raise PassphraseWrong
         num_blocks = num_aes_blocks(len(manifest_data) - 41)
         key.init_ciphers(PREFIX + long_to_bytes(key.extract_nonce(manifest_data) + num_blocks))
+        key._passphrase = passphrase
         return key
 
     def find_key(self):
@@ -327,6 +449,7 @@ class KeyfileKeyBase(AESKeyBase):
             self.enc_hmac_key = key[b'enc_hmac_key']
             self.id_key = key[b'id_key']
             self.chunk_seed = key[b'chunk_seed']
+            self.tam_required = key.get(b'tam_required', tam_required(self.repository))
             return True
         return False
 
@@ -363,15 +486,16 @@ class KeyfileKeyBase(AESKeyBase):
             'enc_hmac_key': self.enc_hmac_key,
             'id_key': self.id_key,
             'chunk_seed': self.chunk_seed,
+            'tam_required': self.tam_required,
         }
         data = self.encrypt_key_file(msgpack.packb(key), passphrase)
         key_data = '\n'.join(textwrap.wrap(b2a_base64(data).decode('ascii')))
         return key_data
 
-    def change_passphrase(self):
-        passphrase = Passphrase.new(allow_empty=True)
+    def change_passphrase(self, passphrase=None):
+        if passphrase is None:
+            passphrase = Passphrase.new(allow_empty=True)
         self.save(self.target, passphrase)
-        logger.info('Key updated')
 
     @classmethod
     def create(cls, repository, args):

borg/testsuite/archiver.py

@@ -2,6 +2,8 @@ from binascii import unhexlify, b2a_base64
 from configparser import ConfigParser
 import errno
 import os
+from datetime import datetime
+from datetime import timedelta
 from io import StringIO
 import random
 import stat
@@ -14,6 +16,7 @@ import unittest
 from unittest.mock import patch
 from hashlib import sha256
 
+import msgpack
 import pytest
 
 from .. import xattr
@@ -21,13 +24,15 @@ from ..archive import Archive, ChunkBuffer, CHUNK_MAX_EXP, flags_noatime, flags_
 from ..archiver import Archiver
 from ..cache import Cache
 from ..crypto import bytes_to_long, num_aes_blocks
-from ..helpers import Manifest, PatternMatcher, parse_pattern, EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, bin_to_hex
-from ..key import RepoKey, KeyfileKey, Passphrase
+from ..helpers import Manifest, PatternMatcher, parse_pattern, EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR, bin_to_hex, \
+    get_security_dir
+from ..key import RepoKey, KeyfileKey, Passphrase, TAMRequiredError
 from ..keymanager import RepoIdMismatch, NotABorgKeyFile
 from ..remote import RemoteRepository, PathNotAllowed
 from ..repository import Repository
 from . import BaseTestCase, changedir, environment_variable, no_selinux
 from .platform import fakeroot_detected
+from . import key
 
 try:
     import llfuse
@@ -1143,8 +1148,8 @@ class ArchiverTestCase(ArchiverTestCaseBase):
 
         def verify_uniqueness():
             with Repository(self.repository_path) as repository:
-                for key, _ in repository.open_index(repository.get_transaction_id()).iteritems():
-                    data = repository.get(key)
+                for id, _ in repository.open_index(repository.get_transaction_id()).iteritems():
+                    data = repository.get(id)
                     hash = sha256(data).digest()
                     if hash not in seen:
                         seen.add(hash)
@@ -1253,7 +1258,7 @@ class ArchiverTestCase(ArchiverTestCaseBase):
             repo_key = RepoKey(repository)
             repo_key.load(None, Passphrase.env_passphrase())
 
-        backup_key = KeyfileKey(None)
+        backup_key = KeyfileKey(key.KeyTestCase.MockRepository())
         backup_key.load(export_file, Passphrase.env_passphrase())
 
         assert repo_key.enc_key == backup_key.enc_key
@@ -1501,6 +1506,63 @@ class ArchiverCheckTestCase(ArchiverTestCaseBase):
         self.cmd('check', self.repository_location, exit_code=0)
 
 
+class ManifestAuthenticationTest(ArchiverTestCaseBase):
+    def test_fresh_init_tam_required(self):
+        self.cmd('init', self.repository_location)
+        repository = Repository(self.repository_path, exclusive=True)
+        with repository:
+            manifest, key = Manifest.load(repository)
+            repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb({
+                'version': 1,
+                'archives': {},
+                'timestamp': (datetime.utcnow() + timedelta(days=1)).isoformat(),
+            })))
+            repository.commit()
+
+        with pytest.raises(TAMRequiredError):
+            self.cmd('list', self.repository_location)
+
+    def test_not_required(self):
+        self.cmd('init', self.repository_location)
+        self.create_src_archive('archive1234')
+        repository = Repository(self.repository_path, exclusive=True)
+        with repository:
+            shutil.rmtree(get_security_dir(bin_to_hex(repository.id)))
+            _, key = Manifest.load(repository)
+            key.tam_required = False
+            key.change_passphrase(key._passphrase)
+
+            manifest = msgpack.unpackb(key.decrypt(None, repository.get(Manifest.MANIFEST_ID)))
+            del manifest[b'tam']
+            repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb(manifest)))
+            repository.commit()
+        output = self.cmd('list', '--debug', self.repository_location)
+        assert 'archive1234' in output
+        assert 'TAM not found and not required' in output
+        # Run upgrade
+        self.cmd('upgrade', '--tam', self.repository_location)
+        # Manifest must be authenticated now
+        output = self.cmd('list', '--debug', self.repository_location)
+        assert 'archive1234' in output
+        assert 'TAM-verified manifest' in output
+        # Try to spoof / modify pre-1.0.9
+        with repository:
+            _, key = Manifest.load(repository)
+            repository.put(Manifest.MANIFEST_ID, key.encrypt(msgpack.packb({
+                'version': 1,
+                'archives': {},
+                'config': {},
+                'timestamp': (datetime.utcnow() + timedelta(days=1)).isoformat(),
+            })))
+            repository.commit()
+        # Fails
+        with pytest.raises(TAMRequiredError):
+            self.cmd('list', self.repository_location)
+        # Force upgrade
+        self.cmd('upgrade', '--tam', '--force', self.repository_location)
+        self.cmd('list', self.repository_location)
+
+
 @pytest.mark.skipif(sys.platform == 'cygwin', reason='remote is broken on cygwin and hangs')
 class RemoteArchiverTestCase(ArchiverTestCase):
     prefix = '__testsuite__:'

borg/testsuite/crypto.py

@@ -2,6 +2,7 @@ from binascii import hexlify
 
 from ..crypto import AES, bytes_to_long, bytes_to_int, long_to_bytes
 from ..crypto import increment_iv, bytes16_to_int, int_to_bytes16
+from ..crypto import hkdf_hmac_sha512
 from . import BaseTestCase
 
 
@@ -50,3 +51,55 @@ class CryptoTestCase(BaseTestCase):
         pdata = aes.decrypt(cdata)
         self.assert_equal(data, pdata)
         self.assert_equal(bytes_to_long(aes.iv, 8), 2)
+
+    # These test vectors come from https://www.kullo.net/blog/hkdf-sha-512-test-vectors/
+    # who claims to have verified these against independent Python and C++ implementations.
+
+    def test_hkdf_hmac_sha512(self):
+        ikm = b'\x0b' * 22
+        salt = bytes.fromhex('000102030405060708090a0b0c')
+        info = bytes.fromhex('f0f1f2f3f4f5f6f7f8f9')
+        l = 42
+
+        okm = hkdf_hmac_sha512(ikm, salt, info, l)
+        assert okm == bytes.fromhex('832390086cda71fb47625bb5ceb168e4c8e26a1a16ed34d9fc7fe92c1481579338da362cb8d9f925d7cb')
+
+    def test_hkdf_hmac_sha512_2(self):
+        ikm = bytes.fromhex('000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f2021222324252627'
+                            '28292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a4b4c4d4e4f')
+        salt = bytes.fromhex('606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f808182838485868'
+                             '788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9fa0a1a2a3a4a5a6a7a8a9aaabacadaeaf')
+        info = bytes.fromhex('b0b1b2b3b4b5b6b7b8b9babbbcbdbebfc0c1c2c3c4c5c6c7c8c9cacbcccdcecfd0d1d2d3d4d5d6d7'
+                             'd8d9dadbdcdddedfe0e1e2e3e4e5e6e7e8e9eaebecedeeeff0f1f2f3f4f5f6f7f8f9fafbfcfdfeff')
+        l = 82
+
+        okm = hkdf_hmac_sha512(ikm, salt, info, l)
+        assert okm == bytes.fromhex('ce6c97192805b346e6161e821ed165673b84f400a2b514b2fe23d84cd189ddf1b695b48cbd1c838844'
+                                    '1137b3ce28f16aa64ba33ba466b24df6cfcb021ecff235f6a2056ce3af1de44d572097a8505d9e7a93')
+
+    def test_hkdf_hmac_sha512_3(self):
+        ikm = bytes.fromhex('0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b')
+        salt = None
+        info = b''
+        l = 42
+
+        okm = hkdf_hmac_sha512(ikm, salt, info, l)
+        assert okm == bytes.fromhex('f5fa02b18298a72a8c23898a8703472c6eb179dc204c03425c970e3b164bf90fff22d04836d0e2343bac')
+
+    def test_hkdf_hmac_sha512_4(self):
+        ikm = bytes.fromhex('0b0b0b0b0b0b0b0b0b0b0b')
+        salt = bytes.fromhex('000102030405060708090a0b0c')
+        info = bytes.fromhex('f0f1f2f3f4f5f6f7f8f9')
+        l = 42
+
+        okm = hkdf_hmac_sha512(ikm, salt, info, l)
+        assert okm == bytes.fromhex('7413e8997e020610fbf6823f2ce14bff01875db1ca55f68cfcf3954dc8aff53559bd5e3028b080f7c068')
+
+    def test_hkdf_hmac_sha512_5(self):
+        ikm = bytes.fromhex('0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c0c')
+        salt = None
+        info = b''
+        l = 42
+
+        okm = hkdf_hmac_sha512(ikm, salt, info, l)
+        assert okm == bytes.fromhex('1407d46013d98bc6decefcfee55f0f90b0c7f63d68eb1a80eaf07e953cfc0a3a5240a155d6e4daa965bb')

borg/testsuite/helpers.py

@@ -11,7 +11,7 @@ import msgpack.fallback
 import time
 
 from ..helpers import Location, format_file_size, format_timedelta, format_line, PlaceholderError, make_path_safe, \
-    prune_within, prune_split, get_cache_dir, get_keys_dir, Statistics, is_slow_msgpack, \
+    prune_within, prune_split, get_cache_dir, get_keys_dir, get_security_dir, Statistics, is_slow_msgpack, \
     yes, TRUISH, FALSISH, DEFAULTISH, \
     StableDict, int_to_bigint, bigint_to_int, parse_timestamp, CompressionSpec, ChunkerParams, \
     ProgressIndicatorPercent, ProgressIndicatorEndless, load_excludes, parse_pattern, \
@@ -654,6 +654,18 @@ def test_get_keys_dir(monkeypatch):
     assert get_keys_dir() == '/var/tmp'
 
 
+def test_get_security_dir(monkeypatch):
+    """test that get_security_dir respects environment"""
+    monkeypatch.delenv('BORG_SECURITY_DIR', raising=False)
+    monkeypatch.delenv('XDG_CONFIG_HOME', raising=False)
+    assert get_security_dir() == os.path.join(os.path.expanduser('~'), '.config', 'borg', 'security')
+    assert get_security_dir(repository_id='1234') == os.path.join(os.path.expanduser('~'), '.config', 'borg', 'security', '1234')
+    monkeypatch.setenv('XDG_CONFIG_HOME', '/var/tmp/.config')
+    assert get_security_dir() == os.path.join('/var/tmp/.config', 'borg', 'security')
+    monkeypatch.setenv('BORG_SECURITY_DIR', '/var/tmp')
+    assert get_security_dir() == '/var/tmp'
+
+
 @pytest.fixture()
 def stats():
     stats = Statistics()

borg/testsuite/key.py

@@ -4,9 +4,14 @@ import shutil
 import tempfile
 from binascii import hexlify, unhexlify
 
+import msgpack
+
+import pytest
+
 from ..crypto import bytes_to_long, num_aes_blocks
 from ..key import PlaintextKey, PassphraseKey, KeyfileKey
-from ..helpers import Location
+from ..key import UnsupportedManifestError, TAMRequiredError, TAMUnsupportedSuiteError, TAMInvalid
+from ..helpers import Location, StableDict
 from . import BaseTestCase
 
 
@@ -42,6 +47,9 @@ class KeyTestCase(BaseTestCase):
         class _Location:
             orig = '/some/place'
 
+            def canonical_path(self):
+                return self.orig
+
         _location = _Location()
         id = bytes(32)
 
@@ -101,3 +109,115 @@ class KeyTestCase(BaseTestCase):
         data = b'foo'
         self.assert_equal(hexlify(key.id_hash(data)), b'818217cf07d37efad3860766dcdf1d21e401650fed2d76ed1d797d3aae925990')
         self.assert_equal(data, key2.decrypt(key2.id_hash(data), key.encrypt(data)))
+
+
+class TestTAM:
+    @pytest.fixture
+    def key(self, monkeypatch):
+        monkeypatch.setenv('BORG_PASSPHRASE', 'test')
+        return KeyfileKey.create(KeyTestCase.MockRepository(), KeyTestCase.MockArgs())
+
+    def test_unpack_future(self, key):
+        blob = b'\xc1\xc1\xc1\xc1foobar'
+        with pytest.raises(UnsupportedManifestError):
+            key.unpack_and_verify_manifest(blob)
+
+        blob = b'\xc1\xc1\xc1'
+        with pytest.raises(msgpack.UnpackException):
+            key.unpack_and_verify_manifest(blob)
+
+    def test_missing_when_required(self, key):
+        blob = msgpack.packb({})
+        with pytest.raises(TAMRequiredError):
+            key.unpack_and_verify_manifest(blob)
+
+    def test_missing(self, key):
+        blob = msgpack.packb({})
+        key.tam_required = False
+        unpacked, verified = key.unpack_and_verify_manifest(blob)
+        assert unpacked == {}
+        assert not verified
+
+    def test_unknown_type_when_required(self, key):
+        blob = msgpack.packb({
+            'tam': {
+                'type': 'HMAC_VOLLBIT',
+            },
+        })
+        with pytest.raises(TAMUnsupportedSuiteError):
+            key.unpack_and_verify_manifest(blob)
+
+    def test_unknown_type(self, key):
+        blob = msgpack.packb({
+            'tam': {
+                'type': 'HMAC_VOLLBIT',
+            },
+        })
+        key.tam_required = False
+        unpacked, verified = key.unpack_and_verify_manifest(blob)
+        assert unpacked == {}
+        assert not verified
+
+    @pytest.mark.parametrize('tam, exc', (
+        ({}, TAMUnsupportedSuiteError),
+        ({'type': b'\xff'}, TAMUnsupportedSuiteError),
+        (None, TAMInvalid),
+        (1234, TAMInvalid),
+    ))
+    def test_invalid(self, key, tam, exc):
+        blob = msgpack.packb({
+            'tam': tam,
+        })
+        with pytest.raises(exc):
+            key.unpack_and_verify_manifest(blob)
+
+    @pytest.mark.parametrize('hmac, salt', (
+        ({}, bytes(64)),
+        (bytes(64), {}),
+        (None, bytes(64)),
+        (bytes(64), None),
+    ))
+    def test_wrong_types(self, key, hmac, salt):
+        data = {
+            'tam': {
+                'type': 'HKDF_HMAC_SHA512',
+                'hmac': hmac,
+                'salt': salt
+            },
+        }
+        tam = data['tam']
+        if hmac is None:
+            del tam['hmac']
+        if salt is None:
+            del tam['salt']
+        blob = msgpack.packb(data)
+        with pytest.raises(TAMInvalid):
+            key.unpack_and_verify_manifest(blob)
+
+    def test_round_trip(self, key):
+        data = {'foo': 'bar'}
+        blob = key.pack_and_authenticate_metadata(data)
+        assert blob.startswith(b'\x82')
+
+        unpacked = msgpack.unpackb(blob)
+        assert unpacked[b'tam'][b'type'] == b'HKDF_HMAC_SHA512'
+
+        unpacked, verified = key.unpack_and_verify_manifest(blob)
+        assert verified
+        assert unpacked[b'foo'] == b'bar'
+        assert b'tam' not in unpacked
+
+    @pytest.mark.parametrize('which', (b'hmac', b'salt'))
+    def test_tampered(self, key, which):
+        data = {'foo': 'bar'}
+        blob = key.pack_and_authenticate_metadata(data)
+        assert blob.startswith(b'\x82')
+
+        unpacked = msgpack.unpackb(blob, object_hook=StableDict)
+        assert len(unpacked[b'tam'][which]) == 64
+        unpacked[b'tam'][which] = unpacked[b'tam'][which][0:32] + bytes(32)
+        assert len(unpacked[b'tam'][which]) == 64
+        blob = msgpack.packb(unpacked)
+
+        with pytest.raises(TAMInvalid):
+            key.unpack_and_verify_manifest(blob)

docs/changes.rst

@@ -1,7 +1,62 @@
 Important notes
 ===============
 
-This section is used for infos about e.g. security and corruption issues.
+This section is used for infos about security and corruption issues.
+
+.. _tam_vuln:
+
+Pre-1.0.9 manifest spoofing vulnerability
+-----------------------------------------
+
+A flaw in the cryptographic authentication scheme in Borg allowed an attacker
+to spoof the manifest. The attack requires an attacker to be able to
+
+1. insert files (with no additional headers) into backups
+2. gain write access to the repository
+
+This vulnerability does not disclose plaintext to the attacker, nor does it
+affect the authenticity of existing archives.
+
+The vulnerability allows an attacker to create a spoofed manifest (the list of archives).
+Creating plausible fake archives may be feasible for small archives, but is unlikely
+for large archives.
+
+The fix adds a separate authentication tag to the manifest. For compatibility
+with prior versions this authentication tag is *not* required by default
+for existing repositories. Repositories created with 1.0.9 and later require it.
+
+Steps you should take:
+
+1. Upgrade all clients to 1.0.9 or later.
+2. Run ``borg upgrade --tam <repository>`` *on every client* for *each* repository.
+3. This will list all archives, including archive IDs, for easy comparison with your logs.
+4. Done.
+
+Prior versions can access and modify repositories with this measure enabled, however,
+to 1.0.9 or later their modifications are indiscernible from an attack and will
+raise an error until the below procedure is followed. We are aware that this can
+be be annoying in some circumstances, but don't see a way to fix the vulnerability
+otherwise.
+
+In case a version prior to 1.0.9 is used to modify a repository where above procedure
+was completed, and now you get an error message from other clients:
+
+1. ``borg upgrade --tam --force <repository>`` once with *any* client suffices.
+
+This attack is mitigated by:
+
+- Noting/logging ``borg list``, ``borg info``, or ``borg create --stats``, which
+  contain the archive IDs.
+
+We are not aware of others having discovered, disclosed or exploited this vulnerability.
+
+Vulnerability time line:
+
+* 2016-11-14: Vulnerability and fix discovered during review of cryptography by Marian Beermann (@enkore)
+* 2016-11-20: First patch
+* 2016-12-18: Released fixed versions: 1.0.9, 1.1.0b3
+
+.. _attic013_check_corruption:
 
 Pre-1.0.9 potential data loss
 -----------------------------
@@ -71,8 +126,14 @@ The best check that everything is ok is to run a dry-run extraction::
 Changelog
 =========
 
-Version 1.0.9 (not released yet)
---------------------------------
+Version 1.0.9 (2016-12-18)
+--------------------------
+
+Security fixes:
+
+- A flaw in the cryptographic authentication scheme in Borg allowed an attacker
+  to spoof the manifest. See :ref:`tam_vuln` above for the steps you should
+  take.
 
 Bug fixes:
 
@@ -96,7 +157,7 @@ Other changes:
   - markup fixes
 - tests:
 
-  - test_get_(cache|keys)_dir: clean env state, #1897
+  - test_get\_(cache|keys)_dir: clean env state, #1897
   - get back pytest's pretty assertion failures, #1938
 - setup.py build_usage:
 

docs/usage.rst

@@ -181,6 +181,9 @@ Some automatic "answerers" (if set, they automatically answer confirmation quest
 Directories:
     BORG_KEYS_DIR
         Default to '~/.config/borg/keys'. This directory contains keys for encrypted repositories.
+    BORG_SECURITY_DIR
+        Default to '~/.config/borg/security'. This directory is used by Borg to track various
+        pieces of security-related data.
     BORG_CACHE_DIR
         Default to '~/.cache/borg'. This directory contains the local cache and might need a lot
         of space for dealing with big repositories).