AES256 CBC W3C padding scheme (instead of PKCS#7)

[danielweck]

In reaction to:
readium/readium-lcp-server#18 (comment)

credits @drminside

Work In Progress

This contribution involves patching Crypto++

[danielweck]

Server implementation: readium/readium-lcp-server#111

[llemeurfr]

I just see in the DRMInside commit that the inline comments (line 99 and others was not changed and seems to indicate PKCS#7 is used, which is not the case anymore. Shouldn' the comment be updated?

[danielweck]

@llemeurfr agreed.

[danielweck]

On the client side, personally I would like to be able to switch between PKCS#7 and W3C padding schemes based on some discoverable metadata, such as the URI http://www.w3.org/2001/04/xmlenc#aes256-cbc
(which impies W3C padding algorithm)
Unfortunately, I am not aware of a URI / metadata that indicates PKCS#7
Any ideas?

[llemeurfr]

What is the advantage, for the ecosystem, of offering such a flexibility?

[danielweck]

No discernable advantage for the "ecosystem", I think, because the LCP profile strictly requires one padding scheme.
However, if PKCS#7 vs. W3C discovery is cheap (e.g. URI comparison), then this would make the code clearer, and less prone to errors in future evolutions.
We already have a well-defined switch for GCM vs. CBC for the AES256 encryption (based on a canonical URI), in both server and client implementations. Yet, right now the LCP ecosystem mandates the use of CBC (in other words, excludes the use of GCM).

At any rate, assuming DRM-Inside's code contribution passes peer-reviews and tests, do we revisit the decision to use PKCS#7?
readium/readium-lcp-server#18 (comment)

My personal opinion is that if the URI http://www.w3.org/2001/04/xmlenc#aes256-cbc indeed implies W3C padding instead of PKCS#7, then we clearly have an inconsistency and we should stop using PKCS#7. Or, we change the URI to mirror the fact that PKCS#7 is used.

[llemeurfr]

I agree that DRMInside's contrib makes us revisit the decision to use PKCS#17.
Note that the Readium LCP Encryption Profile 1.0 draft spec has proactively modified by Taehyun to reflect that :-)
Let's validate this contribution and update readium/readium-lcp-server#18 (comment).

The other question is: should we be able to activate the use of PKCS#7 using some discoverable metadata? My feeling is that there is no real interest in that, and that the code (which is kept in the history) can be removed from the "live" code.

[danielweck]

okay.

[jpbougie]

The only difference between PKCS#7 and W3C padding is that the W3C variant does not specify which values to use to fill the padding bytes (except for the last one), while PKCS specifies that every padding byte must be equal to the padding length. Thus, while appending the padding bytes, PKCS#7 is a superset of the W3C padding.

As we can see in the client PR, the critical part is relaxing the requirement that the padding bytes be the same as the length of the pad. I would thus suggest that we keep the server as is and relax the requirement on the client, which is what DRMInside already proposed.

[thkim2015]

IMO, we need to consider why W3C suddenly added the padding restriction in 2002.

There was no padding details before 2002 in the W3C encryption standard. But after publishing the possibility of the Padding Oracle Attack on the block cipher algorithm in 2002, W3C had instantly updated the specification as size of the padding bytes should not be described in the padding block except last byte.

Key point of the padding oracle is that the last encrypted block before the PKCS#7 padding could be easily cracked, if the decryption program provides unnecessary error codes related padding.

So I think that not allowing using the vulnerable PCKS#7 on both server and client side is the clear way for not only consistency of the specification, but also the security perspective.

[danielweck]

Now that Readium is effectively creating a fork of CryptoPP, I have submitted a Pull Request at Crypto++ to add support for the W3C padding scheme for AES-256-CBC, based on DRM-Inside's code: weidai11/cryptopp#368

[danielweck]

I have successfully tested this code with EPUBs encrypted by the Go server with the existing PKCS#7 method (this works because W3C algorithm operates on the client side as a "superset" functionally-speaking)

[danielweck]

Merging this CryptoPP patch directly into the Readium LCP client lib source tree.

Note that we now maintain a Crypto++ fork in "parallel" at https://github.com/edrlab/cryptopp
So, we can easily track upstream changes and manually merge them into the LCP client lib source tree.

[danielweck]

Follow-up: weidai11/cryptopp@8a177c5

[danielweck]

The above follow-up is merged (cherry-picked) into the EDRLab fork of CryptoPP:
edrlab/cryptopp@CRYPTOPP_5_6_5...CRYPTOPP_5_6_5_CBC_W3C_PADDING

[danielweck]

So, all-in, this is the CBC W3C_PADDING diff:
edrlab/cryptopp@CRYPTOPP_5_6_5_XCode_etc_projects...CRYPTOPP_5_6_5_CBC_W3C_PADDING