See bug 16591 latest comments. Patches still concept.
Created attachment 53418 [details] [review] Bug 16929: Wrapper around CSRF call Just in concept
Created attachment 53487 [details] [review] Bug 16929: Prevent opac-memberentry waiting for random chars Move calls to WWW::CSRF to Koha::Token. Send a safe random string to WWW::CSRF instead of letting CSRF make a blocking call to Bytes::Random::Secure. If your server has not enough entropy, opac-memberentry will hang waiting for more characters in dev/random. Koha::Token uses Bytes::Random::Secure with the NonBlocking flag. Test plan: [1] Do not yet apply this patch. [2] If your server has not enough entropy, calling opac-memberentry may take a while. But this not may be the case for you (no worries). [3] Apply this patch. [4] Verify that opac-memberentry still works as expected. [5] Run t/Token.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Yes, my server had entropy trouble (reason for finding the problem). This patch resolves the delay.
Latest patch always uses Bytes::Random::Secure and does not eval/alarm as in the concept. Includes a test.
I'd prefer to see a generate_csrf method than a CSRF flag. It'd be better to use instead of require the 2 modules.
(In reply to Jonathan Druart from comment #5) > I'd prefer to see a generate_csrf method than a CSRF flag. > It'd be better to use instead of require the 2 modules. Well, we could add generate_csrf of course. The flag on itself is kind of a shortcut for type => $type. The require is just performance: no need to load it for general use.
(In reply to Marcel de Rooy from comment #6) > The require is just performance: no need to load it for general use. It's not recommended. For instance if the module is missing of buggy, we want to know it at the compilation step.
Created attachment 53552 [details] [review] Bug 16929: [QA Follow-up] Shortcut methods and use statements Resolves the following comments: I'd prefer to see a generate_csrf method than a CSRF flag. It'd be better to use instead of require the 2 modules. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Bytes::Random::Secure should be added to PerlDependencies.pm. libbytes-random-secure-perl is not available for Debian Wheezy.
(In reply to Mirko Tietgen from comment #9) > Bytes::Random::Secure should be added to PerlDependencies.pm. > > libbytes-random-secure-perl is not available for Debian Wheezy. It actually came with WWW::CSRF. It loads Bytes::Random::Secure. Is there a package for that one? Will add a follow-up for B::R::S
Created attachment 53809 [details] [review] Bug 16929: [QA Follow-up] Add dependency for Bytes::Random::Secure Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
> It actually came with WWW::CSRF. It loads Bytes::Random::Secure. Is there a > package for that one? If it is used directly, I think it should be in PerlDependencies. There is no package for WWW:CSRF in Wheezy. I created packages for it and dependencies for the unstable branch for bug 16591. https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16591#c7 I did not get any feedback regarding Wheezy so far and did not have time to try it myself, so I have not added them to oldstable yet. > Will add a follow-up for B::R::S Thanks!
libbytes-random-secure-perl is in the Koha stable and olstable repositories now.
Created attachment 54140 [details] [review] Bug 16929: Prevent opac-memberentry waiting for random chars Move calls to WWW::CSRF to Koha::Token. Send a safe random string to WWW::CSRF instead of letting CSRF make a blocking call to Bytes::Random::Secure. If your server has not enough entropy, opac-memberentry will hang waiting for more characters in dev/random. Koha::Token uses Bytes::Random::Secure with the NonBlocking flag. Test plan: [1] Do not yet apply this patch. [2] If your server has not enough entropy, calling opac-memberentry may take a while. But this not may be the case for you (no worries). [3] Apply this patch. [4] Verify that opac-memberentry still works as expected. [5] Run t/Token.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Yes, my server had entropy trouble (reason for finding the problem). This patch resolves the delay.
Created attachment 54141 [details] [review] Bug 16929: [QA Follow-up] Shortcut methods and use statements Resolves the following comments: I'd prefer to see a generate_csrf method than a CSRF flag. It'd be better to use instead of require the 2 modules. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 54142 [details] [review] Bug 16929: [QA Follow-up] Add dependency for Bytes::Random::Secure Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Rebased for opac-memberentry and Dependencies.pm
Created attachment 54143 [details] [review] Bug 16929: Prevent opac-memberentry waiting for random chars Move calls to WWW::CSRF to Koha::Token. Send a safe random string to WWW::CSRF instead of letting CSRF make a blocking call to Bytes::Random::Secure. If your server has not enough entropy, opac-memberentry will hang waiting for more characters in dev/random. Koha::Token uses Bytes::Random::Secure with the NonBlocking flag. Test plan: [1] Do not yet apply this patch. [2] If your server has not enough entropy, calling opac-memberentry may take a while. But this not may be the case for you (no worries). [3] Apply this patch. [4] Verify that opac-memberentry still works as expected. [5] Run t/Token.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Yes, my server had entropy trouble (reason for finding the problem). This patch resolves the delay. Tested all 3 patches together, works as expected. Signed-off-by: Marc <veron@veron.ch>
Created attachment 54144 [details] [review] Bug 16929: [QA Follow-up] Shortcut methods and use statements Resolves the following comments: I'd prefer to see a generate_csrf method than a CSRF flag. It'd be better to use instead of require the 2 modules. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc <veron@veron.ch>
Created attachment 54145 [details] [review] Bug 16929: [QA Follow-up] Add dependency for Bytes::Random::Secure Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc <veron@veron.ch>
Marcel, why did you choose 0.29? 0.28 is packaged for jessie. I also think that we need to fill debian/control for new deps. Mirko, could you confirm that you expect from devs to update it?
(In reply to Jonathan Druart from comment #21) > Marcel, why did you choose 0.29? 0.28 is packaged for jessie. > > I also think that we need to fill debian/control for new deps. > Mirko, could you confirm that you expect from devs to update it? I would say that the PM edits debian/control, not individual devs ..
Created attachment 54151 [details] [review] Bug 16929: [QA Follow-up] Add dependency for Bytes::Random::Secure Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc <veron@veron.ch>
(In reply to Jonathan Druart from comment #21) > Marcel, why did you choose 0.29? 0.28 is packaged for jessie. > > I also think that we need to fill debian/control for new deps. > Mirko, could you confirm that you expect from devs to update it? In theory, yes. but since the file is supposed to be generated, not edited manually, and it will clash with other bugs with dependencies, I think it's easier if I do it once something got pushed. I'm working on automating that.
Created attachment 54152 [details] [review] Bug 16929: Prevent opac-memberentry waiting for random chars Move calls to WWW::CSRF to Koha::Token. Send a safe random string to WWW::CSRF instead of letting CSRF make a blocking call to Bytes::Random::Secure. If your server has not enough entropy, opac-memberentry will hang waiting for more characters in dev/random. Koha::Token uses Bytes::Random::Secure with the NonBlocking flag. Test plan: [1] Do not yet apply this patch. [2] If your server has not enough entropy, calling opac-memberentry may take a while. But this not may be the case for you (no worries). [3] Apply this patch. [4] Verify that opac-memberentry still works as expected. [5] Run t/Token.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Yes, my server had entropy trouble (reason for finding the problem). This patch resolves the delay. Tested all 3 patches together, works as expected. Signed-off-by: Marc <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54153 [details] [review] Bug 16929: [QA Follow-up] Shortcut methods and use statements Resolves the following comments: I'd prefer to see a generate_csrf method than a CSRF flag. It'd be better to use instead of require the 2 modules. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54154 [details] [review] Bug 16929: [QA Follow-up] Add dependency for Bytes::Random::Secure Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54216 [details] [review] Bug 16929: [QA Follow-up] Add dependency for Bytes::Random::Secure Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Marc <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Rebased last patch for Dependencies.pm (trivial)
Pushed to master for 16.11, thanks Marcel!
The corresponding control file update is at bug 17084: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17084#c1
Pushed to 16.05.x, for 16.05.09 release