Update 'key storage' section in readme

[sirAndros]

closes #83

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[SirAndrosBot]

Thanks for expanding this section. I think this should be tightened before merge because a few of the new security claims are stronger than what the implementation actually does.

Review findings:

• [P1] README.md:60 says the Windows Hello/NGC key is used "to encrypt and decrypt all parts of the composite keys." That is not what the code does. Windows Hello only protects the generated random password: ProtectedKey calls keyCipher.Protect(password). The composite key parts are encrypted/decrypted with KeePass AES via KeyCipher.Encrypt/Decrypt. Since this PR closes a security-clarity issue, this should be corrected before merge.

• [P2] README.md:60 says local mode asks Windows to issue a disposable key. Local mode retrieves the existing default NGC decryption key name with NgcGetDefaultDecryptionKeyName and opens it. The plugin-created key is the persistent key path via CreatePersistentKey. "Disposable" and "asks Windows to issue" are misleading here.

• [P2] README.md:61 says encryption and decryption are performed "with a WinHello prompt." Encryption opens the key silently with CngKeyOpenOptions.Silent; the prompt is applied for decrypt and persistent key creation. Better: encryption is performed by Windows Hello/CNG, while later decrypt requires Windows Hello authorization.

• [P2] README.md:66 says persistent mode does not store the key name or encrypted data in memory. It does not keep a long-lived in-memory cache, but it necessarily builds the persistent key name on demand and copies Credential Manager blobs into process memory during read/write. The absolute wording overstates the memory guarantee.

No code changes to test; this is documentation-only.

[SirAndrosBot]

Inline review comments added on the corresponding README lines.

[SirAndrosBot]

This overstates the role of the Windows Hello/NGC key. In the implementation, Windows Hello protects the generated random password (ProtectedKey calls keyCipher.Protect(password)), while the individual composite key parts are encrypted/decrypted with KeePass AES via KeyCipher.Encrypt/Decrypt. Saying the NGC key encrypts/decrypts all composite-key parts gives readers the wrong security model.

Proposed change:

Suggested change

	In this scenario, the plugin asks Windows to issue a disposable encryption key in its security subsystem (NGC) and then uses it by name to encrypt and decrypt all parts of the composite keys of databases.
	In this scenario, the plugin uses the current user's default Windows Hello/NGC decryption key by name to protect a generated random wrapping key; the database composite-key parts are encrypted with that wrapping key before being kept in memory.

[SirAndrosBot]

The local path does not create a new disposable key. WinHelloProvider retrieves the current default NGC decryption key name with NgcGetDefaultDecryptionKeyName and opens that key. The plugin-created key is the persistent-storage path, created by CreatePersistentKey. I would avoid "asks Windows to issue a disposable encryption key" here.

Proposed change:

Suggested change

	In this scenario, the plugin asks Windows to issue a disposable encryption key in its security subsystem (NGC) and then uses it by name to encrypt and decrypt all parts of the composite keys of databases.
	In this scenario, the plugin uses the current user's default Windows Hello/NGC decryption key by name to protect a generated random wrapping key; the database composite-key parts are encrypted with that wrapping key before being kept in memory.

[SirAndrosBot]

Encryption and decryption are not both performed with a Windows Hello prompt. Encryption opens the key with CngKeyOpenOptions.Silent; the user prompt is applied when decrypting and when creating the persistent key. A more precise phrasing would separate Windows/CNG encryption from Windows Hello authorization for later decrypt.

Proposed change:

Suggested change

	The plugin gets and stores only an ID of this key and the encrypted data itself. Encryption and decryption are performed by Windows with a WinHello prompt.
	The plugin stores the encrypted data in memory and resolves the Windows Hello key name when needed. Windows/CNG performs encryption silently; later decryption requires Windows Hello authorization.

[SirAndrosBot]

This absolute memory claim is too strong. Persistent mode does not keep a long-lived in-memory cache, but the code still builds the persistent key name on demand and copies Credential Manager blobs into process memory while reading/writing them. Better to say the encrypted database key is stored in Credential Manager rather than cached in the plugin's in-memory storage.

Proposed change:

Suggested change

	In this scenario, the key has a constant name based on the current user ID, and the plugin does **not** store in memory either the key name or the encrypted data.
	In this scenario, the key has a deterministic name based on the current user ID, and encrypted database keys are stored in Windows Credential Manager rather than in the plugin's long-lived in-memory cache.

Superseded by native inline review comments on the corresponding changed lines.