Azure / Azure/api-management-policy-snippets
Misuse of cryptographic primitives
- Dominant language
- No language data
- Stars
- 399
- Forks
- 170
- PR merge metrics
- No merged PRs in 30d
Description
Hello there :)
During a security review we noticed the following issues in this repository and would like them to confirm them so that they can the hopefully be fixed:
### Missing integrity protection
https://github.com/Azure/api-management-policy-snippets/blob/87225c2090e45add095919e8767c37d9ece42e0c/examples/oauth-proxy/oauth-proxy-slide-session-fragment.xml#L6-L25
Information is stored in an encrypted cookie that is not integrity protected. Therefore an attacker may tamper with the cookie to modify the expiration date.
The documentation correctly points out the steps that would be necessary to protect the integrity, i.e. a signature / HMAC or a cipher mode with integrity protection like AES-GCM
https://github.com/Azure/api-management-policy-snippets/blob/87225c2090e45add095919e8767c37d9ece42e0c/examples/oauth-proxy/readme.md?plain=1#L142-L159
According to the Microsoft documentation at https://learn.microsoft.com/en-us/azure/api-management/api-management-policy-expressions#ref-context-request, there is no parameter for the cipher mode and according to the .NET documentation, the default cipher mode is CBC (https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.symmetricalgorithm.mode?view=net-10.0#property-value) which does not offer integrity protection.
Let me know if there is a part that is missing.
### Using an IV as a secret
According to the comments in the code, the idea is to use store an IV with the cookie so that the cached contents cannot be decrypted without the cookie. The problem is that the IV is not a replacement for the secret and effectively only protects the first block in CBC and otherwise just makes sure the encryption is randomized.
https://github.com/Azure/api-management-policy-snippets/blob/87225c2090e45add095919e8767c37d9ece42e0c/examples/oauth-proxy/oauth-proxy-callback.xml#L78-L106
If you look at the following graphic showing CBC, you can see that even without knowing the IV, all steps can be performed in reverse except for the last one giving you all plaintext blocks except for the first one.
If you wanted to accomplish what is outlined in the plan, you would instead need to generate an ephemeral secret, store that in the cookie which is encrypted with the server-secrets (as done with the IV right now) and then store the IV together with the encrypted tokens in the cache. This way the server does know the IV but cannot decrypt the contents without knowing the ephemeral secret that is stored in the user cookie.
This issue is not as bad as the first one as it would require an attacker to compromise the server in order to decrypt the contents. Note that this secret is of no use to the client unless they also get access to the encrypted tokens which are never shared with them. To decrypt the token, a collaboration of client and server is therefore necessary which seems to be what the original plan was (according to my interpretation of the comments).
As a side note: The variable `ivCookieCacheKey` seems to be redundant.
Thanks you for the great policy fragements and let me know if you disagree on some of the points or would like to discuss them.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with examples/oauth-proxy/oauth-proxy-slide-session-fragment.xml and examples/oauth-proxy/oauth-proxy-callback.xml at the linked ranges, then read examples/oauth-proxy/readme.md and the referenced Azure/.NET documentation. Verify the integrity and IV/secret findings, including whether ivCookieCacheKey is redundant; done requires maintainer confirmation and an agreed remediation scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- azure
- Domain
- security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100