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.

diff --git a/Changes b/Changes
index a20e1c0cae72a17351e4ad1091a4cbd6fc8fb026..d56cd28d3930d15ed51babd86428292d5f953ae0 100644 (file)
--- 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 82796f323a56e473f31c7d1cc37270cfaf10cf51..d26779a03bf5493e6be3631c837cc25c53a748b3 100644 (file)
--- 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 22ddf57bf83537752f3beb82f4ceac9ec0603981..e5c8877d7b32b134685a291bc1d3fbbc73615176 100644 (file)
--- 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 8765e0211a9b4a8b105c57ef3e216cb337509d98..61a2f96a653836901cd70337ab5a1e810c78ef36 100644 (file)
--- 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 63eadcca0dbd03da9881221e2ef2b54c35c59ede..02ab6dc37c0a5ecda80673dbdfeb579184b60fc1 100644 (file)
--- 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 997d04c1a7d70d0bdd895fcb19e3940197369db0..8b26f6aac3d5ef9f6103934b8a6a9334c2ca50e7 100644 (file)
--- 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 {