Re: [PATCH net v3 11/14] afs: Create a server appdata key

[David Howells <dhowells@redhat.com>]

Simon Horman <horms@kernel.org> wrote:

> [Severity: High]
> Does this toksize calculation omit the 4 bytes required for the level field? 
> 
> It looks like the sizes added up correspond to starttime (8), lifetime (4),
> bytelife (4), and expirationtime (8).
> 
> If the level field is omitted, toksize will be 4 bytes too small.

Yeah.

> Can this manual advance of the xdr pointer hide the token size mismatch?
>
> Because xdr is advanced based on the miscalculated encsize rather than the
> actual number of fields written, it appears this will bypass the adatasize
> sanity check on the next line.

As it happens, the encryption padding hides the problem.

> When crypto_krb5_encrypt() executes, it seems it will only process the
> truncated toksize bytes of plaintext. This would leave the trailing 4 bytes
> of the serialized XDR unencrypted and overwritten by the appended Kerberos
> checksum. 

The buffer is sized and padded out by crypto_krb5_how_much_buffer() and
kzalloc() preclears it, so crypto_krb5_encrypt() happens to work correctly
most of the time as the data it encrypts is rounded out to a whole encryption
blocks.

David