Re: KRandom regression + fix

2016-05-05 Thread Kevin Kofler
Albert Astals Cid wrote:
> From my "i know nothing about random numbers", i guess it's hard to write
> a unit test for a sequente of random numbers, you can get ten "3" in a row
> and it's still a valid random sequence.

https://xkcd.com/221/ ;-)

Kevin Kofler



Re: KRandom regression + fix

2016-04-27 Thread Frederik Schwarzer

Am 27.04.2016 23:45 schrieb Albert Astals Cid:
El dimecres, 27 d’abril de 2016, a les 11:42:46 CEST, Frederik 
Schwarzer va

escriure:

Am 27.04.2016 08:48 schrieb Johannes Huber:



> thanks for the patch. When i read "randon numbers were predictable"
> instantly
> a alarm bell rings in my head. Is this a security issue?

The docs of rand() state that you should not use it for serious 
business

like cryptography
http://en.cppreference.com/w/cpp/numeric/random/rand
and the most serious business I could see within KDE was PIN 
generation

for Bluetooth pairing. But you can never know who is using it for what
outside of the KDE infrastructure.

Since I am neither a core developer (just maintaining a game which was
beaten by the consequences of this issue) nor a crypto guy, I cannot
really assess the severity of such a regression but my first thoughts
were:
- why is there no unit test cathing this?


Because noone wrote one (obvious answer)


Yes, of course that's the obvious answer. :)
I asked because the answer could have been something along the lines of 
"because KRandom is old; do not use it; we have something new in 
frameworks" or so.



From my "i know nothing about random numbers", i guess it's hard to 
write a
unit test for a sequente of random numbers, you can get ten "3" in a 
row and

it's still a valid random sequence.


srand() is the same as srand(1) so it uses a fixed seed. Thus two 
initialisations produce the same sequence. Not sure though if this can 
be done in a unit test.




- should KRandom api doc pass through the note of not using it for
serious business in general?


Probably makes sense adopting what rand() says, yes. Would you propose 
a

patch?


Already did: https://git.reviewboard.kde.org/r/127767/
Comments about the wording welcome. :)

Regards,
Frederik


Re: KRandom regression + fix

2016-04-27 Thread Albert Astals Cid
El dimecres, 27 d’abril de 2016, a les 11:42:46 CEST, Frederik Schwarzer va 
escriure:
> Am 27.04.2016 08:48 schrieb Johannes Huber:
> 
> Hi Johannes,
> 
> >> KRandom saw a regression in KCoreAddon's 5.21.0 release, which impacts
> >> a
> >> wide range of applications and use cases. Since the rand() was not
> >> seeded, the numbers generated were predictable, which is ugly in games
> >> and probably alarming for bluetooth pairing.
> > 
> > thanks for the patch. When i read "randon numbers were predictable"
> > instantly
> > a alarm bell rings in my head. Is this a security issue?
> 
> The docs of rand() state that you should not use it for serious business
> like cryptography
> http://en.cppreference.com/w/cpp/numeric/random/rand
> and the most serious business I could see within KDE was PIN generation
> for Bluetooth pairing. But you can never know who is using it for what
> outside of the KDE infrastructure.
> 
> Since I am neither a core developer (just maintaining a game which was
> beaten by the consequences of this issue) nor a crypto guy, I cannot
> really assess the severity of such a regression but my first thoughts
> were:
> - why is there no unit test cathing this?

Because noone wrote one (obvious answer)

From my "i know nothing about random numbers", i guess it's hard to write a 
unit test for a sequente of random numbers, you can get ten "3" in a row and 
it's still a valid random sequence.

> - should KRandom api doc pass through the note of not using it for
> serious business in general?

Probably makes sense adopting what rand() says, yes. Would you propose a 
patch?

Cheers,
  Albert

> 
> That's why I CC'ed kde-core-devel.
> 
> Regards,
> Frederik




Re: KRandom regression + fix

2016-04-27 Thread Ivan Čukić
If used for crypto, it is a security issue.

Now, while I don't expect anyone who implements the crypto stuff to use
KRandom, I do agree that KRandom docs should explicitly state this.

Cheers,
Ivan


Re: KRandom regression + fix

2016-04-27 Thread Frederik Schwarzer

Am 27.04.2016 08:48 schrieb Johannes Huber:

Hi Johannes,

KRandom saw a regression in KCoreAddon's 5.21.0 release, which impacts 
a

wide range of applications and use cases. Since the rand() was not
seeded, the numbers generated were predictable, which is ugly in games
and probably alarming for bluetooth pairing.


thanks for the patch. When i read "randon numbers were predictable" 
instantly

a alarm bell rings in my head. Is this a security issue?


The docs of rand() state that you should not use it for serious business 
like cryptography

http://en.cppreference.com/w/cpp/numeric/random/rand
and the most serious business I could see within KDE was PIN generation 
for Bluetooth pairing. But you can never know who is using it for what 
outside of the KDE infrastructure.


Since I am neither a core developer (just maintaining a game which was 
beaten by the consequences of this issue) nor a crypto guy, I cannot 
really assess the severity of such a regression but my first thoughts 
were:

- why is there no unit test cathing this?
- should KRandom api doc pass through the note of not using it for 
serious business in general?


That's why I CC'ed kde-core-devel.

Regards,
Frederik