Review session manager

[Etiene]

• Review the session module on Sailor and make a better security analysis

[xspager]

I suggest creating a new config entry pointing to the path where the session files are created. Also we should be able to store sessions at least in memory and on a database, maybe even somethings exotic like storing it on signed cookies on the user browser.

[felipedaragon]

IMO this would be more than a simple enhancement, it would be a security patch. I will not be able to work on this, but I'm interested in reviewing the future changes. To anyone working on it, make sure to read this and http://lua-users.org/lists/lua-l/2014-05/msg00715.html. I tried to get this fixed with Tomás but he didn't consider my finding to be accurate and was pretty much unresponsive. I remember checking how @develCuy implemented his SID generator in Ophal and he did a good work using the luuid library, so maybe he can help with this. One issue I thought however is that the library has not been successfully ported to other platforms than Linux yet (I think I found an old post of Hisham somewhere about trying to compile it to Windows but with no luck) and Sailor is pretty much cross-platform and cross-environment now.

[develCuy]

@felipedaragon, you are right, luuid is not viable for cross-platform and cross-environment. Perhaps we can relay on a pure lua library that uses luasocket's gettime() https://github.com/Tieske/uuid it is way more safe, yet not sure if Sailor has luasocket as requirement, if that is the case then we solved the randomness issue.

Regarding where to store sessions, that is another beast, not everyone wants to use a database or in-memory nor they need that much speed, so relaying on filesystem is kind of mandatory. Somehow session storage has to be specified, perhaps a default class and let settings specify other alternatives. Database storage is a good option, the question is: what is the approach for db stuff? any ORM implemented already? pure SQL? specific driver for supported databases? Once we have that clear we can talk about how to tackle that stuff.

For reference read my article on session handling approaches: http://www.develcuy.com/en/session-handling-approaches-future-ophal

[felipedaragon]

Hi Fernando - It looks like there is some issue with Tieske/uuid and nginx workers and someone says it cannot be solved in pure Lua with multiple processes and solved it by replacing it with a C library, check this out https://github.com/Mashape/kong/issues/478#issuecomment-154766034 This leads to https://github.com/Mashape/lua-uuid, which is available for Mac and Ubuntu. We would still need a port to Arch Linux and Windows

Regarding the session storage, I leave this one with @xspager and Etiene to comment :)

EDIT: just noticed that Mashape/lua-uuid relies on the same C code you used in Ophal

[felipedaragon]

It would be wonderful if we could get the final patch for the randomness issue merged with the cgilua project

[develCuy]

Don't loose hope on Lua that fast :) I have proposed a solution in new solution: https://github.com/Tieske/uuid/issues/6

[develCuy]

@Etiene, @felipedaragon, @xspager, can you give this a try?

https://github.com/Tieske/uuid/issues/6#issuecomment-189939496

I created a test script that is supposed to fix the seeding issue. If the method works we need a folder to store the seeds files, along with a method that deletes files older than a configurable lifetime. The same applies to session UUIDs, whether they are stored in database or files, they should have a lifetime.

In Ophal I have a cron.cgi that takes care of expired session files. Crontab runs cron.cgi every minute.

[Tieske]

imho the best solution would be to extend the https://github.com/Mashape/lua-uuid project to include cross-platform support. Uuid generation is an important feature and such a cross-platform implementation would be a major addition to the Lua ecosystem.

[felipedaragon]

@develCuy, maybe you are right and I kind of lost hope on pure Lua for handling this task (would love to have some input from @agentzh on this).

Check out how PHP does it http://stackoverflow.com/a/27298129 It uses /dev/urandom as one of the ingredients, just like the libuuid code used by Mashape/lua-uuid. So what I'm thinking is for it to be rock solid, I'm not sure if things like using urandom when it is available or other sources can be left out of the equation.

[develCuy]

@Tieske, it does the same as https://github.com/LuaDist/luuid/blob/master/luuid.c which also includes support for windows.

[Tieske]

missed that, sorry. Whats the issue then? why not just use that lib?

[develCuy]

@felipedaragon, I forgot you said before that luuid is not cross-platform, can you confirm?

[felipedaragon]

@develCuy yup. I said that because I never saw the library compiled for all the platforms. Just to be sure I downloaded the latest LuaDist binaries and I don't think it's there (it's not included with Lua for Windows neither). Will it work if we give a luarocks install luuid under Mac or Windows? Or we will get something like https://gist.github.com/switzer/5612958 ?

[felipedaragon]

I think this is a major challenge surrounding this. If we go for a C library, then it must be as easily deployable across all platforms as if it was a pure Lua library

[develCuy]

@felipedaragon, I prefer to have a pure Lua solution for now. Yet, if somebody wants to compile a native library we should provide documentation on how to do it. If we agree on it then we just need to wrap @Tieske's module with something like this: https://github.com/Tieske/uuid/issues/6#issuecomment-189939496

[felipedaragon]

@develCuy Fernando - I tried your code using Apache and CGILua and sometimes it is returning repeated IDs. Here is a small sample with the repeated ones marked in bold: 51046901-a3bd-4148-c1dd-e096080a34ed 552f7590-be6f-4359-c398-2bcb6ed0db78 595b811f-d920-4469-c453-7701d3968203 5d878daf-f3d2-4579-c50e-c337395c288e 610625fa-5fa2-4f73-cf99-0d6d4ca37da0 65323189-7a54-4083-c054-59a2b269232b 6d8949a7-b0b8-43a4-c3ca-f10e7df47140 7109e0f2-1b88-4c9d-cc55-3b44903bc552 7534ec82-363a-4eae-ce10-8779f6016bdd 7960f811-51eb-4fbe-cfcb-d3af5bc71268 7960f811-51eb-4fbe-cfcb-d3af5bc71268 810b9beb-d76d-4ac8-ca11-691bd4d40d05 8537a77a-f21f-4bd9-cbcc-b5503a9ab490 8537a77a-f21f-4bd9-cbcc-b5503a9ab490 8962b309-0dd1-4de9-cd87-00869f605a1b 8d8ebf99-2883-4ef9-ce43-4cbc052501a5 910e57e4-9353-47f3-c7ce-97f2186d55b8 99656f02-c9b6-4a14-ca44-2e5de3f8a2cd 9d907b91-e468-4b24-cbff-7a9349be4958 a53c1e6b-6aea-462e-c645-10ffc2cb44f5 ad93368a-a04e-494f-c9bb-a86a8d57910b b53eda64-26d0-4459-c401-3ed606648ca8 b96ae6f3-4181-4569-c5bc-8a0b6b293333 b96ae6f3-4181-4569-c5bc-8a0b6b293333 bd95f282-5c33-4679-c677-d641d1efd9bd

The code for the test app was simply:

---Your code followed by:
cgilua.header('Set-Cookie','TESTSID='..uuid())
cgilua.htmlheader()
print(("SEED: %s | UUID: %s"):format(seed, uuid()))

I used the Burp Sequencer tool to collect these from the response header: http://resources.infosecinstitute.com/session-randomness-analysis-burp-suite-sequencer/

[moteus]

This library builds on Windows without problem. There problen in rockspec because it declare external deps from libuuid on Windows. But in fact there no external deps. It just need link with rpcrt4.lib

[felipedaragon]

@moteus Yeah! This is the same library we were mentioning. IMO we need to make this easily deployable like luasocket is today or we will have an usage barrier for less experienced users. Not everyone knows how to compile stuff using VC

[moteus]

LuaRocks should work. Just need to fix rockspec IMO

[felipedaragon]

@moteus could you make your luuid binaries downloadable from somewhere? Have you tried the 64-bit compilation as well?

[moteus]

this is rockspec which works on Windows

package = "luuid"
version = "20120509-2"
source = {
   url = "http://www.tecgraf.puc-rio.br/~lhf/ftp/lua/5.2/luuid.tar.gz",
   md5 = "cd6c758f163b41b27a76b3d57cf730fd",
   dir = "uuid"
}

description = {
   summary = "A library for UUID generation",
   detailed = [[
      A library for generating universally unique identifiers based on
      libuuid, which is part of e2fsprogs.
   ]],
   homepage = "http://www.tecgraf.puc-rio.br/~lhf/ftp/lua/#luuid",
   license = "Public domain"
}

dependencies = {
   "lua >= 5.2, < 5.4"
}

external_dependencies = {
   platforms = { unix = {
      LIBUUID = {
         header = "uuid/uuid.h",
         library = "libuuid.so"
      }
   }}
}

build = {
   type = "builtin",
   platforms = {
      windows = { modules = { uuid = {
         sources = {'luuid.c', 'wuuid.c'};
         libraries = {'rpcrt4'};
      }}},
      unix    = { modules = { uuid = {
         sources = {'luuid.c'};
         libraries = {'uuid'};
      }}}
   }
}

This works with msvc but to build with MinGW need little fix in wuuid.h

#ifndef UUID_DEFINED
#define _WIN32_WINNT 0x500       /* include Win2K API for UuidCreateSequential() */
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <rpc.h>
#else
#include <sys/time.h>
#endif

Change to

#define _WIN32_WINNT 0x500       /* include Win2K API for UuidCreateSequential() */
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <rpc.h>

#ifdef __MINGW32__
#include <sys/time.h>
#endif

Same rockspec and changes works with luuid for Lua 5.1

[Tieske]

I send 2 updated rockspecs to @hishamhm for review and upload. Both of which build properly on Windows, for 5.1 and the 5.2/5.3 combo.

[Tieske]

I should have refreshed the page, we duplicated efforts. But I didn't have to change anything specific for MinGW (then again, I didn't test with MS toolchains)

(using gcc 4.8.1, on Win10)

[moteus]

I think luarocks support patches. I never use them but I think I saw it in some rockspec. So may be it possible add patch to rockspec itself.

ps. I test this rockspec with LuaRocks 2.2.0 With Lua 5.1-5.3 and with MSVC 2010 and MinGW So I install all 6 modules without errors.

[Tieske]

I didn't need the patch. What OS were you using?

[Tieske]

PS. I did the exact same thing, except for the patch. So the ones I send to Hisham should at least work out of the box on the MS toolchains

[moteus]

I am on Windows 2k3

>mingw32-gcc --version
mingw32-gcc (GCC) 4.8.1
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

It works with MSVC but does not work on MinGW. Because it does not define time_t and timeval in windows.h MinGW require also include sys/time.h

[Tieske]

without the patch;

C:\Users\Gebruiker\Downloads>luarocks install luuid-20120501-2.rockspec
Using luuid-20120501-2.rockspec... switching to 'build' mode
mingw32-gcc -O2 -c -o luuid.o -Ic:/lua/include/lua/5.1 luuid.c
mingw32-gcc -O2 -c -o wuuid.o -Ic:/lua/include/lua/5.1 wuuid.c
mingw32-gcc -shared -o uuid.dll luuid.o wuuid.o -lrpcrt4 c:/lua/bin/lua51.dll -lm
Updating manifest for c:\lua\/lib/luarocks/rocks-5.1
luuid 20120501-2 is now built and installed in c:\lua\ (license: Public domain)

C:\Users\Gebruiker\Downloads>

using rockspec;

package = "luuid"
version = "20120509-2"
source = {
   url = "http://www.tecgraf.puc-rio.br/~lhf/ftp/lua/5.2/luuid.tar.gz",
   md5 = "cd6c758f163b41b27a76b3d57cf730fd",
   dir = "uuid"
}
description = {
   summary = "A library for UUID generation",
   detailed = [[
      A library for generating universally unique identifiers based on
      libuuid, which is part of e2fsprogs.
   ]],
   homepage = "http://www.tecgraf.puc-rio.br/~lhf/ftp/lua/#luuid",
   license = "Public domain"
}
dependencies = {
   "lua >= 5.2, < 5.4"
}
external_dependencies = {
   platforms = {
      unix = {
         LIBUUID = {
            header = "uuid/uuid.h",
            library = "libuuid.so"
         }
      }
   }
}
build = {
   type = "builtin",
   platforms = {
      win32 = {
         modules = {
            uuid = {
               libraries = {
                  "rpcrt4",
               },
               sources = {
                  "luuid.c",
                  "wuuid.c",
               }
            }
         }
      },
      unix = {
         modules = {
            uuid = {
               libraries = {
                  "uuid", 
               },
               sources = {
                  "luuid.c",
               }
            }
         }
      }
   }
}

[moteus]

Strange. Now I also have no errors. But I saw warnings about undefined time_t :)

[felipedaragon]

@Tieske @moteus this is excellent! :) I tried luarocks install luuid in my Arch Linux box where I have Lua 5.2 installed and the luuid library was installed and is working fine!

@Etiene can you check if the Mac install is working as expected?

[felipedaragon]

In my Mint box with Lua 5.1 it complained about LIBUUID not being installed. Got it installed with sudo apt-get install uuid-dev and after this the installation through luarocks also worked fine.

[felipedaragon]

@develCuy Fernando - Just for comparison, here is a sample from uuid-win32 running from within CGILua processes. Again I got these through threaded connections using the Burp Sequencer:

855bdb6c-5552-4ff7-bd27-01de5598af69 9b7287ba-cd15-4fe9-bbd4-2ac096713e3c dd5ff0a9-1784-4f7e-8632-6db80412e54f 32cafce6-ed39-4bdf-af81-6a3a19119229 89aae441-b7b4-429a-8951-3d550aae5338 d652b6d8-cf4e-4262-9427-853d40ce70c1 51741d7b-1172-4b93-8cba-fbdfa08bd760 208d46fe-d98d-4a6d-9443-4a81183ff30f 5c6cba58-1062-43d8-9506-fd4cd0fd8cbd d4ba0c94-314f-4a44-bb9a-29ea6bcdaf8f 1eb6a379-a24d-4e15-b718-8596393fda5d 29645b39-692b-497a-864e-57d37cf8e5bf eca66864-7d28-474c-a6c0-c165fab9cb79 f03edfa4-7fb6-4d05-bff0-546924f9222f 10a8b8b5-102b-4254-bc6f-f76dfb237f48 bbb36120-1e5b-455d-b6b1-0b9734f79bea c6da2dbd-e30f-4665-a2ab-0b003f6dc1b5 44a02600-d675-4eac-9717-c5ba3fa1f0f9 0622145a-4878-413c-b46d-89931e898ef5 911b7d82-0a97-4069-87dc-35001c6ff918 25358f64-143b-4aa9-81b4-474fbfade3d3 ae386e81-f7cf-413c-b76d-dcb812f3bf4b c2472277-8f08-4a45-a25b-2b66b850bff8 a77652fb-4de4-478a-851b-0ec181adb2ad 54a2767a-7777-4b6e-bef8-6aaaf37db956

These look a lot more random. The first sequence of characters from @Tieske's library was following the ascending order (71, 75, 79, 81, 85...) any idea why? And the fourth sequence was starting with "c" (c1dd, c398, etc) and following what seems to be the ascending order. Also the results for the Bit-level analysis is green for uuid, like the automated tests from the tool are indicating these are more random with less anomalies found.

[develCuy]

@felipedaragon, libuuid knows to do random job pretty well, while @Tieske's is relaying on Lua's randomness which is pretty basic. Yet a "pure" Lua way needs to be seeded with something more elaborated. Btw, at this point, getting luuid to compile well looks easier than reinventing the wheel. Hopefully we learned something from exploring a bit.

[Tieske]

the c binding is definitely the better one to use!

[thibaultcha]

As a side note, only compatible with OSX and Linux (so probably not what you are looking for), but worth considering for performance reasons (if the use case is to use LuaJIT) is the FFI implementation: https://github.com/bungle/lua-resty-uuid

[hishamhm]

I just uploaded @Tieske's rockspecs of luuid to luarocks.org — if anyone wants to take over maintenance of this rock in the repo, let me know, as it is currently unmantained.

[develCuy]

@hishamhm I'm happy to take over. Please.

[hishamhm]

@develCuy done, thank you! https://luarocks.org/modules/develcuy/luuid

[Etiene]

@felipedaragon Ok, I'll check it on OS X :)

[moteus]

Just tested

> luarocks-5.2 install luuid
Installing https://rocks.moonscript.org/luuid-20120509-2.src.rock...
Using https://rocks.moonscript.org/luuid-20120509-2.src.rock... switching to 'build' mode
cl /MD /O2 -c -Foluuid.obj -IG:/lua/5.2/include luuid.c
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 16.00.30319.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

luuid.c
c:\windows\temp\luarocks_luuid-20120509-2-8069\uuid\wuuid.h(22) : fatal error C1083: Cannot open include file: 'sys/time.h': No such file or directory

Error: Build error: Failed compiling object luuid.obj

P.S. I test my rockspec with changed sources.

[Tieske]

P.S. I test my rockspec with changed sources.

What do you mean by that exactly? I tested again, now straight from the LuaRocks repo and it works like a charm. I get;

C:\>setlua 53
        1 file(s) copied.
Done. Installed lua53.exe as lua.exe.
Done. Installed luarocks53.bat as luarocks.bat.

C:\>luarocks install luuid
Installing https://luarocks.org/luuid-20120509-2.src.rock...
Using https://luarocks.org/luuid-20120509-2.src.rock... switching to 'build' mode
mingw32-gcc -O2 -c -o luuid.o -Ic:/lua/include/lua/5.3 luuid.c
mingw32-gcc -O2 -c -o wuuid.o -Ic:/lua/include/lua/5.3 wuuid.c
mingw32-gcc -shared -o uuid.dll luuid.o wuuid.o -lrpcrt4 c:/lua/bin/lua53.dll -lm
Updating manifest for c:\lua\/lib/luarocks/rocks-5.3
luuid 20120509-2 is now built and installed in c:\lua\ (license: Public domain)

C:\>setlua 51
        1 file(s) copied.
Done. Installed lua51.exe as lua.exe.
Done. Installed luarocks51.bat as luarocks.bat.

C:\>luarocks install luuid
Installing https://luarocks.org/luuid-20120501-2.src.rock...
Using https://luarocks.org/luuid-20120501-2.src.rock... switching to 'build' mode
mingw32-gcc -O2 -c -o luuid.o -Ic:/lua/include/lua/5.1 luuid.c
mingw32-gcc -O2 -c -o wuuid.o -Ic:/lua/include/lua/5.1 wuuid.c
mingw32-gcc -shared -o uuid.dll luuid.o wuuid.o -lrpcrt4 c:/lua/bin/lua51.dll -lm
Updating manifest for c:\lua\/lib/luarocks/rocks-5.1
luuid 20120501-2 is now built and installed in c:\lua\ (license: Public domain)

C:\>setlua 52
        1 file(s) copied.
Done. Installed lua52.exe as lua.exe.
Done. Installed luarocks52.bat as luarocks.bat.

C:\>luarocks install luuid
Installing https://luarocks.org/luuid-20120509-2.src.rock...
Using https://luarocks.org/luuid-20120509-2.src.rock... switching to 'build' mode
mingw32-gcc -O2 -c -o luuid.o -Ic:/lua/include/lua/5.2 luuid.c
mingw32-gcc -O2 -c -o wuuid.o -Ic:/lua/include/lua/5.2 wuuid.c
mingw32-gcc -shared -o uuid.dll luuid.o wuuid.o -lrpcrt4 c:/lua/bin/lua52.dll -lm
Updating manifest for c:\lua\/lib/luarocks/rocks-5.2
luuid 20120509-2 is now built and installed in c:\lua\ (license: Public domain)

C:\>

[moteus]

MinGW works but msvc complain about sys/time.h I use this library quite a long time. I edit source to do not inclute this on MSVC and I be able build it (I did not use luarocks). And when I write and test rockspec I call luarocks make ... so I test it with changed sources. I what to say that origin sources does not works with MSVC (At least MSVC Express 2010 I did not test other). To support it just need wrap include sys/time.h to some ifdef.

[felipedaragon]

@thibaultCha I imagined there was a FFI implementation for openresty. Thanks for pointing it out! I think we will use this at a later stage

[develCuy]

@felipedaragon, @Etiene, I think that luuid will need more love in the future, including the need to patch it and to update the rockspec. Can we add a repository into https://github.com/sailorproject for luuid? I commit to keep it updated. Actually, I want to create releases so that the rockspec uses that repo instead of http://www.tecgraf.puc-rio.br

[felipedaragon]

@develCuy @Etiene I'm all for it!

[Tieske]

Bad idea. Contact lhf to update the sources.

[Tieske]

for a quick fix include a patch in the rockspec. Here's an example; https://github.com/brunoos/luasec/pull/68

[develCuy]

@Tieske, thanks for the patch example! Regarding my proposal, I want to reduce future bureaucracy, is that a bad idea?

[Tieske]

Regarding my proposal, I want to reduce future bureaucracy, is that a bad idea?

Yes, if it results in a more fragmented module landscape. For a quick fix, update the rockspec with the patch, you'll have a working version today :smile:. Long term, fix it at the source, always the better option.

From a previous post;

Can we add a repository into https://github.com/sailorproject for luuid? I commit to keep it updated. Actually, I want to create releases so that the rockspec uses that repo instead of http://www.tecgraf.puc-rio.br

This is incorrect; note that the luarocks server has both the .rockspec file (just build and dependency info) as well as the .rock file, which includes the complete sources. So there is no dependency on http://www.tecgraf.puc-rio.br whatsoever. There is only a dependency on luarock.org, which has several backup servers.

[develCuy]

Yes, if it results in a more fragmented module landscape. For a quick fix, update the rockspec with the patch, you'll have a working version today :smile:. Long term, fix it at the source, always the better option.

So are you offering help long term?

This is incorrect; note that the luarocks server has both the .rockspec file (just build and dependency info) as well as the .rock file, which includes the complete sources. So there is no dependency on http://www.tecgraf.puc-rio.br whatsoever. There is only a dependency on luarock.org, which has several backup servers.

Err... I'm talking about the rockspeck https://luarocks.org/manifests/develcuy/luuid-20120509-2.rockspec Also, relaying on luarocks only is forcing people to use it... I prefer to provide people with freedom to install the library by them selfs, but to let them track releases as well.