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.

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

SelfSignedCertificate configurable valid dates #4257

Closed
wants to merge 1 commit into from

Conversation

Scottmitch
Copy link
Member

Motivation:
Users may want to control the valid dates for SelfSignedCertificate.

Modifications:

  • Allow NOT_BEFORE and NOT_AFTER to be controlled via java system properties.

Result:
Fixes #3978

@Scottmitch Scottmitch self-assigned this Sep 23, 2015
@Scottmitch Scottmitch added this to the 4.0.32.Final milestone Sep 23, 2015
@Scottmitch
Copy link
Member Author

@normanmaurer @ganskef - FYI.

@@ -59,9 +60,11 @@
private static final InternalLogger logger = InternalLoggerFactory.getInstance(SelfSignedCertificate.class);

/** Current time minus 1 year, just in case software clock goes back due to time synchronization */
static final Date NOT_BEFORE = new Date(System.currentTimeMillis() - 86400000L * 365);
static final Date NOT_BEFORE = new Date(SystemPropertyUtil.getLong(
"io.netty.SelfSignedCertificate.notBefore", System.currentTimeMillis() - 86400000L * 365));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self -> self?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@trustin
Copy link
Member

trustin commented Sep 23, 2015

How about renaming notBefore/After to defaultNotBefore/After and allow a user to specify an alternative notBefore/After value via constructor?

Motivation:
Users may want to control the valid dates for SelfSignedCertificate.

Modifications:
- Allow NOT_BEFORE and NOT_AFTER to be controlled via java system properties.

Result:
Fixes netty#3978
@Scottmitch
Copy link
Member Author

@trustin - Good idea. Done.

@Scottmitch
Copy link
Member Author

Cherry-picked 4.0 (8bc3964, 127886f) 4.1 (c116c35, c471065) master (db0fdfe, aa0e12d)

* @param notBefore Certificate is not valid before this time
* @param notAfter Certificate is not valid after this time
*/
public SelfSignedCertificate(Date notBefore, Date notAfter) throws CertificateException {
this("example.com");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like notBefore and notAfter are unused?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yip. Will fix.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a new set of commits. See #4257 (comment)

Scottmitch added a commit that referenced this pull request Sep 25, 2015
Motivation:
PR #4257 introduced paramters and didn't use them.

Modifications:
- Use the new paramters

Result:
No warnings and correct behavior
Scottmitch added a commit that referenced this pull request Sep 25, 2015
Motivation:
PR #4257 introduced paramters and didn't use them.

Modifications:
- Use the new paramters

Result:
No warnings and correct behavior
Scottmitch added a commit that referenced this pull request Sep 25, 2015
Motivation:
PR #4257 introduced paramters and didn't use them.

Modifications:
- Use the new paramters

Result:
No warnings and correct behavior
@Scottmitch
Copy link
Member Author

@trustin Any thoughts on this test failure? I've seen it a few times now.

@trustin
Copy link
Member

trustin commented Sep 25, 2015

@Scottmitch Some DNS servers were returning NoError for non-existent domains. I removed them from the public DNS server list. Will keep my eyes on the failure..

@normanmaurer
Copy link
Member

@trustin @Scottmitch I think it makes sense to use a "stub" DnsServer for our tests. I will work on this and so make it more robust.

@Scottmitch
Copy link
Member Author

@normanmaurer +1. It would be nice to have isolation in our unit tests that automatically run. It would be nice for dev to switch to the "real" DNS servers too though.

pulllock pushed a commit to pulllock/netty that referenced this pull request Oct 19, 2023
Motivation:
PR netty#4257 introduced paramters and didn't use them.

Modifications:
- Use the new paramters

Result:
No warnings and correct behavior
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants