[pbs-devel] [PATCH v2 proxmox-backup 1/2] docs: add security implications of prune and change detection mode

[pbs-devel] [PATCH v2 proxmox-backup 1/2] docs: add security implications of prune and change detection mode

[Christian Ebner]

Users should be made aware that the data stored in chunks outlives
the backup snapshots on pruning and that backups created using the
change-detection-mode set to metadata might reference chunks
containing files which have vanished since the previous backup, but
might still be accessible when access to the chunks raw data is
possible (client or server side).

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- explicitly mention steps to get rid of chunks for both, regular and
  file-level backups with change detection mode metadata
- reworded and restructured according to feedback

diff --git a/docs/maintenance.rst b/docs/maintenance.rst
index 4bb135e4e..e8a26d69c 100644
--- a/docs/maintenance.rst
+++ b/docs/maintenance.rst
@@ -6,8 +6,34 @@ Maintenance Tasks
 Pruning
 -------
 
-Prune lets you specify which backup snapshots you want to keep.
-The following retention options are available:
+Prune lets you specify which backup snapshots you want to keep, removing others.
+When pruning a snapshot, only the snapshot metadata (manifest, indices, blobs,
+log and notes) is removed. The chunks containing the actual backup data and
+previously referenced by the pruned snapshot, have to be removed by a garbage
+collection run.
+
+.. Caution:: Take into consideration that sensitive information stored in a
+   given data chunk will outlive pruned snapshots and remain present in the
+   datastore as long as referenced by at least one backup snapshot. Further,
+   *even* if no snapshot references a given chunk, it will remain present until
+   removed by the garbage collection.
+
+   Further, file-level backups created using the change detection mode
+   `metadata` can reference backup chunks containing files which have vanished
+   since the previous backup, but might still be accessible when reading the
+   chunks raw data is possible (client or server side).
+
+   To remove chunks containing sensitive data, prune any snapshot made while the
+   data was part of the backup input and run a garbage collection. Further, if
+   using file-based backups with change detection mode `metadata`, additionally
+   prune all snapshots since the sensitive data was no longer part of the backup
+   input and run a garbage collection.
+
+   The no longer referenced chunks will then be marked for deletion on the next
+   garbage collection run and removed by a subsequent run after the grace
+   period.
+
+The following retention options are available for pruning:
 
 ``keep-last <N>``
   Keep the last ``<N>`` backup snapshots.
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 2/2] docs: deduplicate background details for garbage collection

[Christian Ebner]

Currently, common details regarding garbage collection are documented
in the backup client and the maintenance task. Deduplicate this
information by moving the details to the background section of the
maintenance task and reference that section in the backup client
part.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- refine phase 2 garbage collection description
- s/referred/referenced/
- reworded and restructured according to feedback

 docs/backup-client.rst | 28 ++++++++++++----------------
 docs/maintenance.rst   | 38 +++++++++++++++++++++++++++-----------
 2 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index e56e0625b..892be11d9 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -789,29 +789,25 @@ Garbage Collection
 ------------------
 
 The ``prune`` command removes only the backup index files, not the data
-from the datastore. This task is left to the garbage collection
-command. It is recommended to carry out garbage collection on a regular basis.
+from the datastore. Deletion of unused backup data from the datastore is done by
+:ref:`garbage collection<_maintenance_gc>`. It is therefore recommended to
+schedule garbage collection tasks on a regular basis. The working principle of
+garbage collection is described in more details in the related :ref:`background
+section <gc_background>`.
 
-The garbage collection works in two phases. In the first phase, all
-data blocks that are still in use are marked. In the second phase,
-unused data blocks are removed.
+To start garbage collection from the client side, run the following command:
+
+.. code-block:: console
+
+  # proxmox-backup-client garbage-collect
 
 .. note:: This command needs to read all existing backup index files
   and touches the complete chunk-store. This can take a long time
   depending on the number of chunks and the speed of the underlying
   disks.
 
-.. note:: The garbage collection will only remove chunks that haven't been used
-   for at least one day (exactly 24h 5m). This grace period is necessary because
-   chunks in use are marked by touching the chunk which updates the ``atime``
-   (access time) property. Filesystems are mounted with the ``relatime`` option
-   by default. This results in a better performance by only updating the
-   ``atime`` property if the last access has been at least 24 hours ago. The
-   downside is that touching a chunk within these 24 hours will not always
-   update its ``atime`` property.
-
-   Chunks in the grace period will be logged at the end of the garbage
-   collection task as *Pending removals*.
+The progress of the garbage collection will be displayed as shown in the example
+below:
 
 .. code-block:: console
 
diff --git a/docs/maintenance.rst b/docs/maintenance.rst
index e8a26d69c..bba3feff4 100644
--- a/docs/maintenance.rst
+++ b/docs/maintenance.rst
@@ -197,6 +197,8 @@ It's recommended to setup a schedule to ensure that unused space is cleaned up
 periodically. For most setups a weekly schedule provides a good interval to
 start.
 
+.. _gc_background:
+
 GC Background
 ^^^^^^^^^^^^^
 
@@ -222,17 +224,31 @@ datastore or interfering with other backups.
 The garbage collection (GC) process is performed per datastore and is split
 into two phases:
 
-- Phase one: Mark
-  All index files are read, and the access time of the referred chunk files is
-  updated.
-
-- Phase two: Sweep
-  The task iterates over all chunks, checks their file access time, and if it
-  is older than the cutoff time (i.e., the time when GC started, plus some
-  headroom for safety and Linux file system behavior), the task knows that the
-  chunk was neither referred to in any backup index nor part of any currently
-  running backup that has no index to scan for. As such, the chunk can be
-  safely deleted.
+- Phase one (Mark):
+
+  All index files are read, and the access time (``atime``) of the referenced
+  chunk files is updated.
+
+- Phase two (Sweep):
+
+  The task iterates over all chunks and checks their file access time against a
+  cutoff time. The cutoff time is given by either the oldest backup writer
+  instance, if present, or 24 hours and 5 minutes after the start of garbage
+  collection.
+
+  Garbage collection can consider chunk files with access time older than the
+  cutoff time to be neither referenced by any backup snapshot's index, nor part
+  of any currently running backup job. Therefore, these chunks can safeley be
+  deleted.
+
+  Chunks within the grace period will not be deleted and logged at the end of
+  the garbage collection task as *Pending removals*.
+
+.. note:: The grace period for backup chunk removal is not arbitrary, but stems
+   from the fact that filesystems are typically mounted with the ``relatime``
+   option by default. This results in better performance by only updating the
+   ``atime`` property if a file has been modified since the last access or the
+   last access has been at least 24 hours ago.
 
 Manually Starting GC
 ^^^^^^^^^^^^^^^^^^^^
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 1/2] docs: add security implications of prune and change detection mode

[Gabriel Goller]

On 13.11.2024 16:55, Christian Ebner wrote:
>diff --git a/docs/maintenance.rst b/docs/maintenance.rst
>index 4bb135e4e..e8a26d69c 100644
>--- a/docs/maintenance.rst
>+++ b/docs/maintenance.rst
>@@ -6,8 +6,34 @@ Maintenance Tasks
> Pruning
> -------
>
>-Prune lets you specify which backup snapshots you want to keep.
>-The following retention options are available:
>+Prune lets you specify which backup snapshots you want to keep, removing others.
>+When pruning a snapshot, only the snapshot metadata (manifest, indices, blobs,
>+log and notes) is removed. The chunks containing the actual backup data and
>+previously referenced by the pruned snapshot, have to be removed by a garbage
>+collection run.
>+
>+.. Caution:: Take into consideration that sensitive information stored in a
>+   given data chunk will outlive pruned snapshots and remain present in the
>+   datastore as long as referenced by at least one backup snapshot. Further,
>+   *even* if no snapshot references a given chunk, it will remain present until
>+   removed by the garbage collection.
>+
>+   Further, file-level backups created using the change detection mode

Second sentence that begins with 'Further' – maybe substitute this one
with 'Moreover' or 'Additionally' so it reads better.

>+   `metadata` can reference backup chunks containing files which have vanished

use double backticks here to highlight correctly, so: ``metadata``.

>+   since the previous backup, but might still be accessible when reading the
>+   chunks raw data is possible (client or server side).

This sentence is a bit messy and long, maybe we could rewrite it as:

     Moreover, file-level backups created using the change detection mode
     ``metadata`` can reference backup chunks containing files which have
     vanished since the previous backup. These might still be accessible
     by reading the raw data (client or server side).

>+   To remove chunks containing sensitive data, prune any snapshot made while the
>+   data was part of the backup input and run a garbage collection. Further, if
>+   using file-based backups with change detection mode `metadata`, additionally

s/`metadata`/``metadata``/

>+   prune all snapshots since the sensitive data was no longer part of the backup
>+   input and run a garbage collection.
>+
>+   The no longer referenced chunks will then be marked for deletion on the next
>+   garbage collection run and removed by a subsequent run after the grace
>+   period.
>+
>+The following retention options are available for pruning:
>
> ``keep-last <N>``
>   Keep the last ``<N>`` backup snapshots.

Everything else is fine!



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 2/2] docs: deduplicate background details for garbage collection

[Gabriel Goller]

On 13.11.2024 16:55, Christian Ebner wrote:
>diff --git a/docs/maintenance.rst b/docs/maintenance.rst
>index e8a26d69c..bba3feff4 100644
>--- a/docs/maintenance.rst
>+++ b/docs/maintenance.rst
>@@ -197,6 +197,8 @@ It's recommended to setup a schedule to ensure that unused space is cleaned up
> periodically. For most setups a weekly schedule provides a good interval to
> start.
>
>+.. _gc_background:
>+
> GC Background
> ^^^^^^^^^^^^^
>
>@@ -222,17 +224,31 @@ datastore or interfering with other backups.
> The garbage collection (GC) process is performed per datastore and is split
> into two phases:
>
>-- Phase one: Mark
>-  All index files are read, and the access time of the referred chunk files is
>-  updated.
>-
>-- Phase two: Sweep
>-  The task iterates over all chunks, checks their file access time, and if it
>-  is older than the cutoff time (i.e., the time when GC started, plus some
>-  headroom for safety and Linux file system behavior), the task knows that the
>-  chunk was neither referred to in any backup index nor part of any currently
>-  running backup that has no index to scan for. As such, the chunk can be
>-  safely deleted.
>+- Phase one (Mark):
>+
>+  All index files are read, and the access time (``atime``) of the referenced
>+  chunk files is updated.
>+
>+- Phase two (Sweep):
>+
>+  The task iterates over all chunks and checks their file access time against a
>+  cutoff time. The cutoff time is given by either the oldest backup writer
>+  instance, if present, or 24 hours and 5 minutes after the start of garbage
>+  collection.
>+

>+  Garbage collection can consider chunk files with access time older than the

s/can consider/considers/
It always considers chunks with atime older than cutoff to be dangling
afaik.

>+  cutoff time to be neither referenced by any backup snapshot's index, nor part
>+  of any currently running backup job. Therefore, these chunks can safeley be
>+  deleted.

s/safeley/safely/

>+
>+  Chunks within the grace period will not be deleted and logged at the end of
>+  the garbage collection task as *Pending removals*.
>+
>+.. note:: The grace period for backup chunk removal is not arbitrary, but stems
>+   from the fact that filesystems are typically mounted with the ``relatime``
>+   option by default. This results in better performance by only updating the
>+   ``atime`` property if a file has been modified since the last access or the
>+   last access has been at least 24 hours ago.
>
> Manually Starting GC
> ^^^^^^^^^^^^^^^^^^^^

Otherwise this is great!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 1/2] docs: add security implications of prune and change detection mode

[Christian Ebner]

On 11/14/24 10:25, Gabriel Goller wrote:
> On 13.11.2024 16:55, Christian Ebner wrote:
>> diff --git a/docs/maintenance.rst b/docs/maintenance.rst
>> index 4bb135e4e..e8a26d69c 100644
>> --- a/docs/maintenance.rst
>> +++ b/docs/maintenance.rst
>> @@ -6,8 +6,34 @@ Maintenance Tasks
>> Pruning
>> -------
>>
>> -Prune lets you specify which backup snapshots you want to keep.
>> -The following retention options are available:
>> +Prune lets you specify which backup snapshots you want to keep, 
>> removing others.
>> +When pruning a snapshot, only the snapshot metadata (manifest, 
>> indices, blobs,
>> +log and notes) is removed. The chunks containing the actual backup 
>> data and
>> +previously referenced by the pruned snapshot, have to be removed by a 
>> garbage
>> +collection run.
>> +
>> +.. Caution:: Take into consideration that sensitive information 
>> stored in a
>> +   given data chunk will outlive pruned snapshots and remain present 
>> in the
>> +   datastore as long as referenced by at least one backup snapshot. 
>> Further,
>> +   *even* if no snapshot references a given chunk, it will remain 
>> present until
>> +   removed by the garbage collection.
>> +
>> +   Further, file-level backups created using the change detection mode
> 
> Second sentence that begins with 'Further' – maybe substitute this one
> with 'Moreover' or 'Additionally' so it reads better.

True, `Moreover` sounds better to me...

> 
>> +   `metadata` can reference backup chunks containing files which have 
>> vanished
> 
> use double backticks here to highlight correctly, so: ``metadata``.

Acked, thx!

> 
>> +   since the previous backup, but might still be accessible when 
>> reading the
>> +   chunks raw data is possible (client or server side).
> 
> This sentence is a bit messy and long, maybe we could rewrite it as:
> 
>      Moreover, file-level backups created using the change detection mode
>      ``metadata`` can reference backup chunks containing files which have
>      vanished since the previous backup. These might still be accessible

Yes, I do agree that splitting this into two sentences makes it easier 
to read. In that case I would even suggest to explicitly mention that 
this refers to the files, non just the chunks, e.g. by:
...
vanished since the previous backup. These files might still be accessible.
...

>      by reading the raw data (client or server side).
> 
>> +   To remove chunks containing sensitive data, prune any snapshot 
>> made while the
>> +   data was part of the backup input and run a garbage collection. 
>> Further, if
>> +   using file-based backups with change detection mode `metadata`, 
>> additionally
> 
> s/`metadata`/``metadata``/

Acked, thx!

> 
>> +   prune all snapshots since the sensitive data was no longer part of 
>> the backup
>> +   input and run a garbage collection.
>> +
>> +   The no longer referenced chunks will then be marked for deletion 
>> on the next
>> +   garbage collection run and removed by a subsequent run after the 
>> grace
>> +   period.
>> +
>> +The following retention options are available for pruning:
>>
>> ``keep-last <N>``
>>   Keep the last ``<N>`` backup snapshots.
> 
> Everything else is fine!

OK, will send a new version, thx!



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 2/2] docs: deduplicate background details for garbage collection

[Christian Ebner]

On 11/14/24 10:39, Gabriel Goller wrote:
> On 13.11.2024 16:55, Christian Ebner wrote:
>> diff --git a/docs/maintenance.rst b/docs/maintenance.rst
>> index e8a26d69c..bba3feff4 100644
>> --- a/docs/maintenance.rst
>> +++ b/docs/maintenance.rst
>> @@ -197,6 +197,8 @@ It's recommended to setup a schedule to ensure 
>> that unused space is cleaned up
>> periodically. For most setups a weekly schedule provides a good 
>> interval to
>> start.
>>
>> +.. _gc_background:
>> +
>> GC Background
>> ^^^^^^^^^^^^^
>>
>> @@ -222,17 +224,31 @@ datastore or interfering with other backups.
>> The garbage collection (GC) process is performed per datastore and is 
>> split
>> into two phases:
>>
>> -- Phase one: Mark
>> -  All index files are read, and the access time of the referred chunk 
>> files is
>> -  updated.
>> -
>> -- Phase two: Sweep
>> -  The task iterates over all chunks, checks their file access time, 
>> and if it
>> -  is older than the cutoff time (i.e., the time when GC started, plus 
>> some
>> -  headroom for safety and Linux file system behavior), the task knows 
>> that the
>> -  chunk was neither referred to in any backup index nor part of any 
>> currently
>> -  running backup that has no index to scan for. As such, the chunk 
>> can be
>> -  safely deleted.
>> +- Phase one (Mark):
>> +
>> +  All index files are read, and the access time (``atime``) of the 
>> referenced
>> +  chunk files is updated.
>> +
>> +- Phase two (Sweep):
>> +
>> +  The task iterates over all chunks and checks their file access time 
>> against a
>> +  cutoff time. The cutoff time is given by either the oldest backup 
>> writer
>> +  instance, if present, or 24 hours and 5 minutes after the start of 
>> garbage
>> +  collection.
>> +
> 
>> +  Garbage collection can consider chunk files with access time older 
>> than the
> 
> s/can consider/considers/

Acked!

> It always considers chunks with atime older than cutoff to be dangling
> afaik.

This part here is referring to the second phase, so cleaning up the 
chunks. Or what do you mean here by dangling?
> 
>> +  cutoff time to be neither referenced by any backup snapshot's 
>> index, nor part
>> +  of any currently running backup job. Therefore, these chunks can 
>> safeley be
>> +  deleted.
> 
> s/safeley/safely/

Acked!

> 
>> +
>> +  Chunks within the grace period will not be deleted and logged at 
>> the end of
>> +  the garbage collection task as *Pending removals*.
>> +
>> +.. note:: The grace period for backup chunk removal is not arbitrary, 
>> but stems
>> +   from the fact that filesystems are typically mounted with the 
>> ``relatime``
>> +   option by default. This results in better performance by only 
>> updating the
>> +   ``atime`` property if a file has been modified since the last 
>> access or the
>> +   last access has been at least 24 hours ago.
>>
>> Manually Starting GC
>> ^^^^^^^^^^^^^^^^^^^^
> 
> Otherwise this is great!
Thx, will fold in your comments and send a new version



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 2/2] docs: deduplicate background details for garbage collection

[Gabriel Goller]

On 14.11.2024 10:47, Christian Ebner wrote:
>On 11/14/24 10:39, Gabriel Goller wrote:
>>On 13.11.2024 16:55, Christian Ebner wrote:
>>>diff --git a/docs/maintenance.rst b/docs/maintenance.rst
>>>index e8a26d69c..bba3feff4 100644
>>>--- a/docs/maintenance.rst
>>>+++ b/docs/maintenance.rst
>>>@@ -197,6 +197,8 @@ It's recommended to setup a schedule to ensure 
>>>that unused space is cleaned up
>>>periodically. For most setups a weekly schedule provides a good 
>>>interval to
>>>start.
>>>
>>>+.. _gc_background:
>>>+
>>>GC Background
>>>^^^^^^^^^^^^^
>>>
>>>@@ -222,17 +224,31 @@ datastore or interfering with other backups.
>>>The garbage collection (GC) process is performed per datastore and 
>>>is split
>>>into two phases:
>>>
>>>-- Phase one: Mark
>>>-  All index files are read, and the access time of the referred 
>>>chunk files is
>>>-  updated.
>>>-
>>>-- Phase two: Sweep
>>>-  The task iterates over all chunks, checks their file access 
>>>time, and if it
>>>-  is older than the cutoff time (i.e., the time when GC started, 
>>>plus some
>>>-  headroom for safety and Linux file system behavior), the task 
>>>knows that the
>>>-  chunk was neither referred to in any backup index nor part of 
>>>any currently
>>>-  running backup that has no index to scan for. As such, the 
>>>chunk can be
>>>-  safely deleted.
>>>+- Phase one (Mark):
>>>+
>>>+  All index files are read, and the access time (``atime``) of 
>>>the referenced
>>>+  chunk files is updated.
>>>+
>>>+- Phase two (Sweep):
>>>+
>>>+  The task iterates over all chunks and checks their file access 
>>>time against a
>>>+  cutoff time. The cutoff time is given by either the oldest 
>>>backup writer
>>>+  instance, if present, or 24 hours and 5 minutes after the start 
>>>of garbage
>>>+  collection.
>>>+
>>
>>>+  Garbage collection can consider chunk files with access time 
>>>older than the
>>
>>s/can consider/considers/
>
>Acked!
>
>>It always considers chunks with atime older than cutoff to be dangling
>>afaik.
>This part here is referring to the second phase, so cleaning up the 
>chunks. Or what do you mean here by dangling?

Yep that's what I meant, the gc *must* consider these chunks, it's not
optional – so no 'can'.

>>>+  cutoff time to be neither referenced by any backup snapshot's 
>>>index, nor part
>>>+  of any currently running backup job. Therefore, these chunks 
>>>can safeley be
>>>+  deleted.
>>
>>s/safeley/safely/
>
>Acked!

Oh and consider:

Reviewed-by: Gabriel Goller <g.goller@proxmox.com>

for both patches!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel