Skip to content

Commit

Permalink
kdbx3: fix length of transform_rounds field (libkeepass#222)
Browse files Browse the repository at this point in the history
The official KeePass client produces and expects a 64 bit field, not a
32 bit field, for this parameter.

Using the incorrect length happens to parse just fine. The correct
length is encoded into the header so there is no parsing misalignment.
And because the field is little endian, parsing the field itself as a 32
bit field even though it is actually a 64 bit field also works, provided
that the field value fits into 32 bits, which it typically does.

However, if we write header field back out, we write it with a 32 bit
length, and KeePass rejects this. We weren't writing out the header
ourselves previously because kdbx.header.data wasn't being removed (see
libkeepass#219 (comment)).
However if we start doing that, such as to fix issue libkeepass#219, then this bug
is revealed.

The fix is trivial: we change the declaration to be a 64 bit field to
match the official client.
  • Loading branch information
basak authored and pschmitt committed Jan 27, 2021
1 parent 560d9d3 commit 12a30dd
Showing 1 changed file with 63 additions and 88 deletions.
151 changes: 63 additions & 88 deletions pykeepass/kdbx_parsing/kdbx3.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,51 +2,23 @@
# keepass decrypt experimentation

import hashlib

from construct import (
Byte,
Bytes,
Checksum,
Computed,
GreedyBytes,
IfThenElse,
Int16ul,
Int32ul,
Mapping,
Padding,
Pointer,
Prefixed,
RepeatUntil,
Struct,
Switch,
Tell,
len_,
this,
Byte, Bytes, Int16ul, Int32ul, Int64ul, RepeatUntil, GreedyBytes, Struct,
this, Mapping, Switch, Prefixed, Padding, Checksum, Computed, IfThenElse,
Pointer, Tell, len_
)

from .common import (
XML,
AES256Payload,
ChaCha20Payload,
CipherId,
CompressionFlags,
Concatenated,
Decompressed,
DynamicDict,
ProtectedStreamId,
Reparsed,
TwoFishPayload,
Unprotect,
aes_kdf,
compute_key_composite,
compute_master,
aes_kdf, AES256Payload, ChaCha20Payload, TwoFishPayload, Concatenated,
DynamicDict, compute_key_composite, Decompressed, Reparsed,
compute_master, CompressionFlags, XML, CipherId, ProtectedStreamId, Unprotect
)


# -------------------- Key Derivation --------------------

# https://github.com/keepassxreboot/keepassxc/blob/8324d03f0a015e62b6182843b4478226a5197090/src/format/KeePass2.cpp#L24-L26
kdf_uuids = {
"aes": b"\xc9\xd9\xf3\x9ab\x8aD`\xbft\r\x08\xc1\x8aO\xea",
'aes': b'\xc9\xd9\xf3\x9ab\x8aD`\xbft\r\x08\xc1\x8aO\xea',
}


Expand All @@ -57,12 +29,13 @@ def compute_transformed(context):
transformed_key = context._._.transformed_key
else:
key_composite = compute_key_composite(
password=context._._.password, keyfile=context._._.keyfile
password=context._._.password,
keyfile=context._._.keyfile
)
transformed_key = aes_kdf(
context._.header.value.dynamic_header.transform_seed.data,
context._.header.value.dynamic_header.transform_rounds.data,
key_composite,
key_composite
)

return transformed_key
Expand All @@ -72,73 +45,79 @@ def compute_transformed(context):

# https://github.com/dlech/KeePass2.x/blob/dbb9d60095ef39e6abc95d708fb7d03ce5ae865e/KeePassLib/Serialization/KdbxFile.cs#L234-L246
DynamicHeaderItem = Struct(
"id"
/ Mapping(
"id" / Mapping(
Byte,
{
"end": 0,
"comment": 1,
"cipher_id": 2,
"compression_flags": 3,
"master_seed": 4,
"transform_seed": 5,
"transform_rounds": 6,
"encryption_iv": 7,
"protected_stream_key": 8,
"stream_start_bytes": 9,
"protected_stream_id": 10,
},
{'end': 0,
'comment': 1,
'cipher_id': 2,
'compression_flags': 3,
'master_seed': 4,
'transform_seed': 5,
'transform_rounds': 6,
'encryption_iv': 7,
'protected_stream_key': 8,
'stream_start_bytes': 9,
'protected_stream_id': 10,
}
),
"data"
/ Prefixed(
"data" / Prefixed(
Int16ul,
Switch(
this.id,
{
"compression_flags": CompressionFlags,
"cipher_id": CipherId,
"transform_rounds": Int32ul,
"protected_stream_id": ProtectedStreamId,
},
default=GreedyBytes,
),
{'compression_flags': CompressionFlags,
'cipher_id': CipherId,
'transform_rounds': Int64ul,
'protected_stream_id': ProtectedStreamId
},
default=GreedyBytes
)
),
)

DynamicHeader = DynamicDict(
"id", RepeatUntil(lambda item, a, b: item.id == "end", DynamicHeaderItem)
'id',
RepeatUntil(
lambda item, a, b: item.id == 'end',
DynamicHeaderItem
)
)

# -------------------- Payload Verification --------------------

# encrypted payload is split into multiple data blocks with hashes
PayloadBlock = Struct(
"block_index" / Checksum(Int32ul, lambda this: this._index, this),
"block_index" / Checksum(
Int32ul,
lambda this: this._index,
this
),
"block_hash_offset" / Tell,
Padding(32),
"block_data" / Prefixed(Int32ul, GreedyBytes),
# block_hash has to be at the end with a pointer because it needs to
# come after other fields
"block_hash"
/ Pointer(
"block_hash" / Pointer(
this.block_hash_offset,
IfThenElse(
len_(this.block_data) == 0,
Checksum(Bytes(32), lambda _: b"\x00" * 32, this),
Checksum(
Bytes(32),
lambda _: b'\x00' * 32,
this
),
Checksum(
Bytes(32),
lambda block_data: hashlib.sha256(block_data).digest(),
this.block_data,
# exception=PayloadChecksumError
),
),
)
)
),
)

PayloadBlocks = RepeatUntil(
lambda item, a, b: len(item.block_data)
== 0, # and item.block_hash == b'\x00' * 32,
PayloadBlock,
lambda item, a, b: len(item.block_data) == 0, # and item.block_hash == b'\x00' * 32,
PayloadBlock
)


Expand All @@ -149,25 +128,23 @@ def compute_transformed(context):
UnpackedPayload = Reparsed(
Struct(
# validate payload decryption
"cred_check"
/ Checksum(
"cred_check" / Checksum(
Bytes(32),
lambda this: this._._.header.value.dynamic_header.stream_start_bytes.data,
this,
# exception=CredentialsError
),
"xml"
/ Unprotect(
"xml" / Unprotect(
this._._.header.value.dynamic_header.protected_stream_id.data,
this._._.header.value.dynamic_header.protected_stream_key.data,
XML(
IfThenElse(
this._._.header.value.dynamic_header.compression_flags.data.compression, # noqa: E501
this._._.header.value.dynamic_header.compression_flags.data.compression,
Decompressed(Concatenated(PayloadBlocks)),
Concatenated(PayloadBlocks),
Concatenated(PayloadBlocks)
)
),
),
)
)
)
)

Expand All @@ -177,15 +154,13 @@ def compute_transformed(context):
Body = Struct(
"transformed_key" / Computed(compute_transformed),
"master_key" / Computed(compute_master),
"payload"
/ UnpackedPayload(
"payload" / UnpackedPayload(
Switch(
this._.header.value.dynamic_header.cipher_id.data,
{
"aes256": AES256Payload(GreedyBytes),
"chacha20": ChaCha20Payload(GreedyBytes),
"twofish": TwoFishPayload(GreedyBytes),
},
{'aes256': AES256Payload(GreedyBytes),
'chacha20': ChaCha20Payload(GreedyBytes),
'twofish': TwoFishPayload(GreedyBytes),
}
)
),
)

0 comments on commit 12a30dd

Please sign in to comment.