JNI/JSSE: Fix threading issues found by SpotBugs

- Fix inconsistent synchronization in WolfSSLParameters for
  getCipherSuites/setCipherSuites and
  getApplicationProtocols/setApplicationProtocols
- Add volatile to WolfSSLImplementSSLSession.isInTable and
  alpnSelector fields
- Use class lock for static fields in
  detectSecurityPropertySettings()
- Add synchronized to WolfSSLEngine.getEnabledCipherSuites()
pull/344/head
Chris Conlon 2026-03-18 11:48:19 -06:00
parent bd8606dcb5
commit fc867c33d0
6 changed files with 19 additions and 158 deletions

View File

@ -299,153 +299,4 @@
<Bug pattern="IS2_INCONSISTENT_SYNC"/>
</Match>
<!--
NM_METHOD_NAMING_CONVENTION: Public native JNI methods use
PascalCase to match native wolfSSL C function naming.
Renaming would break existing users and require updating
native JNI C code.
-->
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Bug pattern="NM_METHOD_NAMING_CONVENTION"/>
</Match>
<!--
MS_PKGPROTECT: Public API constants in WolfSSL documented
in Javadoc as values users pass to methods like
getExtensionSet(), setKeyUsage(), setPublicKey(), etc.
Reducing visibility would break users.
-->
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_surname"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_serialNumber"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_pkcs9_unstructuredName"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_pkcs9_contentType"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_pkcs9_challengePassword"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_givenName"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_initials"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_key_usage"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_subject_alt_name"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_basic_constraints"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_ext_key_usage"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="NID_dnQualifier"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="RSAk"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSL"/>
<Field name="ECDSAk"/>
<Bug pattern="MS_PKGPROTECT"/>
</Match>
<!--
FI_FINALIZER_NULLS_FIELDS: Finalizers null out fields as
defensive cleanup for JNI native resources. Nulling ensures
references to native pointers and shared objects don't
linger if something holds a reference to the finalized
object. The finalizers also do real work (freeSSL(),
store.clear()). Harmless and intentional.
-->
<Match>
<Class name="com.wolfssl.provider.jsse.WolfSSLAuthStore"/>
<Method name="finalize"/>
<Bug pattern="FI_FINALIZER_NULLS_FIELDS"/>
</Match>
<Match>
<Class name="com.wolfssl.provider.jsse.WolfSSLContext"/>
<Method name="finalize"/>
<Bug pattern="FI_FINALIZER_NULLS_FIELDS"/>
</Match>
<Match>
<Class name="com.wolfssl.provider.jsse.WolfSSLEngine"/>
<Method name="finalize"/>
<Bug pattern="FI_FINALIZER_NULLS_FIELDS"/>
</Match>
<Match>
<Class name=
"com.wolfssl.provider.jsse.WolfSSLEngineHelper"/>
<Method name="finalize"/>
<Bug pattern="FI_FINALIZER_NULLS_FIELDS"/>
</Match>
<Match>
<Class name="com.wolfssl.provider.jsse.WolfSSLSocket"/>
<Method name="finalize"/>
<Bug pattern="FI_FINALIZER_NULLS_FIELDS"/>
</Match>
<Match>
<Class name="com.wolfssl.provider.jsse.WolfSSLTrustX509"/>
<Method name="finalize"/>
<Bug pattern="FI_FINALIZER_NULLS_FIELDS"/>
</Match>
<!--
PA_PUBLIC_PRIMITIVE_ATTRIBUTE: WolfSSLDebug public debug
flags are public API for users to check/control debug
logging at runtime.
-->
<Match>
<Class name="com.wolfssl.WolfSSLDebug"/>
<Field name="DEBUG"/>
<Bug pattern="PA_PUBLIC_PRIMITIVE_ATTRIBUTE"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSLDebug"/>
<Field name="DEBUG_JNI"/>
<Bug pattern="PA_PUBLIC_PRIMITIVE_ATTRIBUTE"/>
</Match>
<Match>
<Class name="com.wolfssl.WolfSSLDebug"/>
<Field name="DEBUG_JSON"/>
<Bug pattern="PA_PUBLIC_PRIMITIVE_ATTRIBUTE"/>
</Match>
</FindBugsFilter>

View File

@ -237,7 +237,11 @@ public class WolfSSLSession {
if (readWritePoolDisabled()) {
this.byteBufferPoolEnabled = false;
}
}
/* Use class lock for static fields shared across all
* WolfSSLSession instances */
synchronized (WolfSSLSession.class) {
/* Check if the maximum size of the static per-thread direct
* ByteBuffer pool has been adjusted by setting of the
* "wolfssl.readWriteByteBufferPool.size" Security property. */

View File

@ -164,7 +164,8 @@ public class WolfSSLEngine extends SSLEngine {
private final Object toSendLock = new Object();
/** ALPN selector callback, if set */
protected BiFunction<SSLEngine, List<String>, String> alpnSelector = null;
protected volatile
BiFunction<SSLEngine, List<String>, String> alpnSelector = null;
/** Turn on extra/verbose SSLEngine debug logging */
private boolean extraDebugEnabled = false;
@ -2069,7 +2070,7 @@ public class WolfSSLEngine extends SSLEngine {
}
@Override
public String[] getEnabledCipherSuites() {
public synchronized String[] getEnabledCipherSuites() {
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
() -> "entered getEnabledCipherSuites()");
return this.engineHelper.getCiphers();

View File

@ -92,7 +92,7 @@ public class WolfSSLImplementSSLSession extends ExtendedSSLSession {
* true and the sesPtr is then freed by that object either during
* setResume() or finalization.
*/
protected boolean isInTable = false;
protected volatile boolean isInTable = false;
/**
* Tracks if WOLFSSL_SESSION pointer has been updated after retreived from

View File

@ -88,7 +88,11 @@ public class WolfSSLParameters extends SSLParameters {
}
}
/* create duplicate copy of these parameters */
/**
* Create duplicate copy of these parameters.
*
* @return new WolfSSLParameters copy of this object
*/
protected synchronized WolfSSLParameters copy() {
WolfSSLParameters cp = new WolfSSLParameters();
cp.setCipherSuites(this.cipherSuites);
@ -127,7 +131,7 @@ public class WolfSSLParameters extends SSLParameters {
}
@Override
public String[] getCipherSuites() {
public synchronized String[] getCipherSuites() {
if (this.cipherSuites == null) {
return null;
}
@ -135,7 +139,7 @@ public class WolfSSLParameters extends SSLParameters {
}
@Override
public void setCipherSuites(String[] cipherSuites) {
public synchronized void setCipherSuites(String[] cipherSuites) {
/* cipherSuites array is sanitized by wolfJSSE caller */
if (cipherSuites == null) {
this.cipherSuites = null;
@ -271,14 +275,14 @@ public class WolfSSLParameters extends SSLParameters {
* methods, which may not exist on older runtimes.
*/
public String[] getApplicationProtocols() {
public synchronized String[] getApplicationProtocols() {
if (this.applicationProtocols == null) {
return null;
}
return this.applicationProtocols.clone();
}
public void setApplicationProtocols(String[] protocols) {
public synchronized void setApplicationProtocols(String[] protocols) {
if (protocols == null) {
this.applicationProtocols = new String[0];
}

View File

@ -114,7 +114,8 @@ public class WolfSSLSocket extends SSLSocket {
private final Object initLock = new Object();
/** ALPN selector callback, if set */
protected BiFunction<SSLSocket, List<String>, String> alpnSelector = null;
protected volatile
BiFunction<SSLSocket, List<String>, String> alpnSelector = null;
/* true if client, otherwise false */
private boolean isClientMode = false;