Skip to content

Add support for password-based encryption scheme 2 params (PBES2)#13539

Merged
normanmaurer merged 4 commits intonetty:4.1from
xiezhaokun:issue-13536
Aug 22, 2023
Merged

Add support for password-based encryption scheme 2 params (PBES2)#13539
normanmaurer merged 4 commits intonetty:4.1from
xiezhaokun:issue-13536

Conversation

@xiezhaokun
Copy link
Copy Markdown
Contributor

Motivation:

Add support for password-based encryption scheme 2 params (PBES2)

Modification:

Describe the modifications you've done.

Result:

Fixes #13536

If there is no issue then describe the changes introduced by this PR.

@hyperxpro
Copy link
Copy Markdown
Contributor

Please also add unit tests.

@normanmaurer
Copy link
Copy Markdown
Member

@xiezhaokun as @hyperxpro said... can you please add a unit test ?

private final boolean startTls;
private final AttributeMap attributes = new DefaultAttributeMap();
private static final String OID_PKCS5_PBES2 = "1.2.840.113549.1.5.13";
private static final String PBES2 = "PBES2";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you add a comment to explain where these values come from ?

@chrisvest
Copy link
Copy Markdown
Member

Sounds like PKCS12 is a bit of a messy landscape but I'm told PBES2 is the "state of the art" there.

A good test for this would be to generate a .p12 file with openssl, that uses PBES2, and verify that the code can load it and use it. Also include the shell script that generated the file, so we don't loose that information.

@normanmaurer
Copy link
Copy Markdown
Member

@xiezhaokun @chrisvest I added a test-case

@normanmaurer normanmaurer added this to the 4.1.97.Final milestone Aug 21, 2023
@normanmaurer
Copy link
Copy Markdown
Member

/cc @hyperxpro

private static String getPBEAlgorithm(EncryptedPrivateKeyInfo encryptedPrivateKeyInfo) {
AlgorithmParameters parameters = encryptedPrivateKeyInfo.getAlgParameters();
String algName = encryptedPrivateKeyInfo.getAlgName();
// Java 8 ~ 16 returns OID_PKCS5_PBES2
Copy link
Copy Markdown
Contributor

@hyperxpro hyperxpro Aug 21, 2023

Choose a reason for hiding this comment

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

Does this mean Java < 8 are not supported?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hyperxpro good point... maybe we should just add a version check as well. Let me do this

@hyperxpro
Copy link
Copy Markdown
Contributor

@chrisvest

Error:  Failures: 
Error:    SingleThreadEventLoopTest.scheduleLaggyTaskAtFixedRateB:236->testScheduleLaggyTaskAtFixedRate:276 
Expected: is a value less than or equal to <10000000L>
     but: <113421600L> was greater than <10000000L>

Copy link
Copy Markdown
Contributor

@hyperxpro hyperxpro left a comment

Choose a reason for hiding this comment

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

:)

@normanmaurer normanmaurer merged commit a026444 into netty:4.1 Aug 22, 2023
normanmaurer added a commit that referenced this pull request Aug 22, 2023
…3539)

Motivation:

Add support for password-based encryption scheme 2 params (PBES2)

Modification:

Describe the modifications you've done.

Result:

Fixes #13536

Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Co-authored-by: Chris Vest <christianvest_hansen@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for password-based encryption scheme 2 params (PBES2)

4 participants