Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused recursive call #155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jaimeperez
Copy link
Contributor

When loading an EncryptedKey element, we call XMLSecEnc::staticLocateKeyInfo() to see if there's another KeyInfo element inside. This could happen when we have more than one level of key encryption. However, the static method does not modify the DOM or the key passed as parameters, it returns the key (or a new one). Given that the result of the call is never assigned to $objKey, this has never actually worked. It makes sense, since the most common escenario is data encrypted with a random symmetric key, and then that session key being sent alongside the cipherdata, in turn encrypted with the public key of the recipient.

We could fix this instead of just removing the line. However, given that it wasn't working, indicates there's no use for it. Fixing it on the other hand would require quite a lot of logic to limit the amount of recursion allowed.

When loading an EncryptedKey element, we call XMLSecEnc::staticLocateKeyInfo() to see if there's another KeyInfo element inside. This could happen when we have more than one level of key encryption. However, the static method does not modify the DOM or the key passed as parameters, it returns the key (or a new one). Given that the result of the call is never assigned to $objKey, this has never actually worked. It makes sense, since the most common escenario is data encrypted with a random symmetric key, and then that session key being sent alongside the cipherdata, in turn encrypted with the public key of the recipient.

We could fix this instead of just removing the line. However, given that it wasn't working, indicates there's no use for it. Fixing it on the other hand would require quite a lot of logic to limit the amount of recursion allowed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant