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

[Troy A. Griffitts]

Hi Jaak,

Thank you for pointing out one problematic input condition which passes
the validation checks already in the code.

Your statement that there is no validation on input is incorrect.  There
is validation on input.  You found one case which passed those
validation checks.  An additional check has been added to handle your
case of passing an empty personalization prefix to the depersonalization
mechanism.

We appreciate your reporting of this unvalidated scenario.  If you find
additional strings which cause issues, please continue to provide feedback.

Regarding the publisher's reason for requesting personalized unlock
codes, in their mind, a user is less likely to share their unlock code
if it is, e.g.,

RISTIOJAJA2019-wEt-lum-UIw-GlQN

They know it does not provide any additional software protection.

I agree with their reasoning that it is a disincentive to share one's
unlock key if the origin of that shared key can be traced back to a user.

Hope this makes sense,

Troy


On 10/30/19 3:30 PM, Jaak Ristioja wrote:
> 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
>
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page