All of lore.kernel.org
 help / color / mirror / Atom feed
* [dm-crypt] Transactional updates for LUKS2 metadata?
@ 2021-04-09 18:46 Schneider, Robert
  2021-04-10 19:27 ` [dm-crypt] " Milan Broz
  0 siblings, 1 reply; 5+ messages in thread
From: Schneider, Robert @ 2021-04-09 18:46 UTC (permalink / raw)
  To: dm-crypt

Hi,

Is there a way to get transactions over multiple metadata operations when using libcryptsetup?

Imagine I have some mechanism for unlocking which requires information from a token associated to a keyslot. Now I'd like to update that information in the token together with the keyslot.
But if the machine reboots in between the API calls, I believe my unlock mechanism would be broken - for example, when I've updated the keyslot but still have the old token.

I could not find an operation to update a token atomically, nor any transaction operations (like open transaction, commit) in the API. I've had a quick glance at the source code and it looks to me like the header is updated in memory and finally written to disk with replica, using a sequence number. This suggests to me that transactions should be relatively easy to implement. However I don't see the full picture of course, so I'd like to know your opinion.

As an alternative to transactions within the libcryptsetup API, it looks like it's possible to perform a header backup, then manipulate the detached (backup) header, and finally restore the header - as long as the volume key is not changed. Do you think that's a reasonable alternative, or are there potential pitfalls here?

Thanks,
Robert
_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dm-crypt] Re: Transactional updates for LUKS2 metadata?
  2021-04-09 18:46 [dm-crypt] Transactional updates for LUKS2 metadata? Schneider, Robert
@ 2021-04-10 19:27 ` Milan Broz
  2021-04-11 12:09   ` Schneider, Robert
  0 siblings, 1 reply; 5+ messages in thread
From: Milan Broz @ 2021-04-10 19:27 UTC (permalink / raw)
  To: Schneider, Robert, dm-crypt

On 09/04/2021 20:46, Schneider, Robert wrote:
> Hi,
> 
> Is there a way to get transactions over multiple metadata operations when using libcryptsetup?
> 
> Imagine I have some mechanism for unlocking which requires information from a token associated to a keyslot. Now I'd like to update that information in the token together with the keyslot.
> But if the machine reboots in between the API calls, I believe my unlock mechanism would be broken - for example, when I've updated the keyslot but still have the old token.
> 
> I could not find an operation to update a token atomically, nor any transaction operations (like open transaction, commit) in the API. I've had a quick glance at the source code and it looks to me like the header is updated in memory and finally written to disk with replica, using a sequence number. This suggests to me that transactions should be relatively easy to implement. However I don't see the full picture of course, so I'd like to know your opinion.
> 
> As an alternative to transactions within the libcryptsetup API, it looks like it's possible to perform a header backup, then manipulate the detached (backup) header, and finally restore the header - as long as the volume key is not changed. Do you think that's a reasonable alternative, or are there potential pitfalls here?

LUKS2 header is not database and never will be. Implementing transactions smells like overengineering here.

For "complex" operations you can recover after failure by removing token metadata and recreate them, it is expected that you still
have a backup keyslot or volume key (or header backup).

And yes, you can modify backup header externally and then update it. But then the recovery can fail with a partial write or a media failure,
so you will end in the same situation you tried to avoid.

Milan
_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dm-crypt] Re: Transactional updates for LUKS2 metadata?
  2021-04-10 19:27 ` [dm-crypt] " Milan Broz
@ 2021-04-11 12:09   ` Schneider, Robert
  2021-04-20  8:43     ` Ondrej Kozina
  0 siblings, 1 reply; 5+ messages in thread
From: Schneider, Robert @ 2021-04-11 12:09 UTC (permalink / raw)
  To: Milan Broz, dm-crypt

Thank you very much for your response!

I am implementing a form of key rotation for a "backup" key (a secondary key). The backup key has associated data which I wanted to store in a token. This associated data is required to get the passphrase. To rotate this backup key, I have to adjust the keyslot json object, the token, and the keyslot binary data.

Currently, I am using two keyslots to implement a safe update. I'm relying on the atomicity of API calls such as crypt_keyslot_add_by_* to a certain degree. To maintain the pair of keyslots, I write a sequence number into the token object. The key is rotated as follows:
1. crypt_keyslot_add_* - add new keyslot
2. crypt_token_set - add metadata for new keyslot, with incremented sequence number
3. crypt_keyslot_wipe - remove old keyslot and tokens

While this is not too difficult, I'll also have to deal with the existence of two keyslot+token pairs (old, new). Plus, it's possible to leak the keyslot if a reboot occurs between steps 1 and 2. It would be an ordinary keyslot without associated metadata, hence I cannot be sure that it's the backup key - the keyslot number is not fixed. At some point I realized that it might be possible to leverage cryptsetup to get rid of this complexity in my tool, since libcryptsetup can provide slightly better atomicity guarantees than currently exposed in the API. For example, if I could add the token first, then the keyslot, the aforementioned leak would be gone. A leaked token is easy to detect, since a token is invalid if the associated keyslot is invalid.

I've been reading through the spec and the implementation again, and it seemed to me that header restore cannot be atomic due to the binary keyslot area: Since there are two copies of the header with a sequence number and checksum, we can restore one header first, sync, and then restore the second header. Should an error occur at any point, one of the header replica should still be valid. However if an error occurs during the restore of the binary keyslot area, we cannot recover nor detect the error since there's no checksum for this area. (I wonder why the binary keyslot area is unprotected...)

It looks to me as if a safe update of a keyslot would therefore have to be done along these lines:
1. Load header into memory.
2. Allocate new keyslot area by using in-memory data of the header. Includes update of keyslot json object in memory to refer to new binary keyslot.
3. Lock the device (locked for exclusive writes) and ensure sequence number is still the same.
4. Write new binary keyslot data to disk. This does not overwrite the binary data of the old keyslot.
5. Sync.
6. Write first header to disk.
7. Sync.
8. Write second header to disk.
9. Sync.
10. Wipe old keyslot binary data.
11. Sync.
12. Unlock device.

In this case, we could leak the old keyslot between step 9 and step 11, but at least we could detect it as a non-zero, unreferenced keyslot. For key rotation, I believe it is important to not leak the old keyslot binary data, since we're trying to make unlocking with the old passphrase impossible (since the salt of the kdf is removed, this might not be an issue).
The LUKS2_keyslot_store function together with the luks2_keyslot keyslot_handler seems to follow this roughly, except that locking is different and it only stores a new keyslot but doesn't remove any.

In this algorithm, we can also persist any modifications of the binary header and the json metadata in steps 6/8 safely due to the checksum and sequence number. Therefore, it should be possible to atomically change both a keyslot and the associated token.

Generic transactions with multiple modifications of the keyslot area now appear to me to be rather complicated to implement. However, multiple modifications of the json metadata and a single, final keyslot_add or keyslot_change should be relatively easy to implement. I understand that you want to keep complexity low, but would you be willing to accept a contribution in this direction? Or do you maybe have a better idea how I could solve my issue?

Thanks again,
Robert


-----Original Message-----
From: Milan Broz <gmazyland@gmail.com> 
Sent: Samstag, 10. April 2021 21:27
To: Schneider, Robert <robert.schneider03@sap.com>; dm-crypt@saout.de
Subject: Re: [dm-crypt] Transactional updates for LUKS2 metadata?

On 09/04/2021 20:46, Schneider, Robert wrote:
> Hi,
> 
> Is there a way to get transactions over multiple metadata operations when using libcryptsetup?
> 
> Imagine I have some mechanism for unlocking which requires information from a token associated to a keyslot. Now I'd like to update that information in the token together with the keyslot.
> But if the machine reboots in between the API calls, I believe my unlock mechanism would be broken - for example, when I've updated the keyslot but still have the old token.
> 
> I could not find an operation to update a token atomically, nor any transaction operations (like open transaction, commit) in the API. I've had a quick glance at the source code and it looks to me like the header is updated in memory and finally written to disk with replica, using a sequence number. This suggests to me that transactions should be relatively easy to implement. However I don't see the full picture of course, so I'd like to know your opinion.
> 
> As an alternative to transactions within the libcryptsetup API, it looks like it's possible to perform a header backup, then manipulate the detached (backup) header, and finally restore the header - as long as the volume key is not changed. Do you think that's a reasonable alternative, or are there potential pitfalls here?

LUKS2 header is not database and never will be. Implementing transactions smells like overengineering here.

For "complex" operations you can recover after failure by removing token metadata and recreate them, it is expected that you still
have a backup keyslot or volume key (or header backup).

And yes, you can modify backup header externally and then update it. But then the recovery can fail with a partial write or a media failure,
so you will end in the same situation you tried to avoid.

Milan
_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dm-crypt] Re: Transactional updates for LUKS2 metadata?
  2021-04-11 12:09   ` Schneider, Robert
@ 2021-04-20  8:43     ` Ondrej Kozina
  2021-04-22  7:00       ` Schneider, Robert
  0 siblings, 1 reply; 5+ messages in thread
From: Ondrej Kozina @ 2021-04-20  8:43 UTC (permalink / raw)
  To: dm-crypt; +Cc: Schneider, Robert, Milan Broz

On 4/11/21 2:09 PM, Schneider, Robert wrote:
> Thank you very much for your response!
> 
> I am implementing a form of key rotation for a "backup" key (a secondary key). The backup key has associated data which I wanted to store in a token. This associated data is required to get the passphrase. To rotate this backup key, I have to adjust the keyslot json object, the token, and the keyslot binary data.
> 
> Currently, I am using two keyslots to implement a safe update. I'm relying on the atomicity of API calls such as crypt_keyslot_add_by_* to a certain degree. To maintain the pair of keyslots, I write a sequence number into the token object. The key is rotated as follows:
> 1. crypt_keyslot_add_* - add new keyslot
> 2. crypt_token_set - add metadata for new keyslot, with incremented sequence number
> 3. crypt_keyslot_wipe - remove old keyslot and tokens

I have a suggestion based on the atomic writes of LUKS2 metadata. (LUKS2 
metadata only because as you've correctly noticed keyslots binary area 
writes does not provide any such guaranties at the moment).

Also it's based on assumption your application understands the token 
content and therefore can do something like this:

1) Write token with following metadata (let's say in token id 6)
{
   "type" : "my_token",
   "keyslots" : [],

   "new_keyslot" : 21,  <-- does not exist yet
   "current_keyslot" : 5, <-- it's the old keyslot, stil valid
}

2) crypt_keyslot_add_* (with keyslot id 21)

3) wipe_keyslot 5

4) replace token 6 with new metadata (this will be atomic):
{
   "type" : "my_token",
   "keyslots" : [],

   "current_keyslot" : 21
}

If anytime in between 1) and 4) crash occurs you can double check:

Does 'new_keyslot' exist and is it ok (just check passphrase by unlock 
API function with name set to NULL). Yes? go on. No? we crashed during 
step 2)

Provided both "current_keyslot" and "new_keyslot" fields are in token 
metadata: Does keyslot referenced in "current_keyslot" still exist? Yes? 
we crashed during step 3) No? we crashed in before step 4) finished 
correctly.

If "current_keyslot" field is missing the transaction was successfully 
completed.

Internal LUKS2 format validation code only enforces content in fields 
"type" and "keyslots". The later can be left empty.

Did I understand your use case correctly?

Kind regards
O.

_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dm-crypt] Re: Transactional updates for LUKS2 metadata?
  2021-04-20  8:43     ` Ondrej Kozina
@ 2021-04-22  7:00       ` Schneider, Robert
  0 siblings, 0 replies; 5+ messages in thread
From: Schneider, Robert @ 2021-04-22  7:00 UTC (permalink / raw)
  To: Ondrej Kozina, dm-crypt; +Cc: Milan Broz

> Did I understand your use case correctly?

Yes! Thank you very much for this proposal :)
It's much better than the two-token system I'm currently using.

Still, I wonder if it's possible to get this entirely inside libcryptsetup: It can do multiple changes to metadata internally, but doesn't expose this in the API.

There might be a relatively simple way to expose transactions: It should be possible to implement an atomic header restore if enough space is available in the keyslots binary area of the target of the restore.

1. Lock the target.
2. Iterate over the metadata in the backup/source header. For each allocation in the keyslots binary area in the backup header, perform an equally-sized allocation in the target header. This can fail if not enough space is available in the target header (for both its current allocations and the allocations from the backup header).
3. Adjust the backup header json metadata in memory to refer to the new allocations in the target header.
4. Copy the contents of the backup header keyslots binary area to the new allocations in the target header. Assuming some form of atomic writes (e.g. sector-size writes), this should be safe since we don't modify the content of the existing allocations in the target header keyslots binary area. At most, we're overwriting them with the same content.
5. Sync.
6. Overwrite primary header + json in the target.
7. Sync.
8. Optional: wipe the old allocations in the target binary keyslots area.
9. Optional: Sync.
10. Overwrite secondary header + json in the target.
11. Sync

What worries me a bit is that you can fail between 7 and 11 and leave some of the old binary keyslots unwiped. This wouldn't be a concern if a repair operation that restores the second header from the first wipes the unused binary keyslots.

There is potential for optimization of 2 and 3: diff the binary areas and only work on the difference. Additionally, if you create the backup header by doing all the keyslot additions first, and then remove other keyslots, the diff between backup and target will be disjunct, and restore won't fail due to size constraints.

What do you think?

Thanks again,
Robert
_______________________________________________
dm-crypt mailing list -- dm-crypt@saout.de
To unsubscribe send an email to dm-crypt-leave@saout.de

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-04-22  7:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09 18:46 [dm-crypt] Transactional updates for LUKS2 metadata? Schneider, Robert
2021-04-10 19:27 ` [dm-crypt] " Milan Broz
2021-04-11 12:09   ` Schneider, Robert
2021-04-20  8:43     ` Ondrej Kozina
2021-04-22  7:00       ` Schneider, Robert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.