Skip to content

Allow validating the created parameter#8

Merged
nomadium merged 2 commits into
nomadium:masterfrom
oneiros:validate_created
Apr 2, 2025
Merged

Allow validating the created parameter#8
nomadium merged 2 commits into
nomadium:masterfrom
oneiros:validate_created

Conversation

@oneiros

@oneiros oneiros commented Mar 26, 2025

Copy link
Copy Markdown
Contributor

Fixes #7

This is a first attempt to add validation of the created parameter. This can be used to mitigate the risk of replay attacks.

This is just a first suggestion. Let me know what you think, I am more than happy to change the API, move stuff around etc.

To mitigate the risk of replay attacks.
Comment thread lib/linzer/verifier.rb Outdated
parameters = signature.parameters
components = signature.components

if no_older_than && (Time.now.to_i - parameters["created"]) > no_older_than.to_i

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the signature being verified doesn't have a created parameter and is requested to check that is not older than some some value, I think we want to raise a Error, otherwise it will fail with an exception like the following:

TypeError: nil can't be coerced into Integer (TypeError)

          puts (Time.now.to_i - parameters["created"])
                                ^^^^^^^^^^^^^^^^^^^^^

Maybe in #validate method in Verifier class can be validated that if no_older_than option is present, then signature parameter must be present and its value should respond to to_i.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed that. I moved some code around and now raise a proper error in this case.

@nomadium

nomadium commented Mar 29, 2025

Copy link
Copy Markdown
Owner

Thanks for PR @oneiros!

I have a couple of comments:

  • I think Linzer::Verifier should do a more thorough input validation. Signature created parameter is recommended according to the RFC but even if it were mandatory, signature input validation has to handle the case when the parameter is missing in the request or is not a valid integer.

  • The RFC also defines an optional parameter named expires that could/should interact with the option introduced in this PR.

...before checking age of signature.
@oneiros

oneiros commented Apr 1, 2025

Copy link
Copy Markdown
Contributor Author
* I think Linzer::Verifier should do a more thorough input validation. Signature `created` parameter is recommended according to the RFC but even if it were mandatory, signature input validation has to handle the case when the parameter is missing in the request or is not a valid integer.

I think it is important to fail in these cases, which is not the same as diligently validating every possible thing that another implementation could conceivably get wrong.

That is to say, you are probably right, but I worry a bit about performance. I guess one needs to find the right balance here.

* The RFC also defines an optional parameter named `expires` that could/should interact with the option introduced in this PR.

I think validating based on expires is a different feature that would deserve its own PR.

@nomadium
nomadium merged commit 408b805 into nomadium:master Apr 2, 2025
@nomadium

nomadium commented Apr 2, 2025

Copy link
Copy Markdown
Owner

Thanks @oneiros for bringing attention to this issue and sending a PR to address it.

Yes, I agree the library should not go over the board with too many validation checks, by "thorough" I just meant that signatures with an invalid or missing created parameter should not raise unexpected exceptions.

I also agree that expiration validation through expires parameter should go on its own PR.

Thanks again for the fix, I'll cut a new release tomorrow or so.

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.

Validating created parameter

2 participants