search

readium / readium-lcp-client

This repository is for the Readium Licenced Content Protection (LCP) client side implementation work.
BSD 3-Clause "New" or "Revised" License
16 stars 12 forks source link

KeyCheck should be verified when attempting user keys from vault storage (currently only from initial passphrase) #51

Open danielweck opened 5 years ago

danielweck commented 5 years ago

The KeyCheck is correctly verified in DecryptUserKey():

https://github.com/readium/readium-lcp-client/blob/e4c02fdd396157062b80dcd6b787a2c6a72a0da0/src/lcp-client-lib/CryptoppCryptoProvider.cpp#L232-L276

However, it should also be verified in DecryptContentKey():

https://github.com/readium/readium-lcp-client/blob/e4c02fdd396157062b80dcd6b787a2c6a72a0da0/src/lcp-client-lib/CryptoppCryptoProvider.cpp#L278-L310

The only reason why the current code "works" is because of a fortunate side effect from the CryptoPP lib, which emits an error when an attempt to decrypt a cypher using the wrong key is made, because of incorrect padding number parsing:

https://github.com/readium/readium-lcp-client/blob/e4c02fdd396157062b80dcd6b787a2c6a72a0da0/src/third-parties/cryptopp/filters.cpp#L762

The source of the problem is DecryptLicenseByStorage():

https://github.com/readium/readium-lcp-client/blob/e4c02fdd396157062b80dcd6b787a2c6a72a0da0/src/lcp-client-lib/LcpService.cpp#L410-L459

...more specifically m_storageProvider->EnumerateVault which will find the first existing user key that "works", without actually verifying the KeyCheck!

This is the call chain:

OpenLicense() => CheckDecrypted() => DecryptLicenseOnOpening() => DecryptLicenseByStorage()

Then DecryptLicenseByUserKey() => DecryptContentKey() where the KeyCheck verification is missing. Thankfully, we have the CryptoPP "crash" to simulate the KeyCheck verification, but this is VERY hacky!

The line that crashes: https://github.com/readium/readium-lcp-client/blob/e4c02fdd396157062b80dcd6b787a2c6a72a0da0/src/lcp-client-lib/CryptoppCryptoProvider.cpp#L301

...is caught here:

https://github.com/readium/readium-lcp-client/blob/e4c02fdd396157062b80dcd6b787a2c6a72a0da0/src/lcp-client-lib/CryptoppCryptoProvider.cpp#L306-L309

...which allows graceful continuation of the m_storageProvider->EnumerateVault iteration, here:

https://github.com/readium/readium-lcp-client/blob/e4c02fdd396157062b80dcd6b787a2c6a72a0da0/src/lcp-client-lib/LcpService.cpp#L454-L456

danielweck commented 5 years ago

Fixed in the feature/latest-working-build-config branch (Pull Request #49) https://github.com/readium/readium-lcp-client/commit/f89dcacf127b7e0f526193692d5714f80cb318b7

danielweck commented 5 years ago

Follow-up fix for more general UTF8 validation checks: https://github.com/readium/readium-lcp-client/commit/1c4bb3f86b3fe6fd902cc4572a66c74f0b39e45d