[sword-devel] Engine personal cipher support / Nestle - Aland 28th ed. German Bible Society

[Jaak Ristioja]

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 at 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