config: LoadConfig panics when not_after is before not_before
Maintainers usually reply within 2 days
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 25/100
Research direction
Start in config/config.go at SigningProfile.populate, around the not_before and not_after validation identified in the panic trace. Check how nearby invalid profile values construct and return InvalidPolicy errors, then inspect the related config tests. Done means LoadConfig returns an InvalidPolicy error for the supplied reversed dates without panicking.
Written by the indexing model from the issue text.
Description
What happens
Loading a signing config whose profile sets not_after earlier than not_before panics instead of returning an error:
panic: Wrap needs a supplied error to initialize.
github.com/cloudflare/cfssl/errors.Wrap(...)
errors/error.go:407
github.com/cloudflare/cfssl/config.(*SigningProfile).populate(...)
config/config.go:217
github.com/cloudflare/cfssl/config.LoadConfig(...)
config/config.go:721
cfssl serve -config, cfssl gencert -config and any library caller of config.LoadFile / config.LoadConfig crash on such a file.
What should happen
LoadConfig should return an InvalidPolicy error, like it does for other invalid profile values.
Minimal repro
_, err := config.LoadConfig([]byte(`{
"signing": {
"default": {
"usages": ["digital signature"],
"expiry": "8760h",
"not_before": "2030-01-01T00:00:00Z",
"not_after": "2020-01-01T00:00:00Z"
}
}
}`))
Root cause
In SigningProfile.populate the check is:
if !p.NotBefore.IsZero() && !p.NotAfter.IsZero() && p.NotAfter.Before(p.NotBefore) {
return cferr.Wrap(cferr.PolicyError, cferr.InvalidPolicy, err)
}
err at that point is the result of the successful time.ParseDuration calls above it, so it is always nil, and cferr.Wrap panics when given a nil error. The check needs its own error value.
- Dominant language
- Go
- Stars
- 9.5k
- Forks
- 1.2k
- Avg merge
- 12h 48m
- Merged PRs (30d)
- 2
Getting set up
- Ships a Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from cloudflare/cfssl
-
gencrl: serial numbers that fail to parse are added to the CRL as 0 or a partial valuePossibly taken @sebxt claimed this 2 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
cloudflare/cfssl#1460 ·
Maintainers usually reply within 2 days
-
`Bundler.BundleFromRemote` fails for IPv6 address literalsPossibly taken @reginaldalfret claimed this 15 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
cloudflare/cfssl#1448 ·
Maintainers usually reply within 2 days
-
crl_override (and extensions) in /api/v1/sign request body are silently ignored by the HTTP APIPossibly taken @Hashim1999164 claimed this 70 days ago. Open
Difficulty 2/5 Half a day Newbie friendliness 78/100
cloudflare/cfssl#1439 ·
Maintainers usually reply within 2 days
-
gencrl silently revokes the wrong serial number when a line is not plain decimalPossibly taken @RaphaelFakhri claimed this 3 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 30/100
cloudflare/cfssl#1458 ·
Maintainers usually reply within 2 days
-
Difficulty 3/5 1-2 days Newbie friendliness 48/100
cloudflare/cfssl#1446 ·
Maintainers usually reply within 2 days
All issues in cloudflare/cfssl
Similar issues
-
native
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
prime-radiant-inc/evener#4839 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100
521xueweihan/HelloGitHub#4073 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 66/100
goharbor/terraform-provider-harbor#655 · 1 comment ·