Hello!

On 29.10.19 23:42, Troy A. Griffitts wrote:
#1 was included as an update to our engine with this commit:

commit f4ac4caeacd762c90c2b2cef5755bf745e3a6d58
Author: scribe <scribe@bcd7d363-81e1-0310-97ec-a550e20fc99c>
Date:   Sat Dec 29 21:23:25 2018 +0000

     Added personalization mechanism for cipher keys
    git-svn-id: https://crosswire.org/svn/sword/trunk@3614
bcd7d363-81e1-0310-97ec-a550e20fc99c

As the maintainer of Sword++, I regularly merge in changes from Sword. When this commit was made in 2018, I did not figure out why exactly it was needed. Because the code also seemed suspicious, I decided not to merge this commit into the Sword++ codebase at that point. Haven now given it some additional thought, I'm even more sceptical.

The current implementation does not seem to provide any additional security benefits. It could actually make things worse by providing a false sense of security. Could you please explain why exactly the "personal keys" logic is needed in the first place? What do the stakeholders believe to gain?

On a more technical side, the function seems to make certain undocumented presumptions about the input string. The function does not validate its inputs and crashes in simple cases like in the following:

    SWBuf test("-asdf");
    SWCipher::personalize(test, false); // SIGFPE !?

Since it is not acceptable for frontends to crash on invalid user input, they would need to validate the input before passing it to this function. How should they do that? What is the format for the input string? Would it be possible document these requirements in the inline code documentation for SWCipher::personalize() please?


Best regards,
J

_______________________________________________
sword-devel mailing list: sword-devel@crosswire.org
http://www.crosswire.org/mailman/listinfo/sword-devel
Instructions to unsubscribe/change your settings at above page

Reply via email to