-
Notifications
You must be signed in to change notification settings - Fork 521
Output warnings for cert within 6 months expiry, check intermediate cert expiry #802
Conversation
0e019a5 to
7701deb
Compare
trustmanager/x509utils.go
Outdated
| } | ||
| // If this certificate is expiring within 6 months, put out a warning | ||
| if (c.NotAfter).Before(time.Now().AddDate(0, 6, 0)) { | ||
| logrus.Warn("certificate is expiring within 6 months") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what other info can we include to make it clear which certificate is expiring?
|
Thanks for checking the intermediate certs too! If it isn't too much trouble, can we add a check for the SHA1 algorithms too for the Other than that, this LGTM! |
|
@cyli of course, great idea! Done. |
84b60d2 to
e5957a4
Compare
…y in root Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
e5957a4 to
2952229
Compare
tuf/utils/x509.go
Outdated
| tomorrow := now.AddDate(0, 0, 1) | ||
| // Give one day leeway on creation "before" time, check "after" against today | ||
| if (tomorrow).Before(c.NotBefore) || now.After(c.NotAfter) { | ||
| return fmt.Errorf("certificate with CN %s is expired", c.Subject.CommonName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole if checkExpiry block should probably be moved to the end of the function. This return here will cause us to accept delegation certs in tuf/tuf.go line 255 that may have serious flaws but have expired and we returned from this validation prematurely, c.f. comment in tuf/tuf.go line 249 Currently we do not reject expired delegation keys but warn if they might expire soon or have already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, updated!
1c7d922 to
e146938
Compare
certs Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
e146938 to
fb03348
Compare
|
LGTM |
Addresses #734 for root certificates, and cleans up some of the certificate validation logic. Also ensures we use non-expired and non-SHA1 certificates for intermediate certs. Though there is no way to include specific certs to the root at the moment, we will need this validation.
Last commit closes #860
Signed-off-by: Riyaz Faizullabhoy riyaz.faizullabhoy@docker.com