On 11/17/2016 11:06 PM, Ondrej Kozina wrote:
> On 11/17/2016 05:35 PM, Andrey Ryabinin wrote:
>> On 11/16/2016 11:47 PM, Ondrej Kozina wrote:
>>> (Please still consider it to be RFC only, I need to modify the uspace 
>>> teststuite
>>> again due to changes in key_string format. Also the changes to dm-crypt 
>>> documentation
>>> will follow before final submit. Feature wide I'd consider the patch being 
>>> complete
>>> unless any bugs would emerge)
>>>
>>> The kernel key service is a generic way to store keys for the use of
>>> other subsystems. Currently there is no way to use kernel keys in dm-crypt.
>>> This patch aims to fix that. Instead of key userspace may pass a key
>>> description with preceding ':'. So message that constructs encryption
>>> mapping now looks like this:
>>>
>>>   <cipher> [<key>|:<key_string>] <iv_offset> <dev_path> <start> 
>>> [<#opt_params> <opt_params>]
>>>
>>> where <key_string> is in format: <key_size>:<key_type>:<key_description>
>>>
>>> Currently we only support two elementary key types: 'user' and 'logon'.
>>> Keys may be loaded in dm-crypt either via <key_string> or using
>>> classical method and pass the key in hex representation directly.
>>>
>>
>> I think we need to hexify key description too, because it can contain spaces.
> 
> I see. You're right the kernel key description may really contain whitespace 
> chars, bummer. Well what I'm thinking atm is rejecting any keys with 
> descriptions containing whitespaces. But let me ask Mike or Alasdair what do 
> they think about it.
> 
>> <key_size> seems like unnecessary complication. Kernel knows key_size, it 
>> doesn't need
>> that information from userspace.
> 
> Milan already explained that. I just add that general rule for dm tables is 
> what you input via load ioctl() you should get back exactly the same via 
> table status ioctl(). And also there's no other way how to get dm-crypt key 
> size if you input it via kernel keyring key.
> 

Yeah, I get that, but Milan said that we need to *get* that information from 
the kernel.
My concern is about loading key_size into the kernel.
If I understand you right, we need it just for consistency between table_load 
and table_status ioctls(). I guess it's ok then.

>> Handling different types is probably an overkill too. If it works with logon 
>> keys,
>> why would we need to use 'user' keys?
> 
> Well your original patch used 'user' type so I assumed you have good reason 
> to use it.

I used 'user' because I wasn't sure if userspace requires ability to read keys 
or not.


> But anyway it's not so painful to add option to choose from 2 different key 
> types imo. Loading tables is not a hot path.
> 

Ok, fair enough. 

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to