Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-65818

Security: Provide admin setting type for secure data (passwords/tokens)

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide

      This change introduces a new admin setting that plugins (and core) can use, but doesn't actually use it anywhere. So to test it you need to install a patch which contains a silly example of the setting.

      1. Set the 'Home page for users' option under admin settings / Appearance / Navigation to 'Site home'
      2. Apply the testing patch from attached file mdl65818.patch.
      3. Purge caches on your server (/admin/purgecaches.php).
      4. Go to the admin notifications page (/admin URL).
        • EXPECTED: You should be prompted to save the new admin setting 'silly password'.
      5. Leave the new setting blank and hit save.
      6. Go to the admin settings under 'Site administration / Server / Cleanup'.
        • EXPECTED: It should say that the setting is 'not set' (because you left it blank), with an edit button.
      7. Click the edit button next to the setting. (You can also click the label.)
        • EXPECTED: A blank edit box should appear and should be focused for text input.
      8. Type a word such as 'Frog' into the box.
      9. Save changes
        • EXPECTED: The setting should now show that it is set and encrypted.
      10. Go to the site home page (/ URL).
        • EXPECTED: A message at the top should tell you that the silly password is: Frog (or whatever you typed).
      11. Go back to the settings page.
      12. Click the edit button again.
        • EXPECTED: The edit box appears, and is blank.
      13. Click the cancel button (to right of box).
        • EXPECTED: The box should disappear and should show 'Set and encrypted' again.
      14. Look at site home.
        • EXPECTED: It should still show the password.
      15. Go back to the settings page.
      16. Click the edit button.
      17. Type a new password, such as 'Zombies' and save changes.
      18. Check site home.
        • EXPECTED: The new password should appear.
      19. Go back to the settings page.
      20. Click the edit button.
      21. Leave the field blank, and save changes.
        • EXPECTED: It should say the password is unset.
      22. Check site home:
        • EXPECTED: The password should now be blank.
      23. Run the Behat test tagged 'mdl65818' and check it passes (this tests the Behat step change that I added to make it possible to set these settings from Behat).
      24. At the command line, run the new CLI script: php admin/cli/generate_key.php
        • EXPECTED: You should see a 'Key already exists' error with the location of the key file.
      25. Delete this file, and re-run the CLI script: php admin/cli/generate_key.php
        • EXPECTED: You should see 'Key created' with the location of the key file, and a short sentence about copying the key file if it's not in shared storage.
      26. On your server, create a new directory to test configuring the secret data storage. It can be anywhere. For the sake of example I'll assume it is /home/sam/secret.
      27. Ensure that your web server has access to read the directory. On Unix, this might require a command like 'chmod a+rx /home/sam/secret'.
      28. Edit your config.php to add the line (replacing my example path with yours): $CFG->secretdataroot = '/home/sam/secret';
      29. Also add a second line: $CFG->nokeygeneration = true;
      30. In the web browser, go to the settings page as before, and try to change the encrypted setting.
        • EXPECTED: You should get an error 'Key not found'. This is because $CFG->nokeygeneration is set, so the server won't automatically generate a key in the new location.
      31. At the commandline, run the CLI script: php admin/cli/generate_key.php
        • EXPECTED: A new key should be created and it should be stored in a 'key' subdirectory of the specified secret folder (e.g. /home/sam/secret/key/sodium.key).
      32. Now retry changing the encrypted setting.
        • EXPECTED: This time it should let you save the change to the setting,

      After finishing the test, remember to remove the patch from your code.

      Show
      This change introduces a new admin setting that plugins (and core) can use, but doesn't actually use it anywhere. So to test it you need to install a patch which contains a silly example of the setting. Set the 'Home page for users' option under admin settings / Appearance / Navigation to 'Site home' Apply the testing patch from attached file mdl65818.patch. Purge caches on your server (/admin/purgecaches.php). Go to the admin notifications page (/admin URL). EXPECTED: You should be prompted to save the new admin setting 'silly password'. Leave the new setting blank and hit save. Go to the admin settings under 'Site administration / Server / Cleanup'. EXPECTED: It should say that the setting is 'not set' (because you left it blank), with an edit button. Click the edit button next to the setting. (You can also click the label.) EXPECTED: A blank edit box should appear and should be focused for text input. Type a word such as 'Frog' into the box. Save changes EXPECTED: The setting should now show that it is set and encrypted. Go to the site home page (/ URL). EXPECTED: A message at the top should tell you that the silly password is: Frog (or whatever you typed). Go back to the settings page. Click the edit button again. EXPECTED: The edit box appears, and is blank. Click the cancel button (to right of box). EXPECTED: The box should disappear and should show 'Set and encrypted' again. Look at site home. EXPECTED: It should still show the password. Go back to the settings page. Click the edit button. Type a new password, such as 'Zombies' and save changes. Check site home. EXPECTED: The new password should appear. Go back to the settings page. Click the edit button. Leave the field blank, and save changes. EXPECTED: It should say the password is unset. Check site home: EXPECTED: The password should now be blank. Run the Behat test tagged 'mdl65818' and check it passes (this tests the Behat step change that I added to make it possible to set these settings from Behat). At the command line, run the new CLI script: php admin/cli/generate_key.php EXPECTED: You should see a 'Key already exists' error with the location of the key file. Delete this file, and re-run the CLI script: php admin/cli/generate_key.php EXPECTED: You should see 'Key created' with the location of the key file, and a short sentence about copying the key file if it's not in shared storage. On your server, create a new directory to test configuring the secret data storage. It can be anywhere. For the sake of example I'll assume it is /home/sam/secret. Ensure that your web server has access to read the directory. On Unix, this might require a command like 'chmod a+rx /home/sam/secret'. Edit your config.php to add the line (replacing my example path with yours): $CFG->secretdataroot = '/home/sam/secret'; Also add a second line: $CFG->nokeygeneration = true; In the web browser, go to the settings page as before, and try to change the encrypted setting. EXPECTED: You should get an error 'Key not found'. This is because $CFG->nokeygeneration is set, so the server won't automatically generate a key in the new location. At the commandline, run the CLI script: php admin/cli/generate_key.php EXPECTED: A new key should be created and it should be stored in a 'key' subdirectory of the specified secret folder (e.g. /home/sam/secret/key/sodium.key). Now retry changing the encrypted setting. EXPECTED: This time it should let you save the change to the setting, After finishing the test, remember to remove the patch from your code.
    • Affected Branches:
      MOODLE_310_STABLE, MOODLE_38_STABLE
    • Fixed Branches:
      MOODLE_311_STABLE
    • Pull 3.11 Branch:
      MDL-65818-m311
    • Pull Master Branch:
      MDL-65818-master

      Description

      (This is something I have implemented locally and am offering for the community in case people want it.)

      We had a requirement to store a certain password (in a plugin admin setting) more securely than Moodle normally allows. Normally, passwords are stored in the mdl_config or mdl_config_plugins tables using the admin_setting_configpasswordunmask control. This has two disadvantages:

      • Any system administrator (or anyone with moodle/site:config) can access the password value from the user interface.
      • Anyone with direct access to the database can access the password using a database query.

      In the OU setup there are quite a few people who are server administrators, and also quite a few people (developers) who have direct access to the database for troubleshooting purposes. Worse, we then have many more people with moodle/site:config, because of stupid. Anyway, all these people are trusted staff, so for most purposes, the Moodle system, while not great, is adequate.

      However the new password we are storing in our custom plugin allows quite a high level of access to student personal data that is not stored in Moodle. A few of the people I listed above do have access to this data already, but most don't - our security office required that we make sure this password is kept secret.

      The change I implemented, and which I propose for standard Moodle, is a new admin setting with the following behaviour:

      • In the user interface, allow admins to change the password, but not to view it.
      • Store the password encrypted in the database.
      • Store the encryption key in the file system - in moodledata or, optionally, in a separate configurable location.
      • The encryption key can be created automatically by the server, or (e.g. if you want to use a non-shared folder in a multi-server cluster) created manually by using a CLI script and then copied to the other servers.

      This means we can configure it so that:

      • Our staff who can (some of them) access the Moodle admin UI, and the database, and the normal moodledata filesystem, have no way of getting the key.
      • An attacker needs to steal both the database, and the protected filesystem area, to get the key. Or they have to gain command line access to the server as root or as the apache user.

      Obviously the webserver does need to be able to decrypt the password so that it can use it when connecting to the remote system that requires it, so we can't make it any more secure than that.

      The cryptography uses the openssl extension which is already required.

      New configuration variables:

      • $CFG->secretdataroot (optional, if not set defaults to $CFG->dataroot . '/secret') - keys are stored inside a 'keypair' folder inside this folder.
      • $CFG->nokeypairgeneration (optional, default false) - the server will normally generate a keypair automatically the first time you encrypt something, but you can set this off if you want to prevent it, if you are planning to generate the keypair manually with the CLI script.

      Issues with this code:

      • I used public/private keys. This is not really necessary (it does not increase security because the same server holds and uses both keys) and is a bit of a hangover from a less-good way I was going to implement it previously. However, it does provide more flexibility, for example if we decided to make the public key public and allow external clients to set the password, pre-encrypted, using a web service or possibly even by email or something crazy.
      • I wasn't previously aware of MDL-36580, which Eloy told me about; that feature relates to encrypting data in the backup file. It is kind of similar but uses symmetric encryption and stores the key in the database. There is a question as to whether a consistent approach should be used!

        Attachments

        1. mdl65818.patch
          2 kB
        2. pic1.png
          pic1.png
          4 kB
        3. pic2.png
          pic2.png
          4 kB
        4. pic3.png
          pic3.png
          4 kB
        5. Screenshot_1.jpg
          Screenshot_1.jpg
          60 kB
        6. Screenshot_2.jpg
          Screenshot_2.jpg
          98 kB

          Issue Links

            Activity

              People

              Assignee:
              quen Sam Marshall
              Reporter:
              quen Sam Marshall
              Peer reviewer:
              Tim Hunt
              Integrator:
              Jake Dallimore
              Tester:
              Janelle Barcega
              Participants:
              Component watchers:
              Andrew Nicols, Dongsheng Cai, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              8 Vote for this issue
              Watchers:
              20 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                10/May/21

                  Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 4 hours, 1 minute
                  4h 1m