Hi,

Its been a while since I did anything serious with Bareos in terms of
development (I wrote the client quota support for bacula a long time
ago) but had a idea I wanted to make work recently and spent a week
doing the first draft of it.

Attached, I've patched in in-format support for encrypted volumes. I
consider it a first draft to prove it can be done and I reckon is
pretty shakey at the moment. 

I do intend on being backwards compatible with older volumes of course.

Currently I dont think its really elegantly written, offering good
debugging output or providing suitable diagnostic information.

It works in the volume formats block layer rather than as a device
backend - providing support for all future storage backends. 
All data after the block header (except block 0) is encrypted.

I wanted this feature for a few reasons.
 - dm-crypt works but only does at-rest encryption.
 - Some tape systems dont do encryption. Would be nice to be able to
supply it.
 - I dont need the client side management overhead using record
encryption yet it would still be definitely nice to protect the volumes
data some way.
 - Its not so simple to make a mount command with dm-crypt devices
without raising bareos privileges in some way.

My personal inclination is actually to be shipping volumes off to
MicroSD cards (cheap, portable and removable) which is what I want to
be using it for.

I was originally going to do this using a device backend, but it turns
out thats really quite tricky where theres no good guarantees on how
much data will be requested on a per-block level. Thats needed for
checksumming. Given the block structure provides it, it works best
there.

The first issue I can see is my implementation is its quite invasive.

1. I bumped the bacula volume label version number up to 21. 
The volume label now carries a 'Salt' parameter (an 8 byte random
nonce) which is used in a password key derivation routine.
The volume label also carries a 'Cipher' parameter, a unsigned short
which denotes the crypto cipher used to encrypt the volume. I've only
implemented AESGCM-256 and NONE (the default for older volumes and non-
encrypted volumes).

2. I bumped the bacula block version up from v2 to v3. It now carries
the readable value 'BB03' instead of 'BB02', the block headers are
increased by 32 bytes in size and carries a cryptographic checksum of
the block it protects. This could be 16 bytes but I'm thinking ahead
for other crypto ciphers.

Secondly another obvious issue is I dont feel the code follows any
standards specific approach used with Bareos at the moment. Its using
OpenSSL without any abstraction in place. I suspect ideally the lib
folder should actually apply the heavy lifting in this case by moving
the OpenSSL/EVP stuff I use into that path.

As I said, I do have a proof of concept implementation that appears to
work but quite honestly needs a LOT more testing and checking to make
sure it deals with anomalous situations (such as end of media halfway
through a block write).

Before I pursued this project further I wanted to get some idea if
there is any value in this code ultimately being merged upstream and if
me changing the on-disk format gives people the wigglies :).

If the feature is desirable I'll redraft it to behave more in line with
bacula libraries doing the lifting and shifting of the crypto. Also add
more diagnostic output.

After patching, its enabled by adding the parameter:

  Volume Crypto Key = "My super secure secret password"

To the Device resource in bacula-sd.

If you write out a backup using this format it can be read out again
transparently decrypted using bextract and bls, providing you have the
device resource setup.

The patch is applied against the latest git clone of the repo.

Thoughts welcome.

-- 
You received this message because you are subscribed to the Google Groups 
"bareos-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to bareos-devel+unsubscr...@googlegroups.com.
To post to this group, send email to bareos-devel@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
diff --git a/core/src/lib/crypto.h b/core/src/lib/crypto.h
index d4e845c..1eb5aaf 100644
--- a/core/src/lib/crypto.h
+++ b/core/src/lib/crypto.h
@@ -164,5 +164,6 @@ const char *crypto_digest_name(crypto_digest_t type);
 const char *crypto_digest_name(DIGEST *digest);
 crypto_digest_t CryptoDigestStreamType(int stream);
 const char *crypto_strerror(crypto_error_t error);
+int CryptoRandomFill(unsigned char *buf, int size);
 
 #endif /* BAREOS_LIB_CRYPTO_H_ */
diff --git a/core/src/lib/crypto_openssl.cc b/core/src/lib/crypto_openssl.cc
index 04eb4b8..8ec06bb 100644
--- a/core/src/lib/crypto_openssl.cc
+++ b/core/src/lib/crypto_openssl.cc
@@ -433,6 +433,13 @@ X509_KEYPAIR *crypto_keypair_dup(X509_KEYPAIR *keypair)
    return newpair;
 }
 
+/* 
+ * Fill a buffer with random bytes
+ */
+int CryptoRandomFill(unsigned char *buf, int size)
+{
+  return RAND_bytes(buf, size);
+}
 
 /*
  * Load a public key from a PEM-encoded x509 certificate.
diff --git a/core/src/stored/CMakeLists.txt b/core/src/stored/CMakeLists.txt
index 2a3b5e6..aec46ae 100644
--- a/core/src/stored/CMakeLists.txt
+++ b/core/src/stored/CMakeLists.txt
@@ -78,7 +78,7 @@ set (LIBBAREOSSD_SRCS acquire.cc ansi_label.cc askdir.cc 
autochanger.cc
          butil.cc crc32.cc dev.cc device.cc ebcdic.cc label.cc lock.cc
          mount.cc read_record.cc record.cc reserve.cc scan.cc
          sd_backends.cc sd_plugins.cc sd_stats.cc spool.cc
-         stored_globals.cc stored_conf.cc vol_mgr.cc wait.cc
+         stored_globals.cc stored_conf.cc vol_mgr.cc vol_crypto.cc wait.cc
          ${AVAILABLE_DEVICE_API_SRCS}
     )
 
diff --git a/core/src/stored/block.cc b/core/src/stored/block.cc
index b34fc17..9a08592 100644
--- a/core/src/stored/block.cc
+++ b/core/src/stored/block.cc
@@ -64,6 +64,7 @@ void DumpBlock(DeviceBlock *b, const char *msg)
    int32_t  FileIndex;
    int32_t  Stream;
    int bhl, rhl;
+   char Sum[BLKHDR_SUM_LENGTH+1];
    char buf1[100], buf2[100];
 
    UnserBegin(b->buf, BLKHDR1_LENGTH);
@@ -73,7 +74,15 @@ void DumpBlock(DeviceBlock *b, const char *msg)
    UnserBytes(Id, BLKHDR_ID_LENGTH);
    ASSERT(UnserLength(b->buf) == BLKHDR1_LENGTH);
    Id[BLKHDR_ID_LENGTH] = 0;
-   if (Id[3] == '2') {
+   if (Id[3] == '3') {
+      unser_uint32(VolSessionId);
+      unser_uint32(VolSessionTime);
+      Sum[BLKHDR_SUM_LENGTH] = 0;
+      UnserBytes(Sum, BLKHDR_SUM_LENGTH);
+      bhl = BLKHDR3_LENGTH;
+      rhl = RECHDR2_LENGTH;
+   }
+   else if (Id[3] == '2') {
       unser_uint32(VolSessionId);
       unser_uint32(VolSessionTime);
       bhl = BLKHDR2_LENGTH;
@@ -188,6 +197,7 @@ void EmptyBlock(DeviceBlock *block)
    block->write_failed = false;
    block->block_read = false;
    block->FirstIndex = block->LastIndex = 0;
+   memset(block->Sum, 0, 32);
 }
 
 /**
@@ -202,16 +212,18 @@ static uint32_t SerBlockHeader(DeviceBlock *block, bool 
DoChecksum)
    uint32_t block_len = block->binbuf;
 
    Dmsg1(1390, "SerBlockHeader: block_len=%d\n", block_len);
-   SerBegin(block->buf, BLKHDR2_LENGTH);
+   SerBegin(block->buf, BLKHDR3_LENGTH);
    ser_uint32(CheckSum);
    ser_uint32(block_len);
    ser_uint32(block->BlockNumber);
    SerBytes(WRITE_BLKHDR_ID, BLKHDR_ID_LENGTH);
-   if (BLOCK_VER >= 2) {
+   if (BLOCK_VER == 2) {
       ser_uint32(block->VolSessionId);
       ser_uint32(block->VolSessionTime);
    }
-
+   if (BLOCK_VER == 3) {
+     SerBytes(block->Sum, BLKHDR_SUM_LENGTH);
+   }
    /*
     * Checksum whole block except for the checksum
     */
@@ -220,7 +232,7 @@ static uint32_t SerBlockHeader(DeviceBlock *block, bool 
DoChecksum)
                     block_len-BLKHDR_CS_LENGTH);
    }
    Dmsg1(1390, "ser_bloc_header: checksum=%x\n", CheckSum);
-   SerBegin(block->buf, BLKHDR2_LENGTH);
+   SerBegin(block->buf, BLKHDR3_LENGTH);
    ser_uint32(CheckSum);              /* now add checksum to block header */
    return CheckSum;
 }
@@ -280,6 +292,24 @@ static inline bool unSerBlockHeader(JobControlRecord *jcr, 
Device *dev, DeviceBl
          block->read_errors++;
          return false;
       }
+   } else if (Id[3] == '3') {
+      unser_uint32(block->VolSessionId);
+      unser_uint32(block->VolSessionTime);
+      UnserBytes(block->Sum, BLKHDR_SUM_LENGTH);
+      bhl = BLKHDR3_LENGTH;
+      block->BlockVer = 3;
+      block->bufp = block->buf + bhl;
+      if (!bstrncmp(Id, BLKHDR3_ID, BLKHDR_ID_LENGTH)) {
+         dev->dev_errno = EIO;
+         Mmsg4(dev->errmsg, _("Volume data error at %u:%u! Wanted ID: \"%s\", 
got \"%s\". Buffer discarded.\n"),
+            dev->file, dev->block_num, BLKHDR3_ID, Id);
+         if (block->read_errors == 0 || verbose >= 2) {
+            Jmsg(jcr, M_ERROR, 0, "%s", dev->errmsg);
+         }
+         block->read_errors++;
+         return false;
+      }
+
    } else {
       dev->dev_errno = EIO;
       Mmsg4(dev->errmsg, _("Volume data error at %u:%u! Wanted ID: \"%s\", got 
\"%s\". Buffer discarded.\n"),
@@ -322,7 +352,9 @@ static inline bool unSerBlockHeader(JobControlRecord *jcr, 
Device *dev, DeviceBl
    block->BlockNumber = BlockNumber;
    Dmsg3(390, "Read binbuf = %d %d block_len=%d\n", block->binbuf,
       bhl, block_len);
-   if (block_len <= block->read_len && dev->DoChecksum()) {
+   /* The crytpo checksum works to checksum the block and crc32 wont work after
+    * the raw block is enciphered */
+   if (block_len <= block->read_len && dev->DoChecksum() && dev->VolHdr.cipher 
== 0) {
       BlockCheckSum = bcrc32((uint8_t *)block->buf+BLKHDR_CS_LENGTH,
                          block_len-BLKHDR_CS_LENGTH);
       if (BlockCheckSum != CheckSum) {
@@ -572,6 +604,7 @@ bool DeviceControlRecord::WriteBlockToDev()
    int hit_max1, hit_max2;
    bool ok = true;
    DeviceControlRecord *dcr = this;
+   struct DeviceBlock *prev_block;
    uint32_t checksum;
 
    if (no_tape_write_test) {
@@ -724,6 +757,17 @@ bool DeviceControlRecord::WriteBlockToDev()
    int retry = 0;
    errno = 0;
    status = 0;
+
+   /* Cryto xfrm the block before write. If somethings goes wrong
+    * such as end-of-media we revert back to the OLD plaintext
+    * block and pass it back up to the handlers up the stack.
+    */
+   prev_block = VolCryptoBlockEncrypt(jcr, block);
+   if (!prev_block) {
+     Dmsg0(200, "VolCryptoBlockEncrypt() returned FALSE!\n");
+     return false;
+   }
+
    do {
       if (retry > 0 && status == -1 && errno == EBUSY) {
          BErrNo be;
@@ -752,6 +796,13 @@ bool DeviceControlRecord::WriteBlockToDev()
 #endif
 
    if (status != (ssize_t)wlen) {
+     /* When using crypto, return to the old, untransformed block before
+      * we pass back */
+      if (prev_block != block) {
+        FreeBlock(block);
+        block = prev_block;
+      }
+
       /*
        * Some devices simply report EIO when the volume is full.
        * With a little more thought we may be able to check
@@ -850,6 +901,11 @@ bool DeviceControlRecord::WriteBlockToDev()
    dev->file_addr += wlen;            /* update file address */
    dev->file_size += wlen;
 
+   /* Discard the old unencrypted block */
+   if (prev_block != block) {
+     FreeBlock(prev_block);
+   }
+
    Dmsg2(1300, "WriteBlock: wrote block %d bytes=%d\n", dev->block_num, wlen);
    EmptyBlock(block);
    return true;
@@ -1075,6 +1131,10 @@ reread:
       return false;
    }
 
+   if (!VolCryptoBlockDecrypt(jcr, block)) {
+     Jmsg0(jcr, M_ERROR, 0, "Volume decryption has failed.\n");
+     return false;
+   }
    /*
     * If the block is bigger than the buffer, we Reposition for
     *  re-reading the block, allocate a buffer of the correct size,
diff --git a/core/src/stored/block.h b/core/src/stored/block.h
index dabc22d..d5f3ebe 100644
--- a/core/src/stored/block.h
+++ b/core/src/stored/block.h
@@ -41,14 +41,17 @@ class Device;                           /* for forward 
reference */
 /* Block Header definitions. */
 #define BLKHDR1_ID                       "BB01"
 #define BLKHDR2_ID                       "BB02"
+#define BLKHDR3_ID                       "BB03"
 #define BLKHDR_ID_LENGTH                  4
 #define BLKHDR_CS_LENGTH                  4     /**< checksum length */
+#define BLKHDR_SUM_LENGTH                32     /**< cryptographic checksum 
len */
 #define BLKHDR1_LENGTH                   16     /**< Total length */
 #define BLKHDR2_LENGTH                   24     /**< Total length */
+#define BLKHDR3_LENGTH                   56     /**< Total length */
 
-#define WRITE_BLKHDR_ID     BLKHDR2_ID
-#define WRITE_BLKHDR_LENGTH BLKHDR2_LENGTH
-#define BLOCK_VER                        2
+#define WRITE_BLKHDR_ID     BLKHDR3_ID
+#define WRITE_BLKHDR_LENGTH BLKHDR3_LENGTH
+#define BLOCK_VER                        3
 
 /* Record header definitions */
 #define RECHDR1_LENGTH                   20
@@ -59,7 +62,8 @@ class Device;                           /* for forward 
reference */
 #define BareosId                         "Bareos 2.0 immortal\n"
 #define OldBaculaId                      "Bacula 1.0 immortal\n"
 #define OlderBaculaId                    "Bacula 0.9 mortal\n"
-#define BareosTapeVersion                20
+#define BareosTapeVersion                21
+#define OldBareosTapeVersion1            20
 #define OldCompatibleBareosTapeVersion1  11
 #define OldCompatibleBareosTapeVersion2  10
 #define OldCompatibleBareosTapeVersion3  9
@@ -102,6 +106,7 @@ struct DeviceBlock {
    uint32_t read_len;                 /* bytes read into buffer, if zero, 
block empty */
    uint32_t VolSessionId;             /* */
    uint32_t VolSessionTime;           /* */
+   uint8_t Sum[BLKHDR_SUM_LENGTH+1];     /* cryptographic checksum */
    uint32_t read_errors;              /* block errors (checksum, header, ...) 
*/
    int      BlockVer;                 /* block version 1 or 2 */
    bool     write_failed;             /* set if write failed */
diff --git a/core/src/stored/label.cc b/core/src/stored/label.cc
index e6315b1..791ceac 100644
--- a/core/src/stored/label.cc
+++ b/core/src/stored/label.cc
@@ -35,6 +35,7 @@
 #include "stored/device.h"
 #include "stored/label.h"
 #include "lib/edit.h"
+#include "lib/crypto.h"
 #include "include/jcr.h"
 
 namespace storagedaemon {
@@ -527,6 +528,8 @@ static void CreateVolumeLabelRecord(DeviceControlRecord 
*dcr, Device *dev, Devic
    SerString(dev->VolHdr.LabelProg);
    SerString(dev->VolHdr.ProgVersion);
    SerString(dev->VolHdr.ProgDate);
+   SerBytes(dev->VolHdr.Salt, sizeof(dev->VolHdr.Salt));
+   ser_uint16(dev->VolHdr.cipher);
 
    SerEnd(rec->data, SER_LENGTH_Volume_Label);
    bstrncpy(dcr->VolumeName, dev->VolHdr.VolumeName, sizeof(dcr->VolumeName));
@@ -577,6 +580,12 @@ void CreateVolumeLabel(Device *dev, const char *VolName, 
const char *PoolName)
    bstrncpy(dev->VolHdr.LabelProg, my_name, sizeof(dev->VolHdr.LabelProg));
    sprintf(dev->VolHdr.ProgVersion, "Ver. %s %s", VERSION, BDATE);
    sprintf(dev->VolHdr.ProgDate, "Build %s %s", __DATE__, __TIME__);
+   memset(dev->VolHdr.Salt, 0, sizeof(dev->VolHdr.Salt));
+   if (device->vol_crypto_key) {
+    /* We dont allow selection, just set something strong */
+     dev->VolHdr.cipher = V_CRYPTO_AESGCM256;
+     CryptoRandomFill((unsigned char *)dev->VolHdr.Salt, 8);
+   }
    dev->SetLabeled();               /* set has Bareos label */
    if (debug_level >= 90) {
       DumpVolumeLabel(dev);
@@ -736,6 +745,8 @@ bool UnserVolumeLabel(Device *dev, DeviceRecord *rec)
 
    dev->VolHdr.LabelType = rec->FileIndex;
    dev->VolHdr.LabelSize = rec->data_len;
+   memset(dev->VolHdr.Key, 0, sizeof(dev->VolHdr.Key));
+   memset(dev->VolHdr.Iv, 0, sizeof(dev->VolHdr.Iv));
 
 
    /* UnSerialize the record into the Volume Header */
@@ -765,6 +776,11 @@ bool UnserVolumeLabel(Device *dev, DeviceRecord *rec)
    UnserString(dev->VolHdr.ProgVersion);
    UnserString(dev->VolHdr.ProgDate);
 
+   if (dev->VolHdr.VerNum > 20) {
+     UnserBytes(dev->VolHdr.Salt, sizeof(dev->VolHdr.Salt));
+     unser_uint16(dev->VolHdr.cipher);
+   }
+
    SerEnd(rec->data, SER_LENGTH_Volume_Label);
    Dmsg0(190, "unser_vol_label\n");
    if (debug_level >= 190) {
@@ -824,7 +840,9 @@ void DumpVolumeLabel(Device *dev)
 {
    int dbl = debug_level;
    uint32_t File;
+   uint64_t salt;
    const char *LabelType;
+   const char *crypto;
    char buf[30];
    struct tm tm;
    struct date_time dt;
@@ -855,7 +873,11 @@ void DumpVolumeLabel(Device *dev)
       break;
    }
 
-   Pmsg11(-1, _("\nVolume Label:\n"
+  /* Bareos formatting doens't let me print this directly */
+   memcpy(&salt, dev->VolHdr.Salt, 8);
+   crypto = GetVolCryptoName(dev->VolHdr.cipher);
+
+   Pmsg13(-1, _("\nVolume Label:\n"
 "Id                : %s"
 "VerNo             : %d\n"
 "VolName           : %s\n"
@@ -867,12 +889,15 @@ void DumpVolumeLabel(Device *dev)
 "MediaType         : %s\n"
 "PoolType          : %s\n"
 "HostName          : %s\n"
+"Cipher            : %s\n"
+"Key Salt          : %llx\n"
 ""),
              dev->VolHdr.Id, dev->VolHdr.VerNum,
              dev->VolHdr.VolumeName, dev->VolHdr.PrevVolumeName,
              File, LabelType, dev->VolHdr.LabelSize,
              dev->VolHdr.PoolName, dev->VolHdr.MediaType,
-             dev->VolHdr.PoolType, dev->VolHdr.HostName);
+             dev->VolHdr.PoolType, dev->VolHdr.HostName,
+             crypto, salt);
 
    if (dev->VolHdr.VerNum >= 11) {
       char dt[50];
diff --git a/core/src/stored/record.h b/core/src/stored/record.h
index 49ba2a9..74d7051 100644
--- a/core/src/stored/record.h
+++ b/core/src/stored/record.h
@@ -160,6 +160,8 @@ struct Volume_Label {
    */
   int32_t LabelType;                  /**< This is written in header only */
   uint32_t LabelSize;                 /**< length of serialized label */
+  uint8_t Key[65];                    /**< Crypto key */
+  uint8_t Iv[65];                     /**< Crypto IV */
   /*
    * The items below this line are stored on
    * the tape
@@ -190,6 +192,8 @@ struct Volume_Label {
   char LabelProg[50];                 /**< Label program name */
   char ProgVersion[50];               /**< Program version */
   char ProgDate[50];                  /**< Program build date/time */
+  char Salt[8];                       /**< Salt used to protect volume */
+  uint16_t cipher;                    /**< Cipher used to encrypt the volume */
 
 };
 
diff --git a/core/src/stored/stored.h b/core/src/stored/stored.h
index d3d909a..1634743 100644
--- a/core/src/stored/stored.h
+++ b/core/src/stored/stored.h
@@ -62,6 +62,7 @@ const int sd_debuglevel = 300;
 #include "stored_conf.h"
 #include "include/jcr.h"
 #include "vol_mgr.h"
+#include "vol_crypto.h"
 #include "reserve.h"
 
 #ifdef BAREOS_LIB_LIB_H_
diff --git a/core/src/stored/stored_conf.cc b/core/src/stored/stored_conf.cc
index e9b295c..907f5e4 100644
--- a/core/src/stored/stored_conf.cc
+++ b/core/src/stored/stored_conf.cc
@@ -262,6 +262,7 @@ static ResourceItem dev_items[] = {
      NULL},
     {"AutoInflate", CFG_TYPE_IODIRECTION, ITEM(res_dev.autoinflate), 0, 0, 
NULL, "13.4.0-", NULL},
     {"CollectStatistics", CFG_TYPE_BOOL, ITEM(res_dev.collectstats), 0, 
CFG_ITEM_DEFAULT, "true", NULL, NULL},
+    {"VolumeCryptoKey", CFG_TYPE_STRNAME, ITEM(res_dev.vol_crypto_key), 0, 0, 
NULL, NULL, NULL},
     {NULL, 0, {0}, 0, 0, NULL, NULL, NULL}};
 
 /**
diff --git a/core/src/stored/stored_conf.h b/core/src/stored/stored_conf.h
index 69a2137..c8b6d13 100644
--- a/core/src/stored/stored_conf.h
+++ b/core/src/stored/stored_conf.h
@@ -179,6 +179,7 @@ public:
    char *unmount_command;             /**< Unmount command */
    char *write_part_command;          /**< Write part command */
    char *free_space_command;          /**< Free space command */
+   char *vol_crypto_key;              /**< Encryption/Decrypt key for volume */
 
    /*
     * The following are set at runtime

Reply via email to