[jira] [Closed] (OFBIZ-7028) Use SecureRandom instead of Random where appropriate, and randomUUID for externalKey

["Jacques Le Roux (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/OFBIZ-7028?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacques Le Roux closed OFBIZ-7028.
----------------------------------
       Resolution: Fixed
    Fix Version/s: 13.07.04
                   15.12.01
                   14.12.01

Committed in
trunk r1741684  
R15.12 r1741687
R14.12 r1741688
R13.07 r1741689 (incomplete missing EntityCrypto, conflicts)




> Use SecureRandom instead of Random where appropriate, and randomUUID for externalKey
> ------------------------------------------------------------------------------------
>
>                 Key: OFBIZ-7028
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-7028
>             Project: OFBiz
>          Issue Type: Sub-task
>          Components: ALL COMPONENTS
>    Affects Versions: Trunk
>            Reporter: Jacques Le Roux
>            Assignee: Jacques Le Roux
>            Priority: Minor
>             Fix For: 14.12.01, 15.12.01, 13.07.04
>
>
> After a discussion we had privately with [~gdraperi]  (on PMC private ML) some time ago, I decided to apply his recommended changes in appropriate places.
> Quoting Grégory:
> {quote}
> I had a look to the source code and I would like to speak about how we use Random and Math.Random() java classes.
> To give a little bit of context, these two functions are marked as unsecured because if you are able to know one output you are able to predict the future outputs.
> As a short overview random java function relies on a linear congruential generator (a complex word for a simple thing). Reference http://franklinta.com/2014/08/31/predicting-the-next-math-random-in-java/
> LCG works like this: (a * x + c) mod m where a,c and m are static values and x is the previous output (except for the first output where we determine x and it is called the seed)
> So for example with a = 3, c = 5, m = 7 and we start with the seed x = 0,
> then the next few random numbers generated will be: 5 = (3 * 0 + 5) mod 7,
> 6 = (3 * 5 + 5) mod 7, 2 = (3 * 6 + 5) mod 7, 4 = (3 * 2 + 5) mod 7
> It means that if I know the last output (4) I can predict what will be the
> next output:
> (3 * 4 +5) mod 7 -> 3.
> We use a lot of Math.Random which relies on Random() class and also Random() class itself especially in the LoginWorker class where it is used to generate the external link id. At that time I have no real proven vulnerability but that worries me for two things .
> # If it is possible to retrieve the state of Math.Random() from another place in the code it will be possible to predict future links
> # Even if the link is complex and almost impossible to guess by brute force we downgrade the security as the space of possible values for the link id is really lower than the space for session id. Example: 3EBA63DE1C0BE677A17CAB483689F9C vs EL708615493372
> The topic is not urgent but I would recommend in the future using SecureRandom class and maybe UUID.randomUUID to generate the external link id
> --
> Grégory Draperi
> {quote}
> Because using SecureRandom comes with a cost, I have identified the places where it's reasonable to keep the non secured Random (like tests, internal sequences, etc.).
> Using UUID.randomUUID to generate the external link id seems also the best possible solution.
> Also, though there are no real proven vulnerabilities, I decided to backport as much as possible since it's now public.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)