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

feat: verify packed x5c start/end dates #151

Merged
merged 3 commits into from
Apr 8, 2019
Merged

Conversation

grzuy
Copy link
Contributor

@grzuy grzuy commented Apr 6, 2019

No description provided.

@grzuy grzuy changed the title feat: verify packed x5c leaf cert start/end dates feat: verify packed x5c start/end dates Apr 7, 2019
end
end

# TODO: Reevaluate this check
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good call - certificates containing RSA keys should be valid too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

def certificate_in_use?(certificate)
now = Time.now

certificate.not_before < now && now < certificate.not_after
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't add real security since any certificate is trusted. A more thorough solution that includes this check as well would be to verify using a certificate trust story (like in Android Safetynet), but then #66 becomes required again to do this properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree this is not thorough.
Just a step in that direction.

@grzuy
Copy link
Contributor Author

grzuy commented Apr 8, 2019

Thank you for reviewing.

@grzuy grzuy merged commit c3627a8 into master Apr 8, 2019
@grzuy grzuy deleted the verify_packed_cert_dates branch April 8, 2019 13:45
@grzuy grzuy added this to the FIDO Conformance Server tests milestone Apr 25, 2019
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.

2 participants