Adds expiry parser with ability to specify hours, days, months and years #47
Conversation
|
|
|
Hi @mikesimons, thank you for your contribution! Looking at the code now. |
cmd/expiry.go
Outdated
| "hour": 0, | ||
| } | ||
| for _, r := range matches { | ||
| addDate[r[2]], _ = strconv.Atoi(r[1]) |
There was a problem hiding this comment.
Why not check the error here?
There was a problem hiding this comment.
r[1] should have been matched from the first group in the regex which is \d+ so we can say with fair certainty that it's going to be a contiguous string of integers.
Thinking about it though Atoi could still fail with an integer overflow (on e.g. "999999999999 days"). Should also probably check for signed int overflow because Atoi would not error in that case but the result is not valid in this context.
I will add those checks.
cmd/expiry.go
Outdated
| now := nowFunc().UTC() | ||
| result := now. | ||
| AddDate(addDate["year"], addDate["month"], addDate["day"]). | ||
| Add(time.Hour * time.Duration(addDate["hour"])) |
There was a problem hiding this comment.
Nit, but I would probably reverse this to make it more readable (i.e. put time.Hour at the end of the multiplication).
cmd/init.go
Outdated
| @@ -37,7 +37,8 @@ func NewInitCommand() cli.Command { | |||
| Flags: []cli.Flag{ | |||
| cli.StringFlag{"passphrase", "", "Passphrase to encrypt private-key PEM block", ""}, | |||
| cli.IntFlag{"key-bits", 4096, "Bit size of RSA keypair to generate", ""}, | |||
| cli.IntFlag{"years", 10, "How long until the CA certificate expires", ""}, | |||
| cli.IntFlag{"years", 0, "DEPRECATED; Use --expires instead", ""}, | |||
| cli.StringFlag{"expires", "10 years", "How long until the certificate expires. Example: 1 year 2 days 3 months 4 hours", ""}, | |||
There was a problem hiding this comment.
We could change this to 18 months as a default while we're at it. @mcpherrinm what do you think?
There was a problem hiding this comment.
Maybe default to 1 year?
There was a problem hiding this comment.
Happy to change to whatever you guys want; I was maintaining existing behaviour as far as possible.
mikesimons
force-pushed
the
finer-expiry
branch
from
5815355 to
f06f3ae
Compare
mikesimons
force-pushed
the
finer-expiry
branch
from
f06f3ae to
8a56a6e
Compare
|
@csstaub I think I've covered most of the points you raised. Golang date manipulation is not as sturdy as I previously thought and I ran in to time.Duration overflows on the hours unit. This caused expiry dates circa 1800 with some values!! Some additional test cases also highlighted the fact that an expiry beyond the year 9999 is not supported by ASN.1! This is where the extra checks came from. During testing I also found it better for the expiry parsing to happen before everything else to avoid only half of the files being created (requiring manual cleanup). |
|
@mikesimons Looks good, thank you so much for your contribution! |
|
We'll make sure to release a new version with these changes included soon. |
We want to to use certstrap to issue certs with a much lower expiry than 1 year.
This PR replaces
--yearswith--expireswhich accepts a simple relative format.The format is
<VALUE> <UNIT> <VALUE UNIT> ....<VALUE>must be an integer.<UNIT>may be one ofyear, month, day, hour. Plural forms of the units are also accepted (ie.years, months, days, hours) and are equivalent to using the singular.There is no validation on the number of times a unit may appear but only the last will be used.
Anything that does not match the format will be discarded.
If the format is entirely invalid then the error
Invalid expiry formatwill be shown.Examples:
certstrap init --common-name test --expires "2 years 1 day 9 hours"cerstrap sign test --CA test --expires "1 month"