MS10 - 070 Post Mortem analysis of the patch
Now that Microsoft has patched the POET vulnerability, I thought it would be instructive to see what they changed. Now I'm not masochistic enough to disassemble aspnet_wp.exe or webengine.dll so there are things changed there that I don't know... but I did do a Reflector Export of the System.Web and System.Web.Extensions assemblies and then used BeyondCompare to do a diff against the files.
The analysis is simple:
- Don't leak exception information - This prevents exploits from seeing what is broken.
- Don't short-circuit on padding checks (take the same amount of time for padding correct verses padding broken) - This prevents exploits from seeing timing difference for incorrect padding.
- Don't be too picky about exception catching in IHttpHandler.ProcessRequest - This prevents exploits from seeing that you caught one kind of exception (CryptographicException) instead of all exceptions.
- Switch from Hash-based initialization vectors to Random IVs - This prevents exploits from using the relationship between the data and the hash to decrypt faster.
- Allow for backward compatibility - In case this breaks something, allow the new behavior to be reverted in-part.
- When doing a code review pass, change to make it clear you've considered the new options.
Here's the blow-by-blow review
Changes in System.Web
- AssemblyInfo.cs
(v2.0/v3.5) Bump AssemblyFileVersion from 2.0.50272.5014 to 2.0.50727.5053
(v4.0) Bump AssemblyFileVersion from 4.0.30319.1 to 4.0.30319.206 - ThisAssembly.cs
(v2.0/v3.5) Bump InformationalVersion from 2.0.50272.5014 to 2.0.50727.5053
(v4.0) Bump the BuildRevisionStr from 1 to 206
(v4.) Bump InformationalVersion from 4.0.30319.1 to 4.0.30319.206 - HttpCapabilitiesEvaluator.cs
When caching the browser capabilities, sets the cache expiration to sliding time based on the the setting specified (instead of no sliding, no absolute) - MachineKeySection.cs
Calls to EncryptOrDecryptData without specifying an initialization vector type now defaults to IVType.Random instead of IVType.Hash.
Now uses AppSettings.UseLegacyEncryption flag to determine if the data should be signed.
If decrypting and signing, now checks to ensure that the data has unhashed content (GetUnHashedData) and throws if there is no data other than the hash block. If there is no data after the signature, throws new Exception()!
VerifyHashedData no longer fast-aborts when the hashed data mismatches. This makes the check take the same amount of time whether a match is found or not. - Handlers\AssemblyResourceLoader.cs
ProcessRequest now catches any exception and morphs to an undistinguished InvalidRequest instead of leaking it out.
(v4.0) Also no longer passes the assembly name to the InvalidRequest exception formatting. - Security\MachineKey.cs (v4.0)
Decode respects the new AppSettings.UseLegacyMachineKeyEncryption setting. [note, not AppSettings.UseLegacyEncryption] - Security\MembershipAdapter.cs (v4.0)
EncryptOrDecryptData now explicitly passes false for signData to MachineKeySection.EncryptOrDecryptData to remain compatible with prior calls, whilst still showing this code has been checked. - Security\MembershipProvider.cs
DecryptPassword and EncryptPassword now explicitly passes false for useValidationSymAlgo and signData to MachineKeySection.EncryptOrDecryptData to remain compatible with prior calls, whilst still showing this code has been checked. - UI\WebControls CheckBoxField.cs CheckBoxList.cs MailDefinition.cs ObjectDataSource.cs and WebControl.cs
(v4.0) Various property attributes have been reordered [unrelated, artifact of a new compiler version?] - UI\ObjectStateFormatter.cs
Deserialize now ignores the caught exception and now throws the MacValidationError without leaking out the exception details. - UI\Page.cs
Now flags the page response as in-error for any exception, not just a CryptographicException, thus not leaking out the exception kind. - Util\AppSettings.cs
Now exposes a UseLegacyEncryption setting to allow reverting to old behavior for encryption. [defaults to false]
(v4.0) Now exposes a UseLegacyMachineKeyEncryption setting to allow reverting to old behavior for MachineKey encryption. [defaults to false]
(v4.0) Now exposes a ScriptResourceAllowNonJsFiles setting to allow reverting to old behavior for ScriptResource encryption. [defaults to false] - Util\VersionInfo.cs (v4.0)
Bumped the serialized SystemWebVersion property from 4.0.30319.1 to 4.0.30319.206
System.Web.Extensions (v4.0)
- ScriptResourceHandler.cs
ProcessRequest now catches any exception and throws an undistinguished 404 error.
ProcessRequest now uses the new AppSettings.ScriptResourceAllowNonJsFiles setting to control behavior when a resource is requested that doesn't end in ".js" If enabled (NOT DEFAULT), then resource is allowed, otherwise now will throw an undistinguished 404 error. - VersionInfo.cs
Bumped the serialized SystemWebVersion property from 4.0.30319.1 to 4.0.30319.206 - ThisAssembly.cs
Bumped the build revision from 1 to 206
Posts
8 comments:
Very interesting! Thanks!
awesome! thanks!
Fantastic, thanks for taking the time to look this over. You're referenced on StackOverflow.com: http://stackoverflow.com/questions/3824125/how-was-the-oracle-padding-attack-on-asp-net-fixed/3824238
Very nice detailed info, saved me the trouble from opening it :)
As I mentioned in the stackoverflow question @Guest linked to, imho the main fix was to sign any encrypted data that is sent to the browser.
Others are there for compatibility, further try to prevent side channels and take out the unnecessary risk of non js files exposure. Not saying these aren't important, specially since what the impact the researches claimed was only possible because of the web.config exposure and having the machine keys in it: http://eglasius.blogspot.com/2010/09/aspnet-padding-oracle-how-it-relates-to.html. Note that was confirmed in a public tweet response by one of the researches, they didn't break features that had validation until they got the keys from the web.config (which was the default in DotNetNuke, which is supposed to have 600k+ installs out there)
Couple of clarifications:
1) Although calls to EncryptOrDecryptData without specifying an initialization vector type now defaults to IVType.Random, the calls coming from Page and the resource handlers (going through Page) still specify Hash as the IV type.
2) VerifyHashedData is new and doesn't fast-abort on a mismatch. This is not the original padding check that's within the crypto services, so the crypto services are still "susceptible" to timing attacks. However, the HMAC hash verification is performed before the padding check and the attacks would fail there even before reaching the crypto services.
WRT preventing leaks. They may have improved it but I do not see how it is possible in 100%. If someone tampers with data (let's say it is not signed) and it is not a valid data for the algorithm to decrypt then this is one type of error. If the data is valid and now the application needs to decide it had decrypted nonsense this is another type. There will always be a side channel (e.g. time) that leaks. Unless they use random sleep?
Also why does using a random IV help? AFAIK the attacker guesses the IV and does not assume anything about it anyway.
Great post! Thanks
Thank you for this post. We are running something that uses aspnet Membership provider passwords directly, and this was helpful in knowing how to call the changed EncryptOrDecryptData call.
Post a Comment