Re: [PATCH] MDEV-31872 Deprecate ENCODE()/DECODE()
Kristian Nielsen <knielsen@knielsen-hq.org> writes:
From: Sergei Golubchik <serg@mariadb.org>
@@ -2724,6 +2725,10 @@ bool Item_func_encode::seed() hash_password(rand_nr, key->ptr(), key->length()); sql_crypt.init(rand_nr);
+ push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, ER_WARN_DEPRECATED_SYNTAX, + ER_THD(thd, ER_WARN_DEPRECATED_SYNTAX_NO_REPLACEMENT), + func_name_cstring().str); // since 11.3 +
NAK. As I already explained in the MDEV-31872, deprecating these functions is not appropriate and will hurt users, not help them. Don't do this. You need to see this from the user's point of view. She is a DBA, she has an application with columns encoded with ENCODE(). What do you expect her to do, rewrite the application because you or someone does not like these functions? That's very disrespectful. If you want to give a warning for these functions, then you need to create an SQL mode or some other handle for the DBA to disable the warning and keep their application running. And the warning should explain why it's there, and what should be done to avoid it. And you need to carefully justify how burdening the DBA with this task of tracking down the source of the new warning and the way to remove it, is worth the benefit. And deprecation suggests eventual removal, which is even worse, please don't do that. You haven't even updated the documentation https://mariadb.com/kb/en/encode/ to eg. say "encode" or "scramble" instead of encrypt. "base64_encode" also does not do encryption, nor is rot13 cryptographically safe, but that doesn't justify removing them either. Most columns in mariadb databases are not encrypted or encoded at all, how is ENCODE() any less secure that that? - Kristian.
Kristian Nielsen via developers <developers@lists.mariadb.org> writes:
If you want to give a warning for these functions, then you need to create an SQL mode or some other handle for the DBA to disable the warning and keep their application running. And the warning should explain why it's there,
Ah, we have --old-mode for this purpose. So add a new mode for --old-mode that disables the warning for the ENCODE/DECODE functions. And later that mode could re-enable the functions, if you want to disable them by default after deprecation period. The new mode could be specific to the ENCODE/DECODE functions (COMPAT_ENCODE_DECODE perhaps), or it could be generic for features whose use is discouraged (eg. COMPAT_DISCOURAGED). I'd suggest the latter to simplify the life of DBAs maintaining legacy applications, but I don't have a strong opinion one way or the other, nor for the exact naming. - Kristian.
Hi, Kristian, On Sep 11, Kristian Nielsen wrote:
Kristian Nielsen via developers <developers@lists.mariadb.org> writes:
If you want to give a warning for these functions, then you need to create an SQL mode or some other handle for the DBA to disable the warning and keep their application running. And the warning should explain why it's there,
Ah, we have --old-mode for this purpose.
old mode isn't quite for that. it's to revert the new behavior *temporarily* to give users time to adjust their applications. so by the very definition every old-mode value is deprecated from the day it was introduced. Adding a deprecated option to disable a deprecated warning elsewhere sounds rather strange :)
So add a new mode for --old-mode that disables the warning for the ENCODE/DECODE functions. And later that mode could re-enable the functions, if you want to disable them by default after deprecation period.
technically, yes, we can add an old mode in 2028, when ENCODE/DECODE supposedly would be removed (if nothing will change during that time). An old mode to preserve ENCODE/DECODE for another five years. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi, Kristian, On Sep 11, Kristian Nielsen wrote:
Kristian Nielsen <knielsen@knielsen-hq.org> writes:
From: Sergei Golubchik <serg@mariadb.org>
@@ -2724,6 +2725,10 @@ bool Item_func_encode::seed() hash_password(rand_nr, key->ptr(), key->length()); sql_crypt.init(rand_nr);
+ push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN, ER_WARN_DEPRECATED_SYNTAX, + ER_THD(thd, ER_WARN_DEPRECATED_SYNTAX_NO_REPLACEMENT), + func_name_cstring().str); // since 11.3 +
NAK. As I already explained in the MDEV-31872, deprecating these functions is not appropriate and will hurt users, not help them. Don't do this.
You need to see this from the user's point of view. She is a DBA, she has an application with columns encoded with ENCODE(). What do you expect her to do, rewrite the application because you or someone does not like these functions? That's very disrespectful.
Correct. We warn her that these "encryption" functions are crap, offer a minimal improvement over the Caesar cypher and her data is far less protected than we might've led her to think. And yes, we expect her to rewrite the application before we read on front pages that a site was hacked and credit card numbers were leaked. Or get the bad press (rightfully) when someone was using ENCODE to meet GDPR requirement of encrypting personally identifiable information.
If you want to give a warning for these functions, then you need to create an SQL mode or some other handle for the DBA to disable the warning and keep their application running. And the warning should explain why it's there, and what should be done to avoid it. And you need to carefully justify how burdening the DBA with this task of tracking down the source of the new warning and the way to remove it, is worth the benefit.
This is purely for the benefit of the user. Sometimes old features need to be removed to make the code easier to maintain or extend, but it's hardly the case here. And don't forget that exponential growth means we're getting more new users than we have old ones. And there are vastly more users who do not use ENCODE *yet* than those, who do. The main point here is to reach those who don't use ENCODE and help them not to start using it.
And deprecation suggests eventual removal, which is even worse, please don't do that.
it's not happening anytime soon
You haven't even updated the documentation https://mariadb.com/kb/en/encode/ to eg. say "encode" or "scramble" instead of encrypt. "base64_encode" also does not do encryption, nor is rot13 cryptographically safe, but that doesn't justify removing them either. Most columns in mariadb databases are not encrypted or encoded at all, how is ENCODE() any less secure that that?
You seem to be confused by ENCODE function name. It does *encryption* and the KB page says that. And also says one shouldn't use it, because the encryption is weak. If ENCODE managed to confuse even you, it certainly needs to go. base64_encode and rot13 don't do encryption. Most columns in mariadb databases are not encrypted, and it's fine - the user decides what needs encryption and what doesn't. But if a user decided that someone needs encryption, we shouldn't lie to her and do rot13 instead. Which is pretty much what ENCODE does. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Sergei Golubchik via developers <developers@lists.mariadb.org> writes:
And yes, we expect her to rewrite the application before we read on
I think this is the core of the problem: "we expect her to rewrite the application" No, "we" have no such expectation. And if you do, then you have become too far removed from the users. The purpose of MariaDB is to provide an Open Source tool to users to run their applications that need database storage. This means that _not_ breaking their applications in upgrades should be our primary concern. We cannot always avoid it, mostly due to human error and in some cases due to technical reasons. But we can avoid doing it *deliberately*. What I have seen talking to different users is that regressions caused by upgrades has become a critical concern for them. We need to do everything we can to improve on this. That's the reason I bother to assist with this issue, not because I think ENCODE is something that will benefit new users obviously.
front pages that a site was hacked and credit card numbers were leaked. Or get the bad press (rightfully) when someone was using ENCODE to meet GDPR requirement of encrypting personally identifiable information.
This is a strawman's argument. I doubt you have a documented case where a security breach would have been prevented by removing ENCODE from the server. Much more realistic is the scenario that a site was hacked because they were stuck on an old version of MariaDB that is out of support and had known vulnerabilities. This is the real issue many users are facing now. Do you agree that not breaking user's old applications is an important goal, and that holding users hostage by preventing them from upgrading if they don't rewrite their application according to specific guidelines is not the correct way to develop MariaDB?
old mode isn't quite for that. it's to revert the new behavior *temporarily* to give users time to adjust their applications.
... if so, then please agree to rework this patch so that users will be able to configure the server to continue running their application without spewing warnings or errors. If you don't like my suggestions, then whatever other method you think is more appropriate for this.
Sometimes old features need to be removed to make the code easier to maintain or extend, but it's hardly the case here.
Exactly!
You seem to be confused by ENCODE function name. It does *encryption* and the KB page says that. And also says one shouldn't use it, because the encryption is weak. If ENCODE managed to confuse even you, it certainly needs to go.
I'm not confused by anything, I don't know why you think so. There are legitimate uses for ENCODE(), though cryptographically safe protection of data against access without the password/key is obviously not one of them. - Kristian.
Hi, Kristian, On Sep 11, Kristian Nielsen wrote:
Sergei Golubchik via developers <developers@lists.mariadb.org> writes:
And yes, we expect her to rewrite the application before we read on
I think this is the core of the problem:
"we expect her to rewrite the application"
No, "we" have no such expectation. And if you do, then you have become too far removed from the users.
No-no, I have no such expectations either, at least not when I'm replying to such a partial quote taken completely out of context :)
The purpose of MariaDB is to provide an Open Source tool to users to run their applications that need database storage. This means that _not_ breaking their applications in upgrades should be our primary concern. We cannot always avoid it, mostly due to human error and in some cases due to technical reasons. But we can avoid doing it *deliberately*.
What I have seen talking to different users is that regressions caused by upgrades has become a critical concern for them. We need to do everything we can to improve on this. That's the reason I bother to assist with this issue, not because I think ENCODE is something that will benefit new users obviously.
Of course, one can see any bug fix as something that breaks applications. And one can argue that providing a weak encryption function is a bug on itself. But we don't do sudden feature removal at random. ENCODE will not go away anytime soon, not before 2028. And then we can add an old mode for it, giving it five more years, if needed. And then again. It is of course possible that someone will use an application in 2033 that was written before 2023 and never updated afterwards. This might be a reason for us to keep ENCODE longer. But I definitely don't want any *new* application, written after 2023, to use ENCODE/DECODE. This is what the warning is for.
front pages that a site was hacked and credit card numbers were leaked. Or get the bad press (rightfully) when someone was using ENCODE to meet GDPR requirement of encrypting personally identifiable information.
This is a strawman's argument. I doubt you have a documented case where a security breach would have been prevented by removing ENCODE from the server.
No, I don't :) And I don't have a documented use case where MariaDB SSL-ed connection was intercepted and data was modified, but despite this all guidelines everywhere on the internel, both on MariaDB and on MySQL, tell that for a connection to be secure the client has to validate server certificate.
Do you agree that not breaking user's old applications is an important goal, and that holding users hostage by preventing them from upgrading if they don't rewrite their application according to specific guidelines is not the correct way to develop MariaDB?
Yes, of course, I agree. That's why we provide a long deprecation period and don't remove features at random. This is a security issue, much like deprecating DES in favor of AES.
You seem to be confused by ENCODE function name. It does *encryption* and the KB page says that. And also says one shouldn't use it, because the encryption is weak. If ENCODE managed to confuse even you, it certainly needs to go.
I'm not confused by anything, I don't know why you think so.
Because you keep saying that ENCODE function "encodes" something and compare it with base64 and rot13. While it is an encryption function, that encrypts given plain-text data with given encryption key. It is unfortunate that it was called "ENCODE", but it happened 25 years ago.
There are legitimate uses for ENCODE(), though cryptographically safe protection of data against access without the password/key is obviously not one of them.
What would be a legitimate use for a weak cryptographic function? Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Sergei Golubchik via developers <developers@lists.mariadb.org> writes:
But I definitely don't want any *new* application, written after 2023, to use ENCODE/DECODE. This is what the warning is for.
This is of course agreed. https://lists.mariadb.org/hyperkitty/list/commits@lists.mariadb.org/thread/U... https://github.com/MariaDB/server/commit/83a1b921812c98b03f7af40a61722c50800... This is what I'm asking for. A way for me to disable the gazillions of SHOW WARNINGS that appear in my logs after I upgraded to 11.3. I have a legacy application with columns inserted using ENCODE(...), and I don't have the resources to do a full data migration to SHA2 or plaintext. (Not "me" personally but the future user I'm trying to help). And a note in the code to help prevent that I someday upgrade to 13.7 or whatever and find that ENCODE() is gone and I now need to reverse-engineer old MariaDB sources to re-implement DECODE() to rescue my data, as MariaDB downgrades between major versions is not supported. Agreed?
There are legitimate uses for ENCODE(), though cryptographically safe protection of data against access without the password/key is obviously not one of them.
What would be a legitimate use for a weak cryptographic function?
I'll explain, but please don't get side-tracked by it. This is not relevant to the issue at hand, which is to help prevent regression during upgrades. I'm in no way trying to suggest using ENCODE() for anything where it can possibly be avoided. The only legitimate use for ENCODE()/DECODE() *now* is to access existing data in legacy MariaDB databases. This is however a very legitimate use, since IIUC it is the only way to do so (other than reverse-engineering the MariaDB implementation). An obvious use for a non-cryptographically strong encryption/scrambling/encoding or whatever you want to call it, is to protect user data from casual glances from developers when they are debugging the application. As an application developer in real life, I'll often find myself looking at binlogs or table data or whatever to debug problems. It's important to be able to do so without accidentally viewing user private messages etc. "rot13" is perhaps a bit too weak for this, but mostly anything that's not immediately recognisable by human eyes should work. It is easy to over-estimate the value of "strong" encryption/decryption of database columns inside an application. Where to keep the key is a serious problem. Unless the encryption is end-to-end (in which case no SQL functions are involved), the key will need to be stored somewhere. Usually hard-coded somewhere in the application code or configuration files. And SHA2() is little better than rot13 if the key is sitting next to the ciphertext. (Here, "rot13" is the Caecar cipher using "13" as the key). That's why passwords are best stored using a one-way hash function, not encrypted. - Kristian.
Here is a case in point about the problem with unnecessary deprecation warnings. It's not just me ;-): https://mariadb.com/kb/en/suppress-mariadb-11-deprecation-warnings/ Is there a way to prevent deprecation warnings from appearing when executing the 'mysql' and 'mysqldump' commands? There are applications, such as Zabbix, that expect an XML when executing these commands, and this warning breaks the format. I mean messages like this: "mariadb mysql: Deprecated program namein a future release, use '/usr/bin/mariadb' instead" Why remove backwards compatibility with binary names? There's no technical necessity to do so, and it breaks things for the users. - Kristian.
Hi, Kristian, This was done to improve coexistence with MySQL. As MariaDB drifts farther from MySQL, there might be more and more users who could want to install them side by side, so it's important to reduce conflicts on file names. On Sep 26, Kristian Nielsen via developers wrote:
Here is a case in point about the problem with unnecessary deprecation warnings. It's not just me ;-):
https://mariadb.com/kb/en/suppress-mariadb-11-deprecation-warnings/
Is there a way to prevent deprecation warnings from appearing when executing the 'mysql' and 'mysqldump' commands? There are applications, such as Zabbix, that expect an XML when executing these commands, and this warning breaks the format.
I mean messages like this: "mariadb mysql: Deprecated program namein a future release, use '/usr/bin/mariadb' instead"
Why remove backwards compatibility with binary names? There's no technical necessity to do so, and it breaks things for the users.
Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org
Hi Sergei, On Tue, Sep 26, 2023 at 3:24 PM Sergei Golubchik via developers <developers@lists.mariadb.org> wrote:
Hi, Kristian,
This was done to improve coexistence with MySQL. As MariaDB drifts farther from MySQL, there might be more and more users who could want to install them side by side, so it's important to reduce conflicts on file names.
Would it be thinkable for the mariadb packages to recommend or suggest a separate compatibility package that would install symbolic links to provide MySQL compatible program names? In that way, no deprecation messages would seem to be necessary at runtime. Marko -- Marko Mäkelä, Lead Developer InnoDB MariaDB plc
Hi, Marko, On Sep 26, Marko Mäkelä wrote:
On Tue, Sep 26, 2023 at 3:24 PM Sergei Golubchik via developers wrote:
This was done to improve coexistence with MySQL. As MariaDB drifts farther from MySQL, there might be more and more users who could want to install them side by side, so it's important to reduce conflicts on file names.
Would it be thinkable for the mariadb packages to recommend or suggest a separate compatibility package that would install symbolic links to provide MySQL compatible program names? In that way, no deprecation messages would seem to be necessary at runtime.
I think we already have that, separate recommended compatibility package. The plan was to make it suggested eventually, as some undefined point in time in the future. But I don't remember it was discussed in relation to whether it makes the warning unnecessary. Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org
participants (3)
-
Kristian Nielsen -
Marko Mäkelä -
Sergei Golubchik