Skip to content

Add new method authenticationMethodsPacked to avoid string and array allocation#968

Open
doom369 wants to merge 1 commit into
netty:mainfrom
doom369:add_new_auth_methods_packed
Open

Add new method authenticationMethodsPacked to avoid string and array allocation#968
doom369 wants to merge 1 commit into
netty:mainfrom
doom369:add_new_auth_methods_packed

Conversation

@doom369

@doom369 doom369 commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Currently, authenticationMethods() allocates multiple strings and a string array during initial connection. This could be avoided with a new method, where we pack all auth methods into a single int. This will also simplify code on the Netty side (2 switch statements could be removed in OpenSslKeyMaterialManager.setKeyMaterialServerSide).

image

* <li>{@code 0x0001} (SSL_aRSA) - RSA authentication (RSA, DHE_RSA, ECDHE_RSA)</li>
* <li>{@code 0x0002} (SSL_aDSS) - DSS authentication (DHE_DSS)</li>
* <li>{@code 0x0004} (SSL_aNULL) - no auth / anonymous (DH_anon, ECDH_anon)</li>
* <li>{@code 0x0040} (SSL_aECDSA) - ECDSA authentication (ECDHE_ECDSA)</li>

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.

I'm wondering is if we should expose these constants through static fields in SSL, initialized from NativeStaticallyReferencedJniMethods calls.

@doom369 doom369 Apr 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea actually, minimizes the chance for error.

#define TCN_SSL_aDSS 0x00000002L
#define TCN_SSL_aNULL 0x00000004L
#define TCN_SSL_aECDSA 0x00000040L
#define TCN_SSL_aALL (TCN_SSL_aRSA | TCN_SSL_aDSS | TCN_SSL_aNULL | TCN_SSL_aECDSA)

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.

Not great to define them again. Maybe they can be moved to ssl_private.h?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll check

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.

2 participants