Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-19341

Make reloadable TLS system variables dynamic

Details

    Description

      With MDEV-16266 implemented in 10.4, it seems like we have an opportunity to make certain SSL/TLS system variables dynamic.

      It looks like FLUSH SSL uses the following variables to reload the TLS context:

      • ssl_cert
      • ssl_key
      • ssl_ca
      • ssl_capath
      • ssl_crl
      • ssl_crlpath
      • ssl_cipher

      https://github.com/MariaDB/server/blob/mariadb-10.4.4/sql/mysqld.cc#L4735

      https://github.com/MariaDB/server/blob/mariadb-10.4.4//vio/viosslfactories.c#L334

      Can all of these variables be made dynamic in 10.4, as long as the system supports FLUSH SSL?

      Attachments

        Issue Links

          Activity

            I'm not sure it is a very good idea.

            I think that the user expectation is that setting a variable has immediate effect - in the given case the SSL context would be reloaded, as result of SET GLOBAL (rather than delayed until FLUSH SSL).

            However, this will not really work for all ssl variables in all cases, some of the parameters are dependent on each other, for example ssl_key cannot be changed without also changing ssl_cert.

            wlad Vladislav Vaintroub added a comment - I'm not sure it is a very good idea. I think that the user expectation is that setting a variable has immediate effect - in the given case the SSL context would be reloaded, as result of SET GLOBAL (rather than delayed until FLUSH SSL). However, this will not really work for all ssl variables in all cases, some of the parameters are dependent on each other, for example ssl_key cannot be changed without also changing ssl_cert.

            Hi wlad,

            I think that the user expectation is that setting a variable has immediate effect - in the given case the SSL context would be reloaded, as result of SET GLOBAL (rather than delayed until FLUSH SSL).

            It would be a bit counter-intuitive for users. However, it's not unprecedented to delay the effect of a changed dynamic variable until some other command is executed.

            For example, if you change one of the dynamic replication filters (replicate_do_db, replicate_do_table, etc.), then the change won't go into effect until the slave threads are restarted.

            https://mariadb.com/kb/en/library/replication-filters/#replicate_do_db

            If you set the global variable to enable semi-synchronous replication on a slave, then that change also doesn't go into effect until the slave's I/O thread is restarted.

            https://mariadb.com/kb/en/library/semisynchronous-replication/#enabling-semisynchronous-replication-on-the-slave

            There might be other variables that are similar, but these are the first that I thought of. In my opinion, I think it might work alright, as long as the behavior is documented well enough.

            However, this will not really work for all ssl variables in all cases, some of the parameters are dependent on each other, for example ssl_key cannot be changed without also changing ssl_cert.

            Would it be possible to validate that the files defined by ssl_key and ssl_cert do actually match before calling new_VioSSLAcceptorFd() in reinit_ssl(), so that the server knows that it isn't reloading the TLS context with mismatched files?

            GeoffMontee Geoff Montee (Inactive) added a comment - Hi wlad , I think that the user expectation is that setting a variable has immediate effect - in the given case the SSL context would be reloaded, as result of SET GLOBAL (rather than delayed until FLUSH SSL). It would be a bit counter-intuitive for users. However, it's not unprecedented to delay the effect of a changed dynamic variable until some other command is executed. For example, if you change one of the dynamic replication filters (replicate_do_db, replicate_do_table, etc.), then the change won't go into effect until the slave threads are restarted. https://mariadb.com/kb/en/library/replication-filters/#replicate_do_db If you set the global variable to enable semi-synchronous replication on a slave, then that change also doesn't go into effect until the slave's I/O thread is restarted. https://mariadb.com/kb/en/library/semisynchronous-replication/#enabling-semisynchronous-replication-on-the-slave There might be other variables that are similar, but these are the first that I thought of. In my opinion, I think it might work alright, as long as the behavior is documented well enough. However, this will not really work for all ssl variables in all cases, some of the parameters are dependent on each other, for example ssl_key cannot be changed without also changing ssl_cert. Would it be possible to validate that the files defined by ssl_key and ssl_cert do actually match before calling new_VioSSLAcceptorFd() in reinit_ssl(), so that the server knows that it isn't reloading the TLS context with mismatched files?
            wlad Vladislav Vaintroub added a comment - - edited

            Maybe some variables have delayed effect, but not for FLUSH so far.
            neither location of des_key_file (closest thing to FLUSH SSL), nor location of error log, general log etc would change after call to FLUSH.

            Anyway, I do not see a big problem with how it is implemented right now, or a need for a change, given that it is now also well documented in the KB. The might be a confusion that variables are not dynamic (which they never were). But if variables are dynamic, there will be a confusion that changing them has no immediate effect.
            serg, do you have an opinion on it?

            Would it be possible to validate that the files defined by ssl_key and ssl_cert do actually match before calling new_VioSSLAcceptorFd() in reinit_ssl(), so that the server knows that it isn't reloading the TLS context with mismatched files?

            I think the best validation of parameters is a call to new_VioSSLAcceptorFd()

            wlad Vladislav Vaintroub added a comment - - edited Maybe some variables have delayed effect, but not for FLUSH so far. neither location of des_key_file (closest thing to FLUSH SSL), nor location of error log, general log etc would change after call to FLUSH. Anyway, I do not see a big problem with how it is implemented right now, or a need for a change, given that it is now also well documented in the KB. The might be a confusion that variables are not dynamic (which they never were). But if variables are dynamic, there will be a confusion that changing them has no immediate effect. serg , do you have an opinion on it? Would it be possible to validate that the files defined by ssl_key and ssl_cert do actually match before calling new_VioSSLAcceptorFd() in reinit_ssl(), so that the server knows that it isn't reloading the TLS context with mismatched files? I think the best validation of parameters is a call to new_VioSSLAcceptorFd()

            I tend to agree that it wouldn't be a particularly useful feature. Few more considerations, in addition to what was already said:

            • it would create that very confusing situation where you can modify ssl variables, you can see their values, but you can never know what values are actually used. Selecting the variable you'll see the last written value — but you won't be able to tell if the server uses it or not.
            • it crosses privilege boundaries. There is a person that has access to the file system and can modify files. There's a person, not necessarily the same as the first one, who can edit database settings, having SUPER privilege. SSL settings always used to be the responsibility of the first person. Competely. Now you suggest to allows the second one to overwrite SSL configuration. I don't see why it might be needed — if one can create SSL certificate files (person 1), one can certainly replace them too. If one cannot create them (person 2), changing ssl variables won't help anyway. It'll only create security complications, with basically no practical benefits.
            serg Sergei Golubchik added a comment - I tend to agree that it wouldn't be a particularly useful feature. Few more considerations, in addition to what was already said: it would create that very confusing situation where you can modify ssl variables, you can see their values, but you can never know what values are actually used. Selecting the variable you'll see the last written value — but you won't be able to tell if the server uses it or not. it crosses privilege boundaries. There is a person that has access to the file system and can modify files. There's a person, not necessarily the same as the first one, who can edit database settings, having SUPER privilege. SSL settings always used to be the responsibility of the first person. Competely. Now you suggest to allows the second one to overwrite SSL configuration. I don't see why it might be needed — if one can create SSL certificate files (person 1), one can certainly replace them too. If one cannot create them (person 2), changing ssl variables won't help anyway. It'll only create security complications, with basically no practical benefits.

            Based on feedback, it does not seem to be needed

            wlad Vladislav Vaintroub added a comment - Based on feedback, it does not seem to be needed
            nunop Nuno added a comment -

            wlad serg GeoffMontee This is really cool! Thank you for implementing this.

            If the variables aren't touched (same file paths), but we do renew the certificates (e.g. Certbot), will FLUSH SSL actually get the new files, even though the values of the variables didn't change?

            This is what I understand from the documentation:

            https://mariadb.com/kb/en/flush/#flush-ssl
            "If you want to dynamically reinitialize the server's TLS context, then you need to change the certificate and key files at the relevant paths defined by these TLS system variables, without actually changing the values of the variables."

            But I just want to clarify that my understanding is correct.

            Thank you very much!

            nunop Nuno added a comment - wlad serg GeoffMontee This is really cool! Thank you for implementing this. If the variables aren't touched (same file paths), but we do renew the certificates (e.g. Certbot), will FLUSH SSL actually get the new files, even though the values of the variables didn't change? This is what I understand from the documentation: https://mariadb.com/kb/en/flush/#flush-ssl "If you want to dynamically reinitialize the server's TLS context, then you need to change the certificate and key files at the relevant paths defined by these TLS system variables, without actually changing the values of the variables." But I just want to clarify that my understanding is correct. Thank you very much!

            Variables do not change, but files are reloaded. That is you'd need to replace already existing certificate files in the relevant paths, then FLUSH SSL

            wlad Vladislav Vaintroub added a comment - Variables do not change, but files are reloaded. That is you'd need to replace already existing certificate files in the relevant paths, then FLUSH SSL
            nunop Nuno added a comment -

            wlad Thanks! That's exactly what I was hoping for. That's perfect.

            nunop Nuno added a comment - wlad Thanks! That's exactly what I was hoping for. That's perfect.

            People

              wlad Vladislav Vaintroub
              GeoffMontee Geoff Montee (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.