Index | Thread | Search

From:
Crystal Kolipe <kolipe.c@exoticsilicon.com>
Subject:
Re: Replace Blowfish with AES in vnode disk driver
To:
Filip Cernoch <filipcernoch@posteo.net>
Cc:
tech@openbsd.org
Date:
Mon, 22 Sep 2025 16:34:32 +0100

Download raw body.

Thread
  • Crystal Kolipe:

    Replace Blowfish with AES in vnode disk driver

  • This thread previously diverted to a general discussion about vnd encryption,
    and I didn't look closely at the actual diff that was attached...
    
    But having taken a closer look now, how exactly is this ever supposed to work?
    
    Apart from the fact that the diff has tabs replaced with spaces, it doesn't
    even compile when that is fixed, due to very basic C errors, (for example it
    removes the declaration of 'bsize' as an int, but continues to reference that
    variable).
    
    I found the referenced post to -tech from back in 2006 that has the original
    proposal to move to twofish, and in that case most of what was required was
    effectively s/blf/twf/ and I can only assume that this diff was created on the
    premise that simply doing little more than s/blf/aes/ would be enough?
    
    However, more worrying is the fact that within all of this confusion, the code
    that initialises the IV based on the block number is being subtly removed:
    
    On Tue, Sep 16, 2025 at 03:50:32PM +0000, Filip Cernoch wrote:
    > -       for (i = 0; i < size/bsize; i++) {
    > -               memset(iv, 0, sizeof(iv));
    > -               memcpy(iv, &off, sizeof(off));
    > -               blf_ecb_encrypt(sc->sc_keyctx, iv, sizeof(iv));
    > +       for (i = 0; i < size/bsize; i++) { 
    > +               AES_Encrypt_ECB(sc->sc_keyctx, addr, daddr, bsize);     
    
    Why?  This would cause identical plaintexts written to different disk blocks
    to encrypt to the same ciphertext.
    
    What exactly was the thinking behind all this?
    
    
  • Crystal Kolipe:

    Replace Blowfish with AES in vnode disk driver