#2735 Secure removal of secret data storage
Closed: fixed Opened by edewata.

When a secret data (e.g. password, private key) is no longer needed, the storage which held the data should be wiped out to ensure it cannot be read again. This applies to arrays in memory and files on disk.

There are several methods:

  • fill with zeroes
  • fill with random data

The first one is faster, but the second one is more secure, especially on disk.


Metadata Update from @mharmsen:
- Custom field component adjusted to General
- Custom field feature adjusted to ''
- Custom field origin adjusted to Community
- Custom field proposedmilestone adjusted to ''
- Custom field proposedpriority adjusted to ''
- Custom field reviewer adjusted to ''
- Custom field type adjusted to defect
- Custom field version adjusted to ''
- Issue priority set to: critical
- Issue set to the milestone: 10.4

Metadata Update from @mharmsen:
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1466066

https://www.lifewire.com/shred-linux-command-4094148

Metadata Update from @mharmsen:
- Issue set to the milestone: 10.5 (was: 10.4)

Metadata Update from @mharmsen:
- Issue priority set to: major (was: critical)

Per CS/DS Meeting 09/25/2017: 10.5 blocker

Metadata Update from @mharmsen:
- Issue priority set to: blocker (was: major)

Metadata Update from @jmagne:
- Issue assigned to jmagne

Testing instructions for phase one of this where, we work on the most crucial kra first:

Testing instructions:

With this fix, we have decided to clear security data only in the kra, to act as a proof of concept.

To test the integrity of this fix, we want to make sure that the kra still functions normally and that the log messages created when we clear memory are showing up when expected. The following is what can be done at this point to test this all out.

  1. Stand up a kra along with a CA.

  2. Configure the kra to allow encryption and decryption when archiving and recovering keys. This is because the fix lies in this area heavily:

kra.allowEncDecrypt.recovery=true
kra.allowEncDecrypt.archival=true

We can also configure the desired obscuring method, with "zeroes" being the default:

jss.obscureMethod=random
or
jss.obscureMethod=zeroes

  1. Restart the kra:

  2. We want to run the test program called "drmtest.py", because this exercises the SecurityData / rest functionality we have added to the kra. This functionality archives and recovers things like sym keys, asym keys, and passphrases or security strings.

  3. The test program is located in a source tree here:

pki/base/kra/functional/drmtest.py

Help to run this test is located in a text file in the same area:

pki/base/kra/functional/drmtest.readme.txt

This file will show how to configure and run the test.

  1. Run drmtest.py and observer that all the tests succeed. This can be observed by simply watching the output in the console. Note that there are several tests that are designed to fail, so make sure that continues.

  2. After running the drmtest, look in the kra debug log to make sure there are places where the function to clear memory is being called.

  3. Finally, in order to test general legacy kra functionality, preform the usual standard / legacy key archival and recovery testing procedures and make sure everything works and that the logs reflect the memory being cleared out.

Subsequent checkin: This handles some more byte array cases:

commit f5ec7c2af4a1fb44d5731c74672bf789e9240499 (HEAD -> master, origin/master, origin/HEAD, obscure-memory)
Author: Jack Magne jmagne@redhat.com
Date: Tue Nov 7 11:05:55 2017 -0800

Fix #2735 Secure removal of secret data storage (phase 2)
This portion of the fix attempts to take care of the remaining secret data
storage issues that could be practically taken care of with respect to
servers and clients.
A new method was placed in CryptoUtil to server the needs of clients.
Change-Id: I1a14daabcad72e531572d1be8bc255e2e501b70a

We still need to address the many String based password type cases.

commit daff3951340246d97a9877d5dde4782c8c675974
Author: Jack Magne jmagne@redhat.com
Date: Fri Nov 10 10:57:36 2017 -0800

Fix #2735 Secure removal of secret data storage (phase 3)
Add more secure data removal with respect to passwords.
Concentrate on the CMC Shared Token area. Done by changing
String based passwords to char[] based password, which then can be cleaned.
Cleaned up a couple of minor review suggestions.
Change-Id: I898814000353978f403f19f679083474548edc5e

Testing instructions for phase 2 and 3:

We added 2 more phases of the checkins to this bug.

Once again, we made some changes to try to secure some more memory data and password data.

The changes are not server wide but we wanted to cover some more important sections of the code. Those sections being all the new CMC client and server code and some more changes to the KRA to support password data cleanup. As for passwords, the framework code for cleaning all passwords has been put in place and exercised in the CMS and KRA area. The following are the areas to test for correctness and sanity. There is no way QE can actually tell if the data is being obscured correctly, but we want to make sure we have not broken anything .Therefore the following tests would be useful:

  1. Test all of the new CC CMC client and server code, with emphasis on the new SHARED TOKEN pathway. The code had to be moved a bit to support cleaning the passoword, thus we want to make sure the shared token authentication still works, and to perform sanity on the rest of the CMC functionality of interest to CC.

  2. Re-test all the kra stuff talked about in phase 1. This is because we added some password functionality to the KRA security data archival and recovery routines.

  3. Make sure configuration works along with pk#12 processing. There were some minor changes to some of the code that creates and processes pk#12 files. This is where sanity should suffice, since I"m sure all this stuff is tested anyway.

  4. A small bit of TKS /TMS code was enhanced for security data removal. Just observe that the standard TMS stuff still works as usual.

Note:

Some but not all of the server changes will have messages placed in the debug logs, indicating that a piece of data is being obscured or clean. Simple inspection of some of that can at least show the idea that this kind of thing is going on. Not all will be recorded since some of the cleaning is taking place in client code.

Closing this one out, since it could be worked on forever. If we feel the need to add to this, we can create a new ticket.

Metadata Update from @jmagne:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

Metadata Update from @mharmsen:
- Issue set to the milestone: 10.5.3 (was: 10.5)

Metadata Update from @mharmsen:
- Custom field fixedinversion adjusted to pki-core-10.5.3-1.fc27

Need to clear out passwords in files as well. reopening...

Metadata Update from @cfu:
- Issue status updated to: Open (was: Closed)

I verified on the wrong version. It appears to be fixed.

Metadata Update from @cfu:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

Dogtag PKI is moving from Pagure issues to GitHub issues. This means that existing or new
issues will be reported and tracked through Dogtag PKI's GitHub Issue tracker.

This issue has been cloned to GitHub and is available here:
https://github.com/dogtagpki/pki/issues/2855

If you want to receive further updates on the issue, please navigate to the
GitHub issue and click on Subscribe button.

Thank you for understanding, and we apologize for any inconvenience.

Metadata