https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134298|0 |1 is obsolete| | --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 134300 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134300&action=edit Bug 30649 - Add ability to encrypt data in Koha, use for EDI vendor accounts Right now, we store passwords for external services in plain text in the database. It would be good to have the option to encrypt those passwords. Test Plan: 1) prove t/Crypt.t 2) Create a new vendor edi account 3) Note the password is encrypted 4) Update the password for an existing EDI vendor account 5) Note the password is now encrypted in the database 6) Stretch goal, set up an FTP server, send an EDI order to the FTP server where the vendor password is encrypted. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134301|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 134302 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134302&action=edit Bug 30649 - Add ability to encrypt data in Koha, use for EDI vendor accounts Right now, we store passwords for external services in plain text in the database. It would be good to have the option to encrypt those passwords. Test Plan: 1) prove t/Crypt.t 2) Create a new vendor edi account 3) Note the password is encrypted 4) Update the password for an existing EDI vendor account 5) Note the password is now encrypted in the database 6) Stretch goal, set up an FTP server, send an EDI order to the FTP server where the vendor password is encrypted. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28998 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Needs Signoff |BLOCKED -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com Depends on| |28998 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28998 [Bug 28998] Encrypt borrowers.secret -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Victor Grousset/tuxayo from comment #8)

I don't get how to encrypt a password to an external service and still be able to use the external service. Does that mean Koha can in full autonomy decrypt it?

Yes, we store a key in the koha konf file for encryption and decryption. I need to rebase this patch to use the work from Bug 28998. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Vendor EDI account |Vendor EDI account |passwords should be |passwords should be |encrypted |encrypted in the database -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134302|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 142056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142056&action=edit Bug 30649: Vendor EDI account passwords should be encrypted in the database We are storing edi vendor acccount passwords in clear text in the database. Now that Koha has the Koha::Encryption module, we should use that to encrypt passwords for all existing and new EDI accounts. Test Plan: 1) Apply this patch 2) Create one or more EDI vendor accounts 3) Run a report to view the account passwords, note they are in clear text 4) Run updatedatabase.pl 5) Re-run the report, account passwords should be encrypted now 6) Edit a vendor EDI account, note you can still view and update the password for an account -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142057|0 |1 is obsolete| | --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 142820 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142820&action=edit Bug 30649: Vendor EDI account passwords should be encrypted in the database We are storing edi vendor acccount passwords in clear text in the database. Now that Koha has the Koha::Encryption module, we should use that to encrypt passwords for all existing and new EDI accounts. Test Plan: 1) Apply this patch 2) Create one or more EDI vendor accounts 3) Run a report to view the account passwords, note they are in clear text 4) Run updatedatabase.pl 5) Re-run the report, account passwords should be encrypted now 6) Edit a vendor EDI account, note you can still view and update the password for an account Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #14 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Kyle M Hall from comment #9)

(In reply to Victor Grousset/tuxayo from comment #8)

...

I don't get how to encrypt a password to an external service and still be able to use the external service. Does that mean Koha can in full autonomy decrypt it?

Yes, we store a key in the koha konf file for encryption and decryption. I need to rebase this patch to use the work from Bug 28998.

Ok IIUC the security value doesn't come from encryption but from having the date out of the DB. So a simple SQL injection can't get it. Is there any gain compared to just storing the passwords into koha-conf.xml directly? (hum, maybe Koha can't write to that file and that would need a separate file) Like is it a plausible attack scenario to be able to read the file but not the DB? That when needing both would help. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Still contemplating QA here.. the code works as expected and I'm happy with the implementation as a whole. However.. I'm not so sure about the in place database upgrade... we tend to try and steer away from referencing Koha modules from within the atomicupdates in case there's a change to said module down the line. That said.. that's not a blocker for me, just a consideration. When we upgraded from SHA to BCrypt for user account hashing we added a layer inside the codebase to upgrade the hash on first access I seem to recall. My other pondering is around what happens if/when an admin wants to change the encryption key for the server.. that's out of scope for this particular bug, but I feel like we should have an option for it somewhere.. either a script to update encrypted data's to use the new key (given the old and new key as input) or a way to defined the keys as an array and upgrade on access or something like that. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK.. I decided to open another bug for my thoughts on key change.. I'll pass this one but highlight to the RM that we may need to rethink the DB update. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #19 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #17)

When we upgraded from SHA to BCrypt for user account hashing we added a layer inside the codebase to upgrade the hash on first access I seem to recall.

We were upgrading from MD5 hashes to BCrypt hashes, which were easy to differentiate, since the BCrypt hashes started with "$2a$08$". We also had the user input so you could always compare hashes. In this case with the decryption I don't think there's any way to know whether or not you got a valid decrypted value (unless the encryption module throws an exception)... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #21 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Martin Renvoize from comment #16)

The value does come from the encryption. If the database is somehow compromised (for example, someone accidentally shares a backup.. it could be as simple as that).. by having the data in the databawse encrypted the nafarious actor doesn't have something useful to them.. They still need to hack the machine to get ahold of the key (from the conf file) and/or read the code to understand what sort of algorithm is used.

That's why I wondered if there was any gain compared to just storing the passwords into koha-conf.xml directly? (or another file) The question would have been more relevant on bug 28998 now that such a mechanism is implemented, the work is done and it's not very hard to use on any data to be protected from SQL injection or accidental backup publication. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It might also hinder a a quick desaster recovery to a different server? At least something more to think about for backups etc. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to David Cook from comment #24)

(In reply to Katrin Fischer from comment #23)

...

It might also hinder a a quick desaster recovery to a different server? At least something more to think about for backups etc.

With the encryption key in koha-conf.xml, they wouldn't be able to decrypt the encrypted passwords in the database either.

True, so something to keep in mind then for the sysadmins? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|21.05 |master -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #28 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I don't know how this is relevant, but borrowers.secret is MEDIUMTEXT and you are using VARCHAR(256) here. By the way, 256? Typo for 255? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144323|0 |1 is obsolete| | --- Comment #30 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 144324 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144324&action=edit Bug 30649: Vendor EDI account passwords should be encrypted in the database We are storing edi vendor acccount passwords in clear text in the database. Now that Koha has the Koha::Encryption module, we should use that to encrypt passwords for all existing and new EDI accounts. Test Plan: 1) Apply this patch 2) Create one or more EDI vendor accounts 3) Run a report to view the account passwords, note they are in clear text 4) Run updatedatabase.pl 5) Re-run the report, account passwords should be encrypted now 6) Edit a vendor EDI account, note you can still view and update the password for an account Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #32 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #28)

I don't know how this is relevant, but borrowers.secret is MEDIUMTEXT and you are using VARCHAR(256) here.

By the way, 256? Typo for 255?

I like the idea of using mediumtext much better for encrypted data fields than varchar. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00 |23.05.00,22.11.07 released in| | Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31059 --- Comment #35 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Reported on the ML, it's breaking the upgrade ERROR - Exception 'Koha::Exceptions::MissingParameter' thrown 'No encryption_key in koha-conf.xml' We should not use Koha module in db revs. Here we have Koha::Database that can be replaced easily with $dbh, however there is no good solution for Koha::Encryption. We could maybe require the module only if there are rows in vendor_edi_accounts? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=33934 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #39 from David Cook <dcook@prosentient.com.au> --- Would this work without the encryption_key being set though? Might need to add a catch for that -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #41 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Follow-up pushed to master. Backporting to 23.05 as well. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emmi.takkinen@koha-suomi.fi --- Comment #42 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- There's a typo in atomicupdate file. "SELECT * FROM vendor_edit_accounts" should be "SELECT * FROM vendor_edi_accounts" -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 --- Comment #44 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Oh and also SET password = $password needs guotation marks around $password :D Ran into this while updating my test database. After fixing them manually update proceeded without problems. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00,22.11.07 |23.05.00,22.11.08,22.11.07 released in| | Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30649 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.