search

OWASP / phpsec

OWASP PHP Security Project - THIS PROJECT IS INACTIVE AND MAY CONTAIN SECURITY FLAWS
197 stars 103 forks source link

code to delete expired session entries from database #81

Closed mebjas closed 10 years ago

mebjas commented 10 years ago

as mentioned by Abbas the code checks for last time expired session entries were deleted and if its greater than a threshold time which I set as 60 minutes, and if it exceeds a single SQL query deletes all those entries in db which has expired.

mebjas commented 10 years ago

@paulocmguerreiro have a look now, I have implemented code to delete data from session_data as well while deleting expired session entries

paulocmguerreiro commented 10 years ago

@mebjas , it's perfect

Although, my opinion, you should call it from a LogOut Session instead of LogIn. At logout time the user can wait a little longer (when this script runs) without any hassle. I normally get more stressed when the login gets delayed that at the logout :)

Question: Some of this problems could be solved with TRIGGERS, CONSTRANTS (as @rash805115 has in his RNJ eCommerce Project) OR SCHEDULLED EVENTS, should they be implemented in OWASP.sql!? What do you guys think?

SvenRtbg commented 10 years ago

Does SQLite support all these database things? Or PostgreSQL in a compatible manner? I know the current tests only use MySQL, but I think using fancy DB stuff can pose an obstacle. I'd try to avoid it if possible.

Paulo Guerreiro notifications@github.com schrieb:

@mebjas , it's perfect

Although, my opinion, you should call it from a LogOut Session instead of LogIn. At logout time the user can wait a little longer (when this script runs) without any hassle. I normally get more stressed when the login gets delayed that at the logout :)

Question: Some of this problems could be solved with TRIGGERS, CONSTRANTS (as @rash805115 has in his RNJ eCommerce Project) OR SCHEDULLED EVENTS, should they be implemented in OWASP.sql!? What do you guys think?

Reply to this email directly or view it on GitHub: https://github.com/OWASP/phpsec/pull/81#issuecomment-27503799

Mit freundlichen Grüßen

Sven Rautenberg

abiusx commented 10 years ago

+1 Sven, Only use basic standard SQL functionality (CRUD) inside the app. Triggers are not only non portable, they are insanely hard to debug and track. -A

Notice: This message is digitally signed, its source and integrity are verifiable. If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Oct 31, 2013, at 1:08 PM, SvenRtbg notifications@github.com wrote:

Does SQLite support all these database things? Or PostgreSQL in a compatible manner? I know the current tests only use MySQL, but I think using fancy DB stuff can pose an obstacle. I'd try to avoid it if possible.

Paulo Guerreiro notifications@github.com schrieb:

@mebjas , it's perfect

Although, my opinion, you should call it from a LogOut Session instead of LogIn. At logout time the user can wait a little longer (when this script runs) without any hassle. I normally get more stressed when the login gets delayed that at the logout :)

Question: Some of this problems could be solved with TRIGGERS, CONSTRANTS (as @rash805115 has in his RNJ eCommerce Project) OR SCHEDULLED EVENTS, should they be implemented in OWASP.sql!? What do you guys think?

Reply to this email directly or view it on GitHub: https://github.com/OWASP/phpsec/pull/81#issuecomment-27503799

Mit freundlichen Grüßen

Sven Rautenberg — Reply to this email directly or view it on GitHub.

paulocmguerreiro commented 10 years ago

@SvenRtbg you have a good argument there. :)

mebjas commented 10 years ago

@paulocmguerreiro but users don't log out usually! what if user closes the browser without actually logging out? on the other hand if this functionality is called during creation of new session, it would be called for few users only and the query wont take a very considerable amount of time as expired sessions are being deleted time to time.

abiusx commented 10 years ago

don’t worry much about the performance, usually session tables are stored on memory, and also deleting records off a table is the fastest operation. -A

Notice: This message is digitally signed, its source and integrity are verifiable. If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Oct 31, 2013, at 4:37 PM, minhaz notifications@github.com wrote:

@paulocmguerreiro but users don't log out usually! what if user closes the browser without actually logging out? on the other hand if this functionality is called during creation of new session, it would be called for few users only and the query wont take a very considerable amount of time as expired sessions are being deleted time to time.

— Reply to this email directly or view it on GitHub.

mebjas commented 10 years ago

@abiusx I think you are with deleting expired session while newSession() function is called? Also I could not understand the probabilistic approach as mentioned by @rash805115

abiusx commented 10 years ago

I haven’t done the code :D I’m not deleting them.

Notice: This message is digitally signed, its source and integrity are verifiable. If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Oct 31, 2013, at 4:54 PM, minhaz notifications@github.com wrote:

@abiusx I think you are with deleting expired session while newSession() function is called? Also I could not understand the probabilistic approach as mentioned by @rash805115

— Reply to this email directly or view it on GitHub.

rash805115 commented 10 years ago

Couple of things as a summary in the above commits: 1) Remove all the join and other queries from the code...they are incompatible. 2) The travis built is failing..check that before I can pull

Now in this method, you created a new table which will at all time will contain only one row...though not an issue...it is something that looks ugly.. With the probability function (see jframework's session library) you do not have to maintain the table...

Have a look on the requirement again...what we want is to sweep the database from time to time. It would not hurt if the database is not sweeped even in 1 month..its just containing some extra data...the thing you proposed will sweep every 60 min...or whatever time you keep. But with the probability approach, the sweep will be done in "non-deterministic" time.. here is the code for reference:

function _Sweep($force=false) { //Removes timed out session if (!$force) if (rand ( 0, 1000 ) / 1000.0 > self::$SweepRatio) return; //10% $Now = jf::time (); jf::SQL ( "DELETE FROM {$this->TablePrefix()}session WHERE LastAccess<? OR LoginDate<?", $Now- self::$NoAccessTimeout, $Now- self::$ForcedTimeout); }

If we put this code in creation of new session, we get what we desire.

On Thu, Oct 31, 2013 at 7:47 PM, AbiusX notifications@github.com wrote:

I haven’t done the code :D I’m not deleting them.

Notice: This message is digitally signed, its source and integrity are verifiable. If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Oct 31, 2013, at 4:54 PM, minhaz notifications@github.com wrote:

@abiusx I think you are with deleting expired session while newSession() function is called? Also I could not understand the probabilistic approach as mentioned by @rash805115

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/OWASP/phpsec/pull/81#issuecomment-27538598 .

Regards, Rahul Chaudhary Ph - 412-519-9634

mebjas commented 10 years ago

Can anyone help me with The Travis CI.... I don't get why its failing?

mebjas commented 10 years ago

The Travis CI build passed ! and now I get what it means ;)

abiusx commented 10 years ago

Create a new issue for this

Notice: This message is digitally signed, its source and integrity are verifiable. If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Dec 11, 2013, at 11:14 AM, minhaz notifications@github.com wrote:

Owasp cheat sheet for session management says we should bind session to ip address to make it more secure. Why didnt we go for this in PHPSEC.?

— Reply to this email directly or view it on GitHub.