hello!
...some time has passed since I've contacted you last time about
crypto/loop issues... but now it's time again to annoy you :-)
btw; while I'm at it, a question which came to mind while writing this
mail; is it safe to call...
if (current->need_resched) schedule();
...from within the transfer function? are there possible race conditions
where another transfer request may come in for the same blocks, while
another one is still in progress?
some months ago, when I proposed to switch IV calculation completely to
fixed a fixed 512 byte blocksize, I was reminded, that the international
crypto patch was not the only one to make use of the IV calculatiion, and
doing so would break other crypto packages around...
...so I suggested that I'd try a more backward compatible approach, which
would allow old packages to retain compatibility, and new packages aware
of the new flag to set IV calculation to 512 byte blocks...
well the result of this experiment can be seen in the attachment...
so... any comments?
ps: this are the only changes to kernel sources optionally (!) 'required',
in order to use the international crypto api
(see http://cryptoapi.sourceforge.net/ and the package README file
accessible through CVS for more information)
regards,
--
Herbert Valerio Riedel / Phone: (EUROPE) +43-1-58801-18840
Email: [email protected] / Finger [email protected] for GnuPG Public Key
GnuPG Key Fingerprint: 7BB9 2D6C D485 CE64 4748 5F65 4981 E064 883F 4142
On Sun, Jul 22, 2001 at 12:21:30PM +0200, Herbert Valerio Riedel wrote:
> hello!
>
> ...some time has passed since I've contacted you last time about
> crypto/loop issues... but now it's time again to annoy you :-)
>
> btw; while I'm at it, a question which came to mind while writing this
> mail; is it safe to call...
>
> if (current->need_resched) schedule();
>
> ...from within the transfer function? are there possible race conditions
yes we are. writes are a no brainer, reads are either handled by the
thread for blkdev backed I/O and from the pagecache for file backed I/O.
> where another transfer request may come in for the same blocks, while
> another one is still in progress?
>
> some months ago, when I proposed to switch IV calculation completely to
> fixed a fixed 512 byte blocksize, I was reminded, that the international
> crypto patch was not the only one to make use of the IV calculatiion, and
> doing so would break other crypto packages around...
>
> ...so I suggested that I'd try a more backward compatible approach, which
> would allow old packages to retain compatibility, and new packages aware
> of the new flag to set IV calculation to 512 byte blocks...
> well the result of this experiment can be seen in the attachment...
>
> so... any comments?
- int IV = index * (PAGE_CACHE_SIZE/bsize) + offset/bsize;
+ unsigned long IV = loop_get_iv(lo, (pos - lo->lo_offset) >> LO_IV_SECTOR_BITS);
+
size = PAGE_CACHE_SIZE - offset;
^^^^
it's more complicated than that.
If you want to change the IV every 512bytes you must as well reduce the
underlined size and recalculate the IV or it won't do what you are
expecting. Same fixup of the logic is needed for even the blkdev backed
I/O (you can't use the same IV for the whole b_size anymore)
I believe the main reason we are using the softblocksize of the device
instead of 512bytes is just because it made strightforward the above
issues. Of course performance will decrease somehow this way.
OTOH the advantage I can see in your change is that the cryptoloop won't
break across a change of the softblocksize if the crypto plugin is using
the IV (like mkfs -b 4096 writing with an IV at 1k and then ext2 reading
with an IV of 4k). Actually if we make this change I believe breaking
backwards compatibility and making it the default could be a good thing,
any crypto plugin depending on the current IV calculation is just
subject to be not completly functional. Furthmore if we make this change
to the IV cryptoloop API, I'm wondering if we really need to make the
default 512bytes and not 1k, if we make the fixed IV granularity 1k then
all current drivers should keep working fine because the mkfs always
defaults to 1k the first time (performance will be better too). Not sure
about the lack of security of having the double number of bits sharing
the same IV but I assume that wasn't the reason of the change.
Andrea
hi
On Sun, 22 Jul 2001, Andrea Arcangeli wrote:
> On Sun, Jul 22, 2001 at 12:21:30PM +0200, Herbert Valerio Riedel wrote:
> > some months ago, when I proposed to switch IV calculation completely to
> > fixed a fixed 512 byte blocksize, I was reminded, that the international
> > crypto patch was not the only one to make use of the IV calculatiion, and
> > doing so would break other crypto packages around...
> >
> > ...so I suggested that I'd try a more backward compatible approach, which
> > would allow old packages to retain compatibility, and new packages aware
> > of the new flag to set IV calculation to 512 byte blocks...
> > well the result of this experiment can be seen in the attachment...
> >
> > so... any comments?
>
> - int IV = index * (PAGE_CACHE_SIZE/bsize) + offset/bsize;
> + unsigned long IV = loop_get_iv(lo, (pos - lo->lo_offset) >> LO_IV_SECTOR_BITS);
> +
> size = PAGE_CACHE_SIZE - offset;
> ^^^^
> it's more complicated than that.
>
> If you want to change the IV every 512bytes you must as well reduce the
> underlined size and recalculate the IV or it won't do what you are
> expecting. Same fixup of the logic is needed for even the blkdev backed
> I/O (you can't use the same IV for the whole b_size anymore)
I haven't told you how the cryptoloop.c module contains the necessary
logic... :-)
static int
transfer_cryptoapi(struct loop_device *lo, int cmd, char *raw_buf,
char *loop_buf, int size, int real_block)
{
struct cipher_context * cx = (struct cipher_context *) lo->key_data;
struct cipher_implementation *ci = cx->ci;
int (*encdecfunc)(struct cipher_context *cx, const u8 *in, u8 *out,
int size, const u32 iv[]);
char *in, *out;
if (cmd == READ) {
#if defined(CRYPTOLOOP_ATOMIC)
encdecfunc = ci->decrypt_atomic;
#else
encdecfunc = ci->decrypt;
#endif
in = raw_buf;
out = loop_buf;
} else {
#if defined(CRYPTOLOOP_ATOMIC)
encdecfunc = ci->encrypt_atomic;
#else
encdecfunc = ci->encrypt;
#endif
in = loop_buf;
out = raw_buf;
}
#if defined(USE_LO_IV_MODE_SECTOR)
#warning USE_LO_IV_MODE_SECTOR enabled -- hope you know what this means
/* split up transfer request into 512 byte data blocks */
while (size > 0) {
int _size = (size > LO_IV_SECTOR_SIZE) ? LO_IV_SECTOR_SIZE : size;
u32 iv[4] = { 0, };
iv[0] = cpu_to_le32 (real_block);
encdecfunc (cx, in, out, _size, iv);
real_block++;
size -= _size;
in += _size;
out += _size;
}
#else
{
u32 iv[4] = { 0, };
iv[0] = cpu_to_le32 (real_block);
encdecfunc (cx, in, out, size, iv);
}
#endif
return 0;
}
> I believe the main reason we are using the softblocksize of the device
> instead of 512bytes is just because it made strightforward the above
> issues. Of course performance will decrease somehow this way.
well that's in part why I left the size untouched, and moved the logic to
the transfer function, that way if we have some means to optimize larger
transfers (maybe because we don't make use at all of the IV so we don't
need 512 byte chunks but can handle bigger ones w/o problems...)
> OTOH the advantage I can see in your change is that the cryptoloop won't
> break across a change of the softblocksize if the crypto plugin is using
> the IV (like mkfs -b 4096 writing with an IV at 1k and then ext2 reading
> with an IV of 4k). Actually if we make this change I believe breaking
> backwards compatibility and making it the default could be a good thing,
> any crypto plugin depending on the current IV calculation is just
> subject to be not completly functional. Furthmore if we make this change
that's especially true for filesystems which change the blocksize
frequently; e.g. XFS uses different blocksizes, since it has different
sections which get accessed with different blocksizes...
> to the IV cryptoloop API, I'm wondering if we really need to make the
> default 512bytes and not 1k, if we make the fixed IV granularity 1k then
> all current drivers should keep working fine because the mkfs always
> defaults to 1k the first time (performance will be better too). Not sure
> about the lack of security of having the double number of bits sharing
> the same IV but I assume that wasn't the reason of the change.
security is not the issue; it's more of practical terms... since 512 byte
seems to be the closest practical transfer size (there isn't any smaller
blocksize supported with linux) it seems natural to use that one....
best regards,
--
Herbert Valerio Riedel / Phone: (EUROPE) +43-1-58801-18840
Email: [email protected] / Finger [email protected] for GnuPG Public Key
GnuPG Key Fingerprint: 7BB9 2D6C D485 CE64 4748 5F65 4981 E064 883F 4142
On Sun, Jul 22, 2001 at 08:53:50PM +0200, Herbert Valerio Riedel wrote:
> I haven't told you how the cryptoloop.c module contains the necessary
> logic... :-)
I see now ;).
> security is not the issue; it's more of practical terms... since 512 byte
> seems to be the closest practical transfer size (there isn't any smaller
> blocksize supported with linux) it seems natural to use that one....
to me it sounds more natural to use the 1k blocksize that seems to be
backwards compatible automatically (without the special case), the only
disavantage of 1k compared to 512bytes is the decreased security, so if
the decreased security is not your concern I'd suggest to use the 1k
fixed granularity for the IV. 1k is also the default BLOCK_SIZE I/O
granularity used by old linux (which incidentally is why it seems
backwards compatible automatically).
Andrea
Andrea Arcangeli writes:
> On Sun, Jul 22, 2001 at 08:53:50PM +0200, Herbert Valerio Riedel wrote:
>> security is not the issue; it's more of practical terms... since
>> 512 byte seems to be the closest practical transfer size (there
>> isn't any smaller blocksize supported with linux) it seems natural
>> to use that one....
>
> to me it sounds more natural to use the 1k blocksize that seems to
> be backwards compatible automatically (without the special case),
> the only disavantage of 1k compared to 512bytes is the decreased
> security, so if the decreased security is not your concern I'd
> suggest to use the 1k fixed granularity for the IV. 1k is also the
> default BLOCK_SIZE I/O granularity used by old linux (which
> incidentally is why it seems backwards compatible automatically).
Being backwards compatible with non-Linus kernels is less important
than choosing a block size that is fit for all block devices.
Disks, partitions, and block-based filesystems are all organized
in power-of-two multiples of 512 bytes.
The smaller size can handle the larger size; the reverse is not true.
We all have to live with the size choice until the end of time.
On Sun, Jul 22, 2001 at 09:47:26PM -0400, Albert D. Cahalan wrote:
> Andrea Arcangeli writes:
> > On Sun, Jul 22, 2001 at 08:53:50PM +0200, Herbert Valerio Riedel wrote:
>
> >> security is not the issue; it's more of practical terms... since
> >> 512 byte seems to be the closest practical transfer size (there
> >> isn't any smaller blocksize supported with linux) it seems natural
> >> to use that one....
> >
> > to me it sounds more natural to use the 1k blocksize that seems to
> > be backwards compatible automatically (without the special case),
> > the only disavantage of 1k compared to 512bytes is the decreased
> > security, so if the decreased security is not your concern I'd
> > suggest to use the 1k fixed granularity for the IV. 1k is also the
> > default BLOCK_SIZE I/O granularity used by old linux (which
> > incidentally is why it seems backwards compatible automatically).
>
> Being backwards compatible with non-Linus kernels is less important
> than choosing a block size that is fit for all block devices.
> Disks, partitions, and block-based filesystems are all organized
> in power-of-two multiples of 512 bytes.
>
> The smaller size can handle the larger size; the reverse is not true.
I think you misunderstood the issue, IV granularity has nothing to do
with anything hardware. It is only a software convention.
> We all have to live with the size choice until the end of time.
1k fits for all block devices and filesystems you can invent, but it
also has the bonus that it should also be backwards compatible with the
current crypto transfer API.
As I just said said twice in this thread: the _only_ disavantage of 1k
compared to 512bytes is the decrease security, you will share the same
IV for the double number of bits, that's all. If the security point is
not your concern 1k is better (and a bit faster).
Andrea
...moving it back to the kernel list...
On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
> You don't read 1k. You only read 512bytes and you encrypt them using
> the IV of such 512byte block.
>
> Then the next 512byte block will use again the same IV again. What's the
> problem?
ok, so I didn't understand you well in the first place :-)
you actually want only to change the IV calculation to be based on a
hardcoded 1024 IV-blocksize, but don't change anything else in the loop.c
module, right? (if not, ignore the following text... :-)
if so, I really don't like it;
1.) I really don't like to encode 2 512-blocks with the same IV, (I
actually thought I could just switch to 1024-blocks and thus have it
sensible again, but since you only change the IV calculation without
changing the data-transfer granularity it won't work)
2.) It will break filesystems using set_blocksize(lo_dev, 512)... why?
say, we had a transfer request from the buffer cache requesting starting
at byte 0 (initial IV=0) of the block device for length of 2048 bytes,
this would lead to
transfer_filter (buf, bufsize=2048, IV=0);
since the transfer_filter contains the logic to split the buffer into
chunks with the blocksize (e.g. 512 byte blocks) required*) for
encryption the loop may look like:
int IV2 = IV << 1;
while (bufsize > 0) {
encdecfunc (buf, 512, IV2 >> 1);
IV2++;
buf += 512;
}
which leads to the following calls for the mentioned example:
encdecfunc (buf, 512, 0);
encdecfunc (buf+512, 512, 0);
encdecfunc (buf+1024, 512, 1);
encdecfunc (buf+1536, 512, 1);
so far so good; but now imagine that the original request get's changed
a bit, to start at offset 512 (initial IV still 0!!) instead of offset 0;
transfer_filter (buf, bufsize=2048, IV=0); // the same values are
// passed for bufsize and IV!
encdecfunc (buf, 512, IV=0); // OK
encdecfunc (buf+512, 512, IV=0); // wrong, IV should be 1
encdecfunc (buf+1024, 512, IV=1); // OK
encdecfunc (buf+1536, 512, IV=1); // wrong, IV should be 2
...so you see?
that's why it's better IMHO to stick to the same IV-granularity that is
used for transfers, otherwise you'd have to lose performance by
passing only single IV-blocksize length data-blocks to the filter
functions which need to be aligned...
*) actually only filters making use of the IV parameter would need to
split it up, other function may process bigger blocks at once, and
thus minimizing possible overhead...
>> and btw, at least one other popular crypto loop filter uses 512 byte
>> based IVs, namely loop-AES...
> I guess it emulates it internally, so it should keep working if we do
> the fixed 1k granularity for the IV (while it should break the on-disk
> format if we do the 512byte granularity).
no, you can't emulate it internally if the passed IV is calculated based
on a bigger IV-blocksize than the needed one... that's the problem, and why
it's IMHO better to calculate based on the smallest used IV-blocksize :-(
(that way you can always convert it to a bigger IV-blocksize based one)
regards,
--
Herbert Valerio Riedel / Phone: (EUROPE) +43-1-58801-18840
Email: [email protected] / Finger [email protected] for GnuPG Public Key
GnuPG Key Fingerprint: 7BB9 2D6C D485 CE64 4748 5F65 4981 E064 883F 4142
On Tue, Jul 24, 2001 at 11:56:02AM +0200, Herbert Valerio Riedel wrote:
>
> ...moving it back to the kernel list...
>
> On Mon, 23 Jul 2001, Andrea Arcangeli wrote:
> > You don't read 1k. You only read 512bytes and you encrypt them using
> > the IV of such 512byte block.
> >
> > Then the next 512byte block will use again the same IV again. What's the
> > problem?
> ok, so I didn't understand you well in the first place :-)
>
> you actually want only to change the IV calculation to be based on a
> hardcoded 1024 IV-blocksize, but don't change anything else in the loop.c
> module, right? (if not, ignore the following text... :-)
yes
>
> if so, I really don't like it;
>
> 1.) I really don't like to encode 2 512-blocks with the same IV, (I
> actually thought I could just switch to 1024-blocks and thus have it
> sensible again, but since you only change the IV calculation without
> changing the data-transfer granularity it won't work)
the only data transfer that you can be using right now is the one with
softblocksize 1k (or you would not been able to mount the fs), so I
don't change the IV granularity for those in use cryptoloops and that's
why 1k is automatically backwards compatible and it will(/should ;) work.
> 2.) It will break filesystems using set_blocksize(lo_dev, 512)... why?
If a filesystem uses set_blocksize(512) you couldn't create it, this is
why I'm saying 1k softblocksize should take care of the existing
cryptoloops.
> say, we had a transfer request from the buffer cache requesting starting
> at byte 0 (initial IV=0) of the block device for length of 2048 bytes,
the length will always be 512bytes because you said the softblocksize is
512.
> this would lead to
>
> transfer_filter (buf, bufsize=2048, IV=0);
bufsize will be 512.
>
> since the transfer_filter contains the logic to split the buffer into
> chunks with the blocksize (e.g. 512 byte blocks) required*) for
but anyways even if bufsize could be 2048 I think the logic for the IV
progression should be in the highlevel if something, not in the low
layer because:
> encryption the loop may look like:
>
> int IV2 = IV << 1;
> while (bufsize > 0) {
> encdecfunc (buf, 512, IV2 >> 1);
> IV2++;
> buf += 512;
> }
the real cost is encdecfunc not the while(bufsize >0) loop, so you can
also take a cleaner transfer API and have the knowledge of the
granularity of the IV only in one place without duplicating the
knowledge and the code of the logic in all the crypto plugins.
Andrea