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

Added a check in case there's no openssl cipher that matches cipher #139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

beaudet
Copy link

@beaudet beaudet commented Jun 5, 2024

I neglected to fill out a description.

As an organization trying to stay on top of IT Security, we disable insecure ciphers and networking protocols for our organization. When using pyspark, spark calls out to the CipherSuiteConverter to attempt conversion of the requested cipher from openssl to a java name. When there's no match found in the openssl stack, it causes an NPE in spark. The actual fix is probably to configure java.security to disable the offending cipher, but the NPE shades the root cause so this pull request tries to provide more useful information to the developer by reporting the root cause rather than an NPE.

Additionally, it's pretty clear from this class and the lack of support for null being returned from the conversion reoutine, that only these ciphers are used with tls 1.3, but there seem to be situations where other ciphers are being used with 1.3.

For now, I will try setting the protocol and ciphers in spark to force the use of 1.3 with an AES cipher, but it seems like wildfly probably needs additional support for ciphers to work in situations where the information security office has disabled certain protocols and ciphers in the stack. Minimally, it would be helpful to produce useful error messages when an NPE is encountered. Thus, the simple P.R. but there's probably more work to be done regarding the cipher mappings.

static {
    Map<String, String> j2oTls13Map = new HashMap<>();
    j2oTls13Map.put("TLS_AES_128_GCM_SHA256", "TLS_AES_128_GCM_SHA256");
    j2oTls13Map.put("TLS_AES_256_GCM_SHA384", "TLS_AES_256_GCM_SHA384");
    j2oTls13Map.put("TLS_CHACHA20_POLY1305_SHA256", "TLS_CHACHA20_POLY1305_SHA256");
    j2oTls13Map.put("TLS_AES_128_CCM_SHA256", "TLS_AES_128_CCM_SHA256");
    j2oTls13Map.put("TLS_AES_128_CCM_8_SHA256", "TLS_AES_128_CCM_8_SHA256");
    j2oTls13 = Collections.unmodifiableMap(j2oTls13Map);

    Map<String, Map<String, String>> o2jTls13Map = new HashMap<>();
    o2jTls13Map.put("TLS_AES_128_GCM_SHA256", singletonMap("TLS", "TLS_AES_128_GCM_SHA256"));
    o2jTls13Map.put("TLS_AES_256_GCM_SHA384", singletonMap("TLS", "TLS_AES_256_GCM_SHA384"));
    o2jTls13Map.put("TLS_CHACHA20_POLY1305_SHA256", singletonMap("TLS", "TLS_CHACHA20_POLY1305_SHA256"));
    o2jTls13Map.put("TLS_AES_128_CCM_SHA256", singletonMap("TLS", "TLS_AES_128_CCM_SHA256"));
    o2jTls13Map.put("TLS_AES_128_CCM_8_SHA256", singletonMap("TLS", "TLS_AES_128_CCM_8_SHA256"));
    o2jTls13 = Collections.unmodifiableMap(o2jTls13Map);
}

that only these ciphers will be supported 

@beaudet beaudet requested review from fjuma and Skyllarr as code owners June 5, 2024 16:52
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.

1 participant