From 91afba0f2c56e3091904e5cc8f33f1839d0d7ddd Mon Sep 17 00:00:00 2001 From: Charles McGarvey Date: Tue, 16 Aug 2022 19:36:52 -0600 Subject: [PATCH] Fix dumping wrong-sized encryption IV New databases were not being initialized correctly. The dumper now also warns when trying to dump wrong-sized encryption IVs. Fixes #5. --- Changes | 9 ++++++--- lib/File/KDBX.pm | 21 ++++++++++++++------- lib/File/KDBX/Dumper/V3.pm | 5 +++++ lib/File/KDBX/Dumper/V4.pm | 6 ++++++ lib/File/KDBX/Object.pm | 6 +++--- t/database.t | 9 +++++++++ 6 files changed, 43 insertions(+), 13 deletions(-) diff --git a/Changes b/Changes index a20e1c0..d56cd28 100644 --- a/Changes +++ b/Changes @@ -1,8 +1,11 @@ Revision history for File-KDBX. {{$NEXT}} - * Fixed transform_rounds method to work with Argon KDF. Thanks HIGHTOWE. - * Fixed bug preventing memory protection on new databases. Thanks HIGHTOWE. + * Fixed bug where dumping a fresh database could write wrong-sized encryption IV, making the resulting + serialization unreadable by some KeePass implementations. Thanks HIGHTOWE. + * Fixed bugs preventing the use of memory protection with fresh databases. Thanks HIGHTOWE. + * Fixed the transform_rounds method to work with Argon KDF; this now maps to the Argon iterations value if + the current KDF is Argon. Thanks HIGHTOWE. 0.905 2022-08-06 12:12:42-0600 * Declared Time::Local 1.19 as a required dependency. @@ -22,7 +25,7 @@ Revision history for File-KDBX. * Added support for 32-bit perls. * API change: Rename iterator accessors on group to all_*. * Declared perl 5.10.0 prerequisite. I have no intention of supporting 5.8 or earlier. - * Fixed more other broken tests -- thanks CPAN testers. + * Fixed more other broken tests. Thanks CPAN testers. 0.901 2022-05-02 01:18:13-0600 diff --git a/lib/File/KDBX.pm b/lib/File/KDBX.pm index 82796f3..d26779a 100644 --- a/lib/File/KDBX.pm +++ b/lib/File/KDBX.pm @@ -40,9 +40,12 @@ sub new { # copy constructor return $_[0]->clone if @_ == 1 && blessed $_[0] && $_[0]->isa($class); - my $self = bless {}, $class; + my $data; + $data = shift if is_plain_hashref($_[0]); + + my $self = bless $data // {}, $class; $self->init(@_); - $self->_set_nonlazy_attributes if empty $self; + $self->_set_nonlazy_attributes if !$data; return $self; } @@ -238,10 +241,12 @@ has raw => coerce => \&to_string; # HEADERS has 'headers.comment' => '', coerce => \&to_string; -has 'headers.cipher_id' => CIPHER_UUID_CHACHA20, coerce => \&to_uuid; +has 'headers.cipher_id' => sub { $_[0]->version < KDBX_VERSION_4_0 ? CIPHER_UUID_AES256 : CIPHER_UUID_CHACHA20 }, + coerce => \&to_uuid; has 'headers.compression_flags' => COMPRESSION_GZIP, coerce => \&to_compression_constant; has 'headers.master_seed' => sub { random_bytes(32) }, coerce => \&to_string; -has 'headers.encryption_iv' => sub { random_bytes(16) }, coerce => \&to_string; +has 'headers.encryption_iv' => sub { random_bytes($_[0]->version < KDBX_VERSION_4_0 ? 16 : 12) }, + coerce => \&to_string; has 'headers.stream_start_bytes' => sub { random_bytes(32) }, coerce => \&to_string; has 'headers.kdf_parameters' => sub { +{ @@ -1421,7 +1426,9 @@ You normally do not need to call this method explicitly because the dumper does sub randomize_seeds { my $self = shift; - $self->encryption_iv(random_bytes(16)); + my $iv_size = 16; + $iv_size = $self->cipher(key => "\0" x 32)->iv_size if KDBX_VERSION_4_0 <= $self->version; + $self->encryption_iv(random_bytes($iv_size)); $self->inner_random_stream_key(random_bytes(64)); $self->master_seed(random_bytes(32)); $self->stream_start_bytes(random_bytes(32)); @@ -1555,8 +1562,8 @@ sub cipher { my $self = shift; my %args = @_; - $args{uuid} //= $self->headers->{+HEADER_CIPHER_ID}; - $args{iv} //= $self->headers->{+HEADER_ENCRYPTION_IV}; + $args{uuid} //= $self->cipher_id; + $args{iv} //= $self->encryption_iv; require File::KDBX::Cipher; return File::KDBX::Cipher->new(%args); diff --git a/lib/File/KDBX/Dumper/V3.pm b/lib/File/KDBX/Dumper/V3.pm index 22ddf57..e5c8877 100644 --- a/lib/File/KDBX/Dumper/V3.pm +++ b/lib/File/KDBX/Dumper/V3.pm @@ -31,6 +31,11 @@ sub _write_headers { local $headers->{+HEADER_TRANSFORM_SEED} = $kdbx->transform_seed; local $headers->{+HEADER_TRANSFORM_ROUNDS} = $kdbx->transform_rounds; + my $got_iv_size = length($headers->{+HEADER_ENCRYPTION_IV}); + alert 'Encryption IV should be exactly 16 bytes long', + got => $got_iv_size, + expected => 16 if $got_iv_size != 16; + if (nonempty (my $comment = $headers->{+HEADER_COMMENT})) { $buf .= $self->_write_header($fh, HEADER_COMMENT, $comment); } diff --git a/lib/File/KDBX/Dumper/V4.pm b/lib/File/KDBX/Dumper/V4.pm index 8765e02..61a2f96 100644 --- a/lib/File/KDBX/Dumper/V4.pm +++ b/lib/File/KDBX/Dumper/V4.pm @@ -238,6 +238,12 @@ sub _write_body { my $cipher = $kdbx->cipher(key => $final_key); $fh = File::KDBX::IO::Crypt->new($fh, cipher => $cipher); + my $got_iv_size = length($kdbx->headers->{+HEADER_ENCRYPTION_IV}); + my $iv_size = $cipher->iv_size; + alert "Encryption IV should be $iv_size bytes long", + got => $got_iv_size, + expected => $iv_size if $got_iv_size != $iv_size; + my $compress = $kdbx->headers->{+HEADER_COMPRESSION_FLAGS}; if ($compress == COMPRESSION_GZIP) { load_optional('IO::Compress::Gzip'); diff --git a/lib/File/KDBX/Object.pm b/lib/File/KDBX/Object.pm index 63eadcc..02ab6dc 100644 --- a/lib/File/KDBX/Object.pm +++ b/lib/File/KDBX/Object.pm @@ -664,9 +664,9 @@ sub _txns { $TXNS{$_[0]} //= [] } sub _commit { die 'Not implemented' } # Get a reference to an object that represents an object's committed state. If there is no pending -# transaction, this is just $self. If there is a transaction, this is the snapshot take before the transaction -# began. This method is private because it provides direct access to the actual snapshot. It is important that -# the snapshot not be changed or a rollback would roll back to an altered state. +# transaction, this is just $self. If there is a transaction, this is the snapshot taken immediately before +# the transaction began. This method is private because it provides direct access to the actual snapshot. It +# is important that the snapshot not be changed or a rollback would roll back to an altered state. # This is used by File::KDBX::Dumper::XML so as to not dump uncommitted changes. sub _committed { my $self = shift; diff --git a/t/database.t b/t/database.t index 997d04c..8b26f6a 100644 --- a/t/database.t +++ b/t/database.t @@ -8,6 +8,7 @@ use FindBin qw($Bin); use lib "$Bin/lib"; use TestCommon; +use File::KDBX::Constants qw(:cipher :version); use File::KDBX; use File::Temp qw(tempfile); use Test::Deep; @@ -29,6 +30,14 @@ subtest 'Create a new database' => sub { $entry->remove; ok $kdbx->_has_implicit_root, 'Removing group makes the root group implicit again'; + + cmp_ok $kdbx->version, '==', KDBX_VERSION_3_1, 'Default KDBX file version is 3.1'; + is $kdbx->cipher_id, CIPHER_UUID_AES256, 'Cipher of new database is AES256'; + cmp_ok length($kdbx->encryption_iv), '==', 16, 'Encryption IV of new databse is 16 bytes'; + + my $kdbx2 = File::KDBX->new(version => KDBX_VERSION_4_0); + is $kdbx2->cipher_id, CIPHER_UUID_CHACHA20, 'Cipher of new v4 database is ChaCha20'; + cmp_ok length($kdbx2->encryption_iv), '==', 12, 'Encryption IV of new databse is 12 bytes'; }; subtest 'Clone' => sub { -- 2.44.0