Hi Margarita,

Margarita Manterola <ma...@debian.org> writes:
> Please respect the original author indenting style.
There hardly is such a thing. The source code as-is uses different
indenting styles. But that is a minor nitpick not worthy of
discussion. Feel free to change my patch as you see fit.

> This should be perror("open(outfile)");
Correct.

> +    } else {
> +      if ((sz = BFDecrypt(infd, outfd, key, key2, &options)) == 0) {
>          fprintf(stderr, "Invalid encryption key for file: %s\n", infile);
>          exit(1);
>        }
> +      ftruncate(outfd, sz);
> +    }
>
> What's the point of the truncate there?
Since I don’t do all the processing in memory anymore, I cannot truncate
the file in memory (as the old code did). Therefore I just write the key
+ padding block to the output file and then truncate it. There is a
comment in the decryption function which explains how to improve this
situation, but since that is not really related to this issue, I didn’t
spend any time on that.

> For the complicated part, I would need more time to review it, but I think
> it's a too disruptive change to make it into wheezy's release.  It's
> basically a complete refactor of the whole encryption/decryption
> functions.
I agree that this is a large change. I tried to convince you that the
change is alright by providing a fuzzing-style automated testing script
which ran on my machine for thousands of iterations and did not find any
regressions.

I have to wonder: Since you seem opposed to fixing this for wheezy, why
did you not update your bugreport accordingly so that my — apparently
unnecessary and useless — work could have been prevented? It cost me
many hours to work on this.

> Also, I'm really not sure it makes much sense to have this tool.  It could
> easily be replaced by a shell script that calls openssl and shred
> appropriately.
I do agree, but that would break at least around 300 installations
(according to popcon). Consider that bcrypt could be part of custom
backup scripts.

In case you want to provide a wrapper script which is binary-compatible
with the original file format, feel free, but I think that is definitely
a too large change for wheezy either ;-).

-- 
Best regards,
Michael


--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to