-
Notifications
You must be signed in to change notification settings - Fork 98
Add support for FIPS for TlsKeyGenerator #289
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: master
Are you sure you want to change the base?
Conversation
git-svn-id: https://svn.apache.org/repos/asf/directory/apacheds/tags/2.0.0-M20@1676627 13f79535-47bb-0310-9956-ffa450edef68
Modify TlsKeyGenerator to support the BouncyCastle FIPS provider. Additional changes * key size expanded * methods that create X509 were modified
import java.security.spec.X509EncodedKeySpec; | ||
import java.util.Date; | ||
|
||
import javax.security.auth.x500.X500Principal; |
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.
I think we should keep the order of import as is (to conform with the code style of this repo)
/** | ||
* Generates the default RSA key pair for the server. | ||
* | ||
* @see https://github.com/apache/directory-server/blob/2.0.0-M20/core/src/main/java/org/apache/directory/server/core/security/TlsKeyGenerator.java |
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.
This comment is a bit superfluous I think, please remove this.
*/ | ||
public class TlsKeyGenerator | ||
@SuppressWarnings("all") | ||
public final class TlsKeyGenerator |
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.
Is there a reason to make this a final class?
static | ||
{ | ||
Security.addProvider( new BouncyCastleProvider() ); | ||
System.out.println("Using a modified version of TlsKeyGenerator to ensure Bouncy Castle FIPS provider is used."); |
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.
Please use LOG
for this.
* http://www.apache.org/licenses/exports | ||
*/ | ||
private static final int KEY_SIZE = 512; | ||
private static final int KEY_SIZE = 1024; |
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.
Is this change required?
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.
Yes FIPS requires 1024+ key size
<configuration> | ||
<source>1.7</source> | ||
<target>1.7</target> | ||
<source>1.8</source> |
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.
is this ok to keep 1.7?
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.
I couldn't compile any more unless this was set.
1.7 is not supported on my JDK.. Authors can reject this part of the contributions.
Thanks, will have a look at it! |
I mostly wanted to share my fixes for anyone else struggling with FIPS compliance -- I am not sure on the stance of FIPS generally for the repo. |
Some changes I made internally at our company to support FIPS.
We are on a pretty old version but upstreaming the changes if other people find it useful.