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.

Sign up for GitHub

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

Jump to bottom

ssh_key::certificate::Builder::new_with_validity_times incorrectly names validity time arguments #142

Closed
pjht opened this issue Jul 29, 2023 · 2 comments · Fixed by #143
Closed

ssh_key::certificate::Builder::new_with_validity_times incorrectly names validity time arguments #142

pjht opened this issue Jul 29, 2023 · 2 comments · Fixed by #143

Comments

@pjht
Copy link

pjht commented Jul 29, 2023

As described in the title, new_with_validity_time has the validity time arguments the wrong way around. What is listed as valid_after is really valid_before, and valid_before is really valid_after. This seems to be due to a swap when it calls Builder::new, passing valid_after as valid_before and vice versa.
@pjht pjht changed the title ssh_key::certificate::Builder::new_with_validity_times incorrectly names validity time arguments ssh_key::certificate::Builder::new_with_validity_times incorrectly names validity time arguments Jul 29, 2023
@tarcieri
Copy link
Member

Indeed they are swapped.

Looks like this method has no tests. Adding some should likely cause it to fail until they're swapped into the correct order.

tarcieri added a commit that referenced this issue Jul 29, 2023
@tarcieri
ssh-key: fix certificate::Builder::new_with_validity_times
80fe51c 
It previously had the `valid_after` and `valid_before` arguments
swapped, which would've caused errors with expected usage.

This commit adds a test that confirmed the certificate builder
initializes successfully after swapping the arguments back.

Closes #142
@tarcieri tarcieri mentioned this issue Jul 29, 2023
Merged
@tarcieri
Copy link
Member

Opened #143

tarcieri added a commit that referenced this issue Jul 29, 2023
@tarcieri
ssh-key: fix certificate::Builder::new_with_validity_times (#143)
3c4b5f3 
It previously had the `valid_after` and `valid_before` arguments
swapped, which would've caused errors with expected usage.

This commit adds a test that confirmed the certificate builder
initializes successfully after swapping the arguments back.

Closes #142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ssh-key: fix certificate::Builder::new_with_validity_times RustCrypto/SSH
2 participants
@tarcieri @pjht