-
Notifications
You must be signed in to change notification settings - Fork 471
WFCORE-5744 - improve keystore certificate health check #6420
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thank you for the pull request, I have added some comments specific to this pull request but I think we also need to step back and get the WildFly feature development process started so we can begin to discuss the general proposed solution to the problem.
--> | ||
|
||
<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" | ||
targetNamespace="urn:wildfly:elytron:community:18.1" |
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.
Need to think about what is the correct updates needed here but we avoid minor bumps.
elytron/src/main/java/org/wildfly/extension/elytron/CertificateChainAttributeDefinitions.java
Show resolved
Hide resolved
elytron/src/main/java/org/wildfly/extension/elytron/ElytronExtension.java
Outdated
Show resolved
Hide resolved
//delay in ms between service start and periodical check of certificate health | ||
//if set to '0', this will mean one-time off warning, as prior to RFE | ||
private long expirationCheckDelay = DEFAULT_DELAY; | ||
private ScheduledExecutorService scheduledExecutorService; |
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.
FYI as part of the CVE I am working on I am adding the scheduled executor service to the root of the Elytron subsystem so all Elytron services can share the same executor.
pathResolver = null; | ||
} | ||
} finally { | ||
this.scheduledExecutorService.shutdownNow(); |
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.
Looking at how this is handled for the executors elsewhere a more complete shutdown solution is needed but the shared executor I am adding will have that,
final CertificateValidity certificateValidity = CertificateValidity.getValidity(xCertificate.getNotBefore(), xCertificate.getNotAfter()); | ||
switch(certificateValidity) { | ||
case ABOUT_TO_EXPIRE: | ||
ROOT_LOGGER.certificateAboutToExpire(keyStoreName, alias); |
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.
We probably need a mechanism to avoid spamming the logs for the same certificate, especially if they want a frequent check.
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.
hmm, whats the point of check than ?
elytron/src/main/java/org/wildfly/extension/elytron/CertificateValidity.java
Outdated
Show resolved
Hide resolved
elytron/src/main/java/org/wildfly/extension/elytron/KeyStoreDefinition.java
Show resolved
Hide resolved
f90129f
to
8a54d77
Compare
8a54d77
to
7445a58
Compare
Core -> Full Integration Build 14455 outcome was FAILURE using a merge of 7445a58 |
Core -> Full Integration Build 14461 outcome was UNKNOWN using a merge of 7445a58 |
Issue: https://issues.redhat.com/browse/WFCORE-5744
Proposal: https://issues.redhat.com/browse/EAP7-1863