[PATCH] smb3.1.1: allow dumping GCM256 keys to improve debugging of encrypted shares

[Aurélien Aptel]

Stefan Metzmacher <metze at samba.org> writes:
> No in order to derive the aes256 keys we need the full session key that
> falls out of the authentication and that's 32 bytes is kerberos
> with aes256-cts-hmac-sha1-96 (the default) is used.

Ok

>> I would call it server_in_key and server_out_key.
>
> That's fine too.
>
>> I think we should have 2 ioctl:
>> - GET_FULL_KEY_SIZE: return the size of the struct
>> - GET_FULL_KEY: return the struct
>> 
>> (note: no __packed, this might create all sorts of issues and kernel ABI
>> will be stable across the system anyway. I hope we didn't pack other
>> ioctl in the past... need to check)

I've checked and sadly we use __packed for every struct :(
That means we are forcing users to have packed struct as well which is
not a standard C attribute. Portable C code will have to process each
member field manually. I guess clang and gcc both support it so that's
not a huge deal... anyway that's a problem for another day.

>> struct smb3_full_key_debug_info {
>> 	__u64 session_id;
>> 	__u16 cipher_type;
>>         union {
>>             struct smb3_aes128_debug_info {
>>         	__u8 session_key[16];
>> 		__u8 server_in_key[16];
>> 		__u8 server_out_key[16];
>>             } aes128;
>>             struct smb3_aes256_debug_info {
>>         	__u8 session_key[16];
>> 		__u8 server_in_key[32];
>> 		__u8 server_out_key[32];
>>             } aes256;
>>         } keys;
>> };
>> 
>> * Users will call GET_FULL_KEY_SIZE to allocate a properly sized buffer.
>> * Users can do a switch on cipher_type and process what they know exist
>> * We can update the struct if we need to in the future without breaking
>>   userspace
>> 
>> This should cover any possible updates (AES-512 or some new cipher...)
>> unless I'm missing something.
>
> I think we should have explicit length fields for each key, if it's easier
> we can still use fixed size arrays.

The problem with this is that if we make the keys bigger for a future
cipher, old userspace program will send us a buffer of the size of the
old smaller struct.

eg:

* user on kernel version N compiles:

    struct smb3_cipher_key_debug_info {
        __u64 session_id;
        __u16 cipher_algorithm;
        __u8 session_key_length;
        __u8 session_key[32];
        __u8 server_in_key_length;
        __u8 server_in_key[32];
        __u8 server_out_key_length;
        __u8 server_out_key[32];
    } buf; // sizeof => 16
    
    ioctl(fd, CIFS_DUMP_FULL_KEY, &buf);

* user keeps compiled program and upgrades kernel to version N+1 where the
  kernel added AES-512 or whatever:

* in the kernel, sizeof(smb3_cipher_key_debug_info) is now let's say 24

    copy_to_user(userbuf, &kernelbuf, sizeof(struct smb3_full_key_debug_info));

Since we don't know the size of the user provided buffer and we assume
it's the same as the current size of the struct, So we will actually
write past it (24-16 => invalid write of 8 bytes).

With the extra ioctl to get the size, userspace will always give us a
buffer that matches the struct size of the running system.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)