Skip to content

Commit

Permalink
Move to proper feature checks
Browse files Browse the repository at this point in the history
SSLCipher was the first use of a feature which could appear in a later
version of NSS than we support. Rather than bumping the minimum NSS
version, we chose to use compile-time detection of the NSS version and
limit our code accordingly. However, this sets a precedence for ignoring
the features actually present in the NSS system. Certain downstream
distributions are fond of backporting features, which means our code
could've executed but didn't.

Switching to feature detection (via the check_struct_has_member macro)
allows us to be sure we execute this code on as many platforms as
possible.

Signed-off-by: Alexander Scheel <[email protected]>
  • Loading branch information
cipherboy committed Mar 23, 2020
1 parent a3a91a8 commit ed90754
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 9 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ build/

# These files are automatically generated from their .in equivalents
org/mozilla/jss/util/jssver.h
org/mozilla/jss/jssconfig.h
4 changes: 3 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ include(Shims)
# Since we found Java, include UseJava to provide the find_jar function.
include(UseJava)

# This include is required for the macro check_symbol_exists in jss_config()
# These includes are required for the macro check_symbol_exists and
# check_struct_has_member in jss_config().
include(CheckSymbolExists)
include(CheckStructHasMember)

# Load JSSConfig module; this defines the jss_config() macro which defines
# JSS-specific configuration values.
Expand Down
17 changes: 14 additions & 3 deletions cmake/JSSConfig.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ macro(jss_config)
# Configure test variables
jss_config_tests()

# Template auto-generated files
jss_config_template()

# Check symbols to see what tests we run
jss_config_symbols()

# Template auto-generated files
jss_config_template()
endmacro()

macro(jss_config_version MAJOR MINOR PATCH BETA)
Expand Down Expand Up @@ -325,6 +325,10 @@ endmacro()

macro(jss_config_template)
# Template files
configure_file(
"${PROJECT_SOURCE_DIR}/org/mozilla/jss/jssconfig.h.in"
"${PROJECT_SOURCE_DIR}/org/mozilla/jss/jssconfig.h"
)
configure_file(
"${PROJECT_SOURCE_DIR}/org/mozilla/jss/util/jssver.h.in"
"${PROJECT_SOURCE_DIR}/org/mozilla/jss/util/jssver.h"
Expand Down Expand Up @@ -397,6 +401,13 @@ macro(jss_config_symbols)
set(HAVE_NSS_KBKDF FALSE)
message(WARNING "Your NSS version is broken: between NSS v3.47 and v3.50, the values of CKM_AES_CMAC and CKM_AES_CMAC_GENERAL were swapped. Disabling CMAC and KBKDF support.")
endif()

check_struct_has_member(
SSLCipherSuiteInfo
kdfHash
ssl.h
HAVE_NSS_CIPHER_SUITE_INFO_KDFHASH
)
endmacro()

macro(jss_config_tests)
Expand Down
6 changes: 6 additions & 0 deletions org/mozilla/jss/jssconfig.h.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef _CONFIG_H
#define _CONFIG_H

#cmakedefine HAVE_NSS_CIPHER_SUITE_INFO_KDFHASH 1

#endif
13 changes: 8 additions & 5 deletions org/mozilla/jss/ssl/SSLCipher.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <pk11pub.h>

#include "_jni/org_mozilla_jss_ssl_SSLCipher.h"
#include "jssconfig.h"

/* Copied from NSS's ssl3con.c. */
static const CK_MECHANISM_TYPE auth_alg_defs[] = {
Expand Down Expand Up @@ -35,7 +36,7 @@ static const CK_MECHANISM_TYPE kea_alg_defs[] = {
};
PR_STATIC_ASSERT(PR_ARRAY_SIZE(kea_alg_defs) == ssl_kea_size);

#if (NSS_VMAJOR >= 3) && (NSS_VMINOR >= 43)
#ifdef HAVE_NSS_CIPHER_SUITE_INFO_KDFHASH
/* Not present in ssl3con.c. */
static const CK_MECHANISM_TYPE hash_alg_defs[] = {
CKM_INVALID_MECHANISM, /* ssl_hash_none */
Expand Down Expand Up @@ -109,10 +110,12 @@ Java_org_mozilla_jss_ssl_SSLCipher_checkSupportedStatus(JNIEnv *env, jclass claz
return JNI_FALSE;
}

/* Only check if NSS >= 3.43. Note that when this condition holds at
* compile time, and we're executing under an older NSS version, we'd
* have exited due to the check in SSL_GetCipherSuiteInfo(...). */
#if (NSS_VMAJOR >= 3) && (NSS_VMINOR >= 43)
/* Only check if NSS >= 3.43 or if this feature was backported. Note that
* when this condition holds at compile time, and we're executing under
* an older NSS version, we'd have exited due to the check in
* SSL_GetCipherSuiteInfo(...). That means that the value read here is
* always correct. */
#ifdef HAVE_NSS_CIPHER_SUITE_INFO_KDFHASH
if (info.kdfHash != ssl_hash_none &&
!PK11_TokenExists(hash_alg_defs[info.kdfHash])) {
return JNI_FALSE;
Expand Down

0 comments on commit ed90754

Please sign in to comment.