Who needs parameters anyway?



  • I'm working on an application which stores data in encrypted files. To facilitate reading those files back in, someone-who-works-here-no-longer years ago created an implementation of the abstract Java class "InputStream" to allow decrypting such files on-the-fly while reading from them.

    Yesterday, while trying to add a new feature, I used that implementation to try and read some files, and was quite puzzled by the fact that whatever I read came out broken. The class had been in use for years, unchanged, in dozens of places all over the application, and it had always had seemed to work fine. So in order to understand what was going wrong, I took a look at the code, and that was when my head hit the desk because what I saw there was this:

    public int read(byte[]] buffer, int offset, int length) throws IOException {
        return read(buffer);
    }

    For those not familar with the Java Stream classes: that method is supposed to read the next couple of bytes into the given buffer to the position given by 'offset', and up to 'length' in number.

    I'll spare you the horrible details of the (other) read method, but believe me, it just got worse - because that one was only able to read blocks of the same size as the 'blocksize' parameter of the encryption algorithm used (normally 1024 bytes), ignoring the size of the input buffer (the Stream read method is not strictly required to fill up the buffer every time, to account for things like limitations of devices that it might be reading from, but it is generally expected to try its utmost to read as many bytes as possible in one go).

    The only reason why the stream had been working before, was simply because every caller so far had (accidentally?) only been calling the read methods with buffers of the correct blocksize, and an offset (when it was specified) of 0 - and I was the first one to try something "new"; namely, actually trying to use the Stream the way specified by the InputStream contract...



  • @Anonymouse said:

    To facilitate reading those files back in, someone-who-works-here-no-longer years ago created an implementation of the abstract Java class "InputStream" to allow decrypting such files on-the-fly while reading from them.

    I really hope it was years ago, because CipherInputStream has been in the standard API since 2002.

    PS 1024 bytes? That's an awfully large block size. Most block ciphers use blocks of 64 bits or 128 bits.



  • @pjt33 said:

    PS 1024 bytes? That's an awfully large block size. Most block ciphers use blocks of 64 bits or 128 bits.
    Well, they're going to be really secure, by doing RSA-8192 by default ... non of that pesky symmetric ciphers, only lo05ers use them!

     



  • @pjt33 said:

    I really hope it was years ago, because CipherInputStream has been in the standard API since 2002.
    Sigh ... the origins of that part of the code are lost in the murky depths of ancient version control repositories, so I don't know exactly when it was created. But when has that sort of thing ever stopped a developer hell-bent on inventing his own wheel? ("What do you mean, the wheel already exists? But mine has much prettier colors!").

    The code of that application has plenty of WTFs, that whole encryption/decryption part is just one of them - one that I'd love to rip out in it's entirety and replace with something sane(r). But the harsh reality is that there's terabytes of files already encrypted the 'old' way, and simply replacing the decryption logic would (most likely) make them unreadable.

    That place is like a mine field that none of the other developers (current and previous ... there have been many of them over the years) dares to tread on. I was the first with the courage to actually make a significant change by making the decrypting input stream behave like a regular stream. The standard practise for reading crypted files before that was to read the entire crypted file (in 'blocksize' chunks) from the stream (the only way it worked, as I outlined), write all of it into a new temporary file and then open a new stream on the temp file and serve all requests from there...

    Fortunately, they at least didn't try inventing their own encryption schemes instead of using established algorithms - they only wrote the code themselves (no idea how robust these implementations are, and for the aforementioned reasons, they're "DO NOT TOUCH!!!" anyway).


Log in to reply