You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "lordgamez (via GitHub)" <gi...@apache.org> on 2025/02/06 13:02:11 UTC
[PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
lordgamez opened a new pull request, #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925
This change adds the option to enable FIPS mode in the OpenSSL library for cryptographic functions used by MiNiFi C++. This includes the following:
- Introduces the `nifi.openssl.fips.support.enable` option in `minifi.properties` to enable this option, if the option is enabled MiNiFi C++ will try to load the `openssl.cnf` that loads the FIPS provider and then enables FIPS mode in OpenSSL.
- Adds compilation of the FIPS provider library from OpenSSL 3.0.9 which is the latest FIPS validated OpenSSL version as listed here: https://openssl-library.org/source/
- Adds the FIPS provider library and the required `openssl.cnf` files to the install package under the $MINIFI_HOME/fips directory
- Adds the `openssl` binary to the install package under the $MINIFI_HOME/fips directory for running the module tests and generating the fipsmodule.cnf file on the target platform as required for the FIPS compliancy, referenced in these sources: https://github.com/openssl/openssl/discussions/25036, https://openssl-library.org/source/fips-doc/openssl-3.0.9-security-policy-2024-01-12.pdf Appendix A
- Updated paho-mqtt, librdkafka libraries and added `no-engine` flag for OpenSSL compilation to remove legacy API usage required for FIPS compliancy as referenced here: https://docs.openssl.org/master/man7/fips_module/#description
OpenSSL discussion about this issue: https://github.com/openssl/openssl/discussions/26378
https://issues.apache.org/jira/browse/MINIFICPP-2524
---------------------
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1987659275
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -165,4 +169,77 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set_property(TARGET OpenSSL::SSL APPEND PROPERTY INTERFACE_LINK_LIBRARIES crypt32.lib)
endif()
+ if (WIN32)
+ set(BYPRODUCT_DYN_SUFFIX ".dll" CACHE STRING "" FORCE)
+ elseif(APPLE)
+ set(BYPRODUCT_DYN_SUFFIX ".dylib" CACHE STRING "" FORCE)
+ else()
+ set(BYPRODUCT_DYN_SUFFIX ".so" CACHE STRING "" FORCE)
+ endif()
+
+ set(FIPS_BYPRODUCTS
+ "${LIBDIR}/ossl-modules/fips${BYPRODUCT_DYN_SUFFIX}"
+ )
+
+ set(OPENSSL_FIPS_BIN_DIR "${BINARY_DIR}/thirdparty/openssl-fips-install" CACHE STRING "" FORCE)
+
+ FOREACH(BYPRODUCT ${FIPS_BYPRODUCTS})
+ LIST(APPEND OPENSSL_FIPS_FILE_LIST "${OPENSSL_FIPS_BIN_DIR}/${BYPRODUCT}")
+ ENDFOREACH(BYPRODUCT)
+
+ install(FILES ${OPENSSL_FIPS_FILE_LIST} DESTINATION fips COMPONENT bin)
+ install(FILES "${OPENSSL_BIN_DIR}/bin/openssl${EXECUTABLE_SUFFIX}" DESTINATION fips COMPONENT bin
+ PERMISSIONS OWNER_EXECUTE OWNER_WRITE OWNER_READ GROUP_EXECUTE GROUP_READ WORLD_READ WORLD_EXECUTE)
+
+ set(OPENSSL_FIPS_EXTRA_FLAGS
+ no-tests # Disable tests
+ no-capieng # disable CAPI engine (legacy)
+ no-legacy # disable legacy modules
+ no-ssl # disable SSLv3
+ no-engine # disable Engine API as it is deprecated since OpenSSL 3.0 and not FIPS compatible
+ enable-fips) # enable FIPS module
+
+ if (WIN32)
+ find_program(JOM_EXECUTABLE_PATH
+ NAMES jom.exe
+ PATHS ENV PATH
+ NO_DEFAULT_PATH)
+ if(JOM_EXECUTABLE_PATH)
+ include(ProcessorCount)
+ processorcount(jobs)
+ set(OPENSSL_BUILD_COMMAND ${JOM_EXECUTABLE_PATH} -j${jobs})
+ set(OPENSSL_WINDOWS_COMPILE_FLAGS /FS)
+ else()
+ message("Using nmake for OpenSSL build")
+ set(OPENSSL_BUILD_COMMAND nmake)
+ set(OPENSSL_WINDOWS_COMPILE_FLAGS "")
+ endif()
+ ExternalProject_Add(
+ openssl-fips-external
+ URL https://github.com/openssl/openssl/releases/download/openssl-3.0.9/openssl-3.0.9.tar.gz
+ URL_HASH "SHA256=eb1ab04781474360f77c318ab89d8c5a03abc38e63d65a603cabbf1b00a1dc90"
+ SOURCE_DIR "${BINARY_DIR}/thirdparty/openssl-fips-src"
+ BUILD_IN_SOURCE true
+ CONFIGURE_COMMAND perl Configure "CFLAGS=${PASSTHROUGH_CMAKE_C_FLAGS} ${OPENSSL_WINDOWS_COMPILE_FLAGS}" "CXXFLAGS=${PASSTHROUGH_CMAKE_CXX_FLAGS} ${OPENSSL_WINDOWS_COMPILE_FLAGS}" ${OPENSSL_SHARED_FLAG} ${OPENSSL_FIPS_EXTRA_FLAGS} enable-fips "--prefix=${OPENSSL_FIPS_BIN_DIR}" "--openssldir=${OPENSSL_FIPS_BIN_DIR}"
+ BUILD_BYPRODUCTS ${OPENSSL_FIPS_FILE_LIST}
+ EXCLUDE_FROM_ALL TRUE
+ BUILD_COMMAND ${OPENSSL_BUILD_COMMAND}
+ INSTALL_COMMAND nmake install_fips
+ )
+ else()
+ ExternalProject_Add(
+ openssl-fips-external
+ URL https://github.com/openssl/openssl/releases/download/openssl-3.0.9/openssl-3.0.9.tar.gz
+ URL_HASH "SHA256=eb1ab04781474360f77c318ab89d8c5a03abc38e63d65a603cabbf1b00a1dc90"
+ SOURCE_DIR "${BINARY_DIR}/thirdparty/openssl-fips-src"
+ BUILD_IN_SOURCE true
+ CONFIGURE_COMMAND ./Configure "CFLAGS=${PASSTHROUGH_CMAKE_C_FLAGS} -fPIC" "CXXFLAGS=${PASSTHROUGH_CMAKE_CXX_FLAGS} -fPIC" ${OPENSSL_SHARED_FLAG} ${OPENSSL_FIPS_EXTRA_FLAGS} "--prefix=${OPENSSL_FIPS_BIN_DIR}" "--openssldir=${OPENSSL_FIPS_BIN_DIR}"
+ BUILD_BYPRODUCTS ${OPENSSL_FIPS_FILE_LIST}
+ EXCLUDE_FROM_ALL TRUE
+ INSTALL_COMMAND make install_fips
+ )
+ endif()
+
+ add_dependencies(OpenSSL::Crypto openssl-fips-external)
Review Comment:
is this necessary? I thought these two could be built in parallel.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983282622
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
- no-apps is removed because we need the openssl binary to be packaged with fips
- no-dso and no-module need to be enabled for shared libraries to be loaded in our case the fips provider which cannot be statically linked
- IMO no-pinshared should not be enabled to have the loaded fips provider available through the lifetime of the application and avoid the accidental unloading of the fips shared library
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983442423
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
aah ok. I thought both the binary and the fips module come from the fips target.
What about `no-pinshared`, is that necessary too?
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
aah ok. I thought both the binary and the fips module come from the fips target.
What about `no-pinshared`, is removing that necessary too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983241697
##########
minifi_main/MiNiFiMain.cpp:
##########
Review Comment:
I don't think `utils` is the place for that. It's only relevant for the agent process. `libminifi` (which seems to be agent process stuff) or `minifi_main` (which is another place for the same) would be more appropriate IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983241697
##########
minifi_main/MiNiFiMain.cpp:
##########
Review Comment:
I don't think `utils` is the place for that. It's only relevant for the agent process.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1987684401
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -165,4 +169,77 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set_property(TARGET OpenSSL::SSL APPEND PROPERTY INTERFACE_LINK_LIBRARIES crypt32.lib)
endif()
+ if (WIN32)
+ set(BYPRODUCT_DYN_SUFFIX ".dll" CACHE STRING "" FORCE)
+ elseif(APPLE)
+ set(BYPRODUCT_DYN_SUFFIX ".dylib" CACHE STRING "" FORCE)
+ else()
+ set(BYPRODUCT_DYN_SUFFIX ".so" CACHE STRING "" FORCE)
+ endif()
+
+ set(FIPS_BYPRODUCTS
+ "${LIBDIR}/ossl-modules/fips${BYPRODUCT_DYN_SUFFIX}"
+ )
+
+ set(OPENSSL_FIPS_BIN_DIR "${BINARY_DIR}/thirdparty/openssl-fips-install" CACHE STRING "" FORCE)
+
+ FOREACH(BYPRODUCT ${FIPS_BYPRODUCTS})
+ LIST(APPEND OPENSSL_FIPS_FILE_LIST "${OPENSSL_FIPS_BIN_DIR}/${BYPRODUCT}")
+ ENDFOREACH(BYPRODUCT)
+
+ install(FILES ${OPENSSL_FIPS_FILE_LIST} DESTINATION fips COMPONENT bin)
+ install(FILES "${OPENSSL_BIN_DIR}/bin/openssl${EXECUTABLE_SUFFIX}" DESTINATION fips COMPONENT bin
+ PERMISSIONS OWNER_EXECUTE OWNER_WRITE OWNER_READ GROUP_EXECUTE GROUP_READ WORLD_READ WORLD_EXECUTE)
+
+ set(OPENSSL_FIPS_EXTRA_FLAGS
+ no-tests # Disable tests
+ no-capieng # disable CAPI engine (legacy)
+ no-legacy # disable legacy modules
+ no-ssl # disable SSLv3
+ no-engine # disable Engine API as it is deprecated since OpenSSL 3.0 and not FIPS compatible
+ enable-fips) # enable FIPS module
+
+ if (WIN32)
+ find_program(JOM_EXECUTABLE_PATH
+ NAMES jom.exe
+ PATHS ENV PATH
+ NO_DEFAULT_PATH)
+ if(JOM_EXECUTABLE_PATH)
+ include(ProcessorCount)
+ processorcount(jobs)
+ set(OPENSSL_BUILD_COMMAND ${JOM_EXECUTABLE_PATH} -j${jobs})
+ set(OPENSSL_WINDOWS_COMPILE_FLAGS /FS)
+ else()
+ message("Using nmake for OpenSSL build")
+ set(OPENSSL_BUILD_COMMAND nmake)
+ set(OPENSSL_WINDOWS_COMPILE_FLAGS "")
+ endif()
+ ExternalProject_Add(
+ openssl-fips-external
+ URL https://github.com/openssl/openssl/releases/download/openssl-3.0.9/openssl-3.0.9.tar.gz
+ URL_HASH "SHA256=eb1ab04781474360f77c318ab89d8c5a03abc38e63d65a603cabbf1b00a1dc90"
+ SOURCE_DIR "${BINARY_DIR}/thirdparty/openssl-fips-src"
+ BUILD_IN_SOURCE true
+ CONFIGURE_COMMAND perl Configure "CFLAGS=${PASSTHROUGH_CMAKE_C_FLAGS} ${OPENSSL_WINDOWS_COMPILE_FLAGS}" "CXXFLAGS=${PASSTHROUGH_CMAKE_CXX_FLAGS} ${OPENSSL_WINDOWS_COMPILE_FLAGS}" ${OPENSSL_SHARED_FLAG} ${OPENSSL_FIPS_EXTRA_FLAGS} enable-fips "--prefix=${OPENSSL_FIPS_BIN_DIR}" "--openssldir=${OPENSSL_FIPS_BIN_DIR}"
+ BUILD_BYPRODUCTS ${OPENSSL_FIPS_FILE_LIST}
+ EXCLUDE_FROM_ALL TRUE
+ BUILD_COMMAND ${OPENSSL_BUILD_COMMAND}
+ INSTALL_COMMAND nmake install_fips
+ )
+ else()
+ ExternalProject_Add(
+ openssl-fips-external
+ URL https://github.com/openssl/openssl/releases/download/openssl-3.0.9/openssl-3.0.9.tar.gz
+ URL_HASH "SHA256=eb1ab04781474360f77c318ab89d8c5a03abc38e63d65a603cabbf1b00a1dc90"
+ SOURCE_DIR "${BINARY_DIR}/thirdparty/openssl-fips-src"
+ BUILD_IN_SOURCE true
+ CONFIGURE_COMMAND ./Configure "CFLAGS=${PASSTHROUGH_CMAKE_C_FLAGS} -fPIC" "CXXFLAGS=${PASSTHROUGH_CMAKE_CXX_FLAGS} -fPIC" ${OPENSSL_SHARED_FLAG} ${OPENSSL_FIPS_EXTRA_FLAGS} "--prefix=${OPENSSL_FIPS_BIN_DIR}" "--openssldir=${OPENSSL_FIPS_BIN_DIR}"
+ BUILD_BYPRODUCTS ${OPENSSL_FIPS_FILE_LIST}
+ EXCLUDE_FROM_ALL TRUE
+ INSTALL_COMMAND make install_fips
+ )
+ endif()
+
+ add_dependencies(OpenSSL::Crypto openssl-fips-external)
Review Comment:
This is needed to actually build the fips target. Without setting it as a dependency of any target in the build (as it is used only in runtime), it will not be built because no target has this library linked in build time and cmake will not make it part of the build hierarchy. The two openssl builds can actually run in parallel as it does not depend on the openssl-external target. OpenSSL::Crypto depends on both openssl-external and openssl-fips-external targets so they can be built in parallel.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1977560842
##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -183,6 +187,100 @@ void writeSchemaIfRequested(const argparse::ArgumentParser& parser, const std::s
std::exit(0);
}
+bool replaceMinifiHomeVariable(const std::filesystem::path& file_path, std::string minifi_home_path, const std::shared_ptr<core::logging::Logger>& logger) {
+ std::ifstream input_file(file_path);
+ if (!input_file) {
+ logger->log_error("Failed to open file: {}", file_path.string());
+ return false;
+ }
+
+ std::ostringstream buffer;
+ buffer << input_file.rdbuf();
+ std::string content = buffer.str();
+ input_file.close();
+
+ const std::string placeholder = "${MINIFI_HOME}";
+ size_t pos = content.find(placeholder, 0);
+ if (pos == std::string::npos) {
+ return true;
+ }
+
+ std::replace(minifi_home_path.begin(), minifi_home_path.end(), '\\', '/');
Review Comment:
Instead of this, we could pass in `minifi_home` as a `const std::filesystem::path &`, and use `generic_string()`.
##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -183,6 +187,100 @@ void writeSchemaIfRequested(const argparse::ArgumentParser& parser, const std::s
std::exit(0);
}
+bool replaceMinifiHomeVariable(const std::filesystem::path& file_path, std::string minifi_home_path, const std::shared_ptr<core::logging::Logger>& logger) {
+ std::ifstream input_file(file_path);
+ if (!input_file) {
+ logger->log_error("Failed to open file: {}", file_path.string());
+ return false;
+ }
+
+ std::ostringstream buffer;
+ buffer << input_file.rdbuf();
+ std::string content = buffer.str();
+ input_file.close();
+
+ const std::string placeholder = "${MINIFI_HOME}";
+ size_t pos = content.find(placeholder, 0);
+ if (pos == std::string::npos) {
+ return true;
+ }
+
+ std::replace(minifi_home_path.begin(), minifi_home_path.end(), '\\', '/');
+ do {
+ content.replace(pos, placeholder.length(), minifi_home_path);
+ pos += minifi_home_path.length();
+ } while((pos = content.find(placeholder, pos)) != std::string::npos);
+
+ std::ofstream output_file(file_path);
+ if (!output_file) {
+ logger->log_error("Failed to open file for writing: {}", file_path.string());
+ return false;
+ }
+
+ output_file << content;
+ output_file.close();
+ return true;
+}
+
+void initializeFipsMode(const std::shared_ptr<minifi::Configure>& configure, const std::filesystem::path& minifi_home, const std::shared_ptr<core::logging::Logger>& logger) {
+ if (!(configure->get(minifi::Configure::nifi_openssl_fips_support_enable) | utils::andThen(utils::string::toBool)).value_or(false)) {
+ return;
+ }
+
+#ifdef WIN32
+ static constexpr std::string_view FIPS_LIB = "fips.dll";
+#elif defined(__APPLE__)
+ static constexpr std::string_view FIPS_LIB = "fips.dylib";
+#else
+ static constexpr std::string_view FIPS_LIB = "fips.so";
+#endif
+
+ if (!std::filesystem::exists(minifi_home / "fips" / FIPS_LIB)) {
+ logger->log_error("FIPS mode is enabled, but {} is not available in MINIFI_HOME/fips directory", FIPS_LIB);
+ std::exit(1);
+ }
+
+ if (!std::filesystem::exists(minifi_home / "fips" / "fipsmodule.cnf")) {
+ logger->log_error("FIPS mode is enabled, but fipsmodule.cnf is not available in MINIFI_HOME/fips directory. "
+ "Run MINIFI_HOME/fips/openssl fipsinstall -out fipsmodule.cnf -module MINIFI_HOME/fips/{} command to generate the configuration file", FIPS_LIB);
+ std::exit(1);
+ }
+
+ if (!std::filesystem::exists(minifi_home / "fips" / "openssl.cnf")) {
+ logger->log_error("FIPS mode is enabled, but openssl.cnf is not available in MINIFI_HOME/fips directory");
+ std::exit(1);
+ }
+
+ if (!replaceMinifiHomeVariable(minifi_home / "fips" / "openssl.cnf", minifi_home.string(), logger)) {
+ logger->log_error("Failed to replace MINIFI_HOME variable in openssl.cnf");
+ std::exit(1);
+ }
+
+ utils::Environment::setEnvironmentVariable("OPENSSL_CONF", (minifi_home / "fips" / "openssl.cnf").string().c_str(), true);
+
+ if (!OSSL_PROVIDER_set_default_search_path(nullptr, (minifi_home / "fips").string().c_str())) {
+ logger->log_error("Failed to set FIPS module path: {}", (minifi_home / "fips").string());
+ std::exit(1);
+ }
+
+ if (OSSL_PROVIDER_available(nullptr, "fips") != 1) {
+ logger->log_error("FIPS provider not available in default search path");
+ std::exit(1);
+ }
+
+ if (!EVP_default_properties_enable_fips(nullptr, 1)) {
+ logger->log_error("Failed to enable FIPS mode");
+ std::exit(1);
+ }
+
+ if (!EVP_default_properties_is_fips_enabled(nullptr)) {
+ logger->log_error("FIPS mode is not enabled");
+ std::exit(1);
+ }
Review Comment:
We should use `ERR_print_errors_fp()` to print the OpenSSL error(s) in these error cases.
##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -333,6 +431,8 @@ int main(int argc, char **argv) {
configure->loadConfigureFile(DEFAULT_NIFI_PROPERTIES_FILE);
overridePropertiesFromCommandLine(argument_parser, configure);
+ initializeFipsMode(configure, minifiHome, logger);
Review Comment:
Can this be moved after `dumpDocsIfRequested` and `writeSchemaIfRequested`? I don't think those need FIPS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983075899
##########
minifi_main/MiNiFiMain.cpp:
##########
Review Comment:
Updated in https://github.com/apache/nifi-minifi-cpp/pull/1925/commits/0418b1866dcc20de4494130deacae025d61879e5
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1987138470
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
From what I read in the description I think it's better to keep shared libraries pinned, in our case the fips provider, as without this it could happen in runtime that the library is unloaded when unused, I think it's safer to have it loaded through the agent's lifetime. If FIPS is unused then it should not have any affect as no other shared libraries are used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1981862390
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -59,7 +65,8 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
no-module # disable dynamically loadable engines
no-pinshared # don't pin shared libraries in the process memory
enable-tfo # Enable TCP Fast Open
- no-ssl) # disable SSLv3
+ no-ssl # disable SSLv3
+ no-engine)
Review Comment:
I'm not that familiar with it, but there is an Engine API in OpenSSL that is controlled by this option. An engine that is loaded in OpenSSL is a hardware or software implementation used for performing cryptographic operations. Engines are not FIPS compatible and corresponding API is deprecated since OpenSSL 3.0, so for FIPS compliancy we should remove the engine support as well. (According to the documention it is also a good guidline for FIPS compliancy to remove anything that is deprecated in OpenSSL 3.0 as that is probably not FIPS compatible.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983442423
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
aah ok. I thought both the binary and the fips module come from the fips target.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983343787
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
But these options are meant for the OpenSSL library options for loading the FIPS module, not for the FIPS target.
- We need the binary of the latest OpenSSL target, not the FIPS target
- Without the options to be able to load shared libraries, loading of the FIPS module fails with the following error:
```
Failed to load FIPS module
INSTALL FAILED
4077F3DFF87D0000:error:07880025:common libcrypto routines:provider_init:reason(37):crypto/provider_core.c:949:name=fips
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1981745145
##########
minifi_main/MiNiFiMain.cpp:
##########
Review Comment:
I think the new functions should be extracted to a separate file.
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -59,7 +65,8 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
no-module # disable dynamically loadable engines
no-pinshared # don't pin shared libraries in the process memory
enable-tfo # Enable TCP Fast Open
- no-ssl) # disable SSLv3
+ no-ssl # disable SSLv3
+ no-engine)
Review Comment:
What does this exactly do? INSTALL.md says:
> Don't build support for loading engines.
But I'm not sure what loadable engines are. Is it related to FIPS in any way? Also, the FIPS target omits most of these, probably leading to some unnecessary extra build steps, consider adding some more options there, like no-tests, no-docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1987135939
##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -333,6 +431,8 @@ int main(int argc, char **argv) {
configure->loadConfigureFile(DEFAULT_NIFI_PROPERTIES_FILE);
overridePropertiesFromCommandLine(argument_parser, configure);
+ initializeFipsMode(configure, minifiHome, logger);
Review Comment:
Had to revert this change in https://github.com/apache/nifi-minifi-cpp/pull/1925/commits/84a48c6a513e94f143d0fb15b4008efaeb8e1284 as loading FIPS needs to happen before the extension manager is initialized to work with the extensions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1981642009
##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -183,6 +187,100 @@ void writeSchemaIfRequested(const argparse::ArgumentParser& parser, const std::s
std::exit(0);
}
+bool replaceMinifiHomeVariable(const std::filesystem::path& file_path, std::string minifi_home_path, const std::shared_ptr<core::logging::Logger>& logger) {
+ std::ifstream input_file(file_path);
+ if (!input_file) {
+ logger->log_error("Failed to open file: {}", file_path.string());
+ return false;
+ }
+
+ std::ostringstream buffer;
+ buffer << input_file.rdbuf();
+ std::string content = buffer.str();
+ input_file.close();
+
+ const std::string placeholder = "${MINIFI_HOME}";
+ size_t pos = content.find(placeholder, 0);
+ if (pos == std::string::npos) {
+ return true;
+ }
+
+ std::replace(minifi_home_path.begin(), minifi_home_path.end(), '\\', '/');
+ do {
+ content.replace(pos, placeholder.length(), minifi_home_path);
+ pos += minifi_home_path.length();
+ } while((pos = content.find(placeholder, pos)) != std::string::npos);
+
+ std::ofstream output_file(file_path);
+ if (!output_file) {
+ logger->log_error("Failed to open file for writing: {}", file_path.string());
+ return false;
+ }
+
+ output_file << content;
+ output_file.close();
+ return true;
+}
+
+void initializeFipsMode(const std::shared_ptr<minifi::Configure>& configure, const std::filesystem::path& minifi_home, const std::shared_ptr<core::logging::Logger>& logger) {
+ if (!(configure->get(minifi::Configure::nifi_openssl_fips_support_enable) | utils::andThen(utils::string::toBool)).value_or(false)) {
+ return;
+ }
+
+#ifdef WIN32
+ static constexpr std::string_view FIPS_LIB = "fips.dll";
+#elif defined(__APPLE__)
+ static constexpr std::string_view FIPS_LIB = "fips.dylib";
+#else
+ static constexpr std::string_view FIPS_LIB = "fips.so";
+#endif
+
+ if (!std::filesystem::exists(minifi_home / "fips" / FIPS_LIB)) {
+ logger->log_error("FIPS mode is enabled, but {} is not available in MINIFI_HOME/fips directory", FIPS_LIB);
+ std::exit(1);
+ }
+
+ if (!std::filesystem::exists(minifi_home / "fips" / "fipsmodule.cnf")) {
+ logger->log_error("FIPS mode is enabled, but fipsmodule.cnf is not available in MINIFI_HOME/fips directory. "
+ "Run MINIFI_HOME/fips/openssl fipsinstall -out fipsmodule.cnf -module MINIFI_HOME/fips/{} command to generate the configuration file", FIPS_LIB);
+ std::exit(1);
+ }
+
+ if (!std::filesystem::exists(minifi_home / "fips" / "openssl.cnf")) {
+ logger->log_error("FIPS mode is enabled, but openssl.cnf is not available in MINIFI_HOME/fips directory");
+ std::exit(1);
+ }
+
+ if (!replaceMinifiHomeVariable(minifi_home / "fips" / "openssl.cnf", minifi_home.string(), logger)) {
+ logger->log_error("Failed to replace MINIFI_HOME variable in openssl.cnf");
+ std::exit(1);
+ }
+
+ utils::Environment::setEnvironmentVariable("OPENSSL_CONF", (minifi_home / "fips" / "openssl.cnf").string().c_str(), true);
+
+ if (!OSSL_PROVIDER_set_default_search_path(nullptr, (minifi_home / "fips").string().c_str())) {
+ logger->log_error("Failed to set FIPS module path: {}", (minifi_home / "fips").string());
+ std::exit(1);
+ }
+
+ if (OSSL_PROVIDER_available(nullptr, "fips") != 1) {
+ logger->log_error("FIPS provider not available in default search path");
+ std::exit(1);
+ }
+
+ if (!EVP_default_properties_enable_fips(nullptr, 1)) {
+ logger->log_error("Failed to enable FIPS mode");
+ std::exit(1);
+ }
+
+ if (!EVP_default_properties_is_fips_enabled(nullptr)) {
+ logger->log_error("FIPS mode is not enabled");
+ std::exit(1);
+ }
Review Comment:
Updated in https://github.com/apache/nifi-minifi-cpp/pull/1925/commits/a893f09f0598bcc9d38bcf2a23bee3f06868c985
##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -183,6 +187,100 @@ void writeSchemaIfRequested(const argparse::ArgumentParser& parser, const std::s
std::exit(0);
}
+bool replaceMinifiHomeVariable(const std::filesystem::path& file_path, std::string minifi_home_path, const std::shared_ptr<core::logging::Logger>& logger) {
+ std::ifstream input_file(file_path);
+ if (!input_file) {
+ logger->log_error("Failed to open file: {}", file_path.string());
+ return false;
+ }
+
+ std::ostringstream buffer;
+ buffer << input_file.rdbuf();
+ std::string content = buffer.str();
+ input_file.close();
+
+ const std::string placeholder = "${MINIFI_HOME}";
+ size_t pos = content.find(placeholder, 0);
+ if (pos == std::string::npos) {
+ return true;
+ }
+
+ std::replace(minifi_home_path.begin(), minifi_home_path.end(), '\\', '/');
Review Comment:
Updated in https://github.com/apache/nifi-minifi-cpp/pull/1925/commits/a893f09f0598bcc9d38bcf2a23bee3f06868c985
##########
minifi_main/MiNiFiMain.cpp:
##########
@@ -333,6 +431,8 @@ int main(int argc, char **argv) {
configure->loadConfigureFile(DEFAULT_NIFI_PROPERTIES_FILE);
overridePropertiesFromCommandLine(argument_parser, configure);
+ initializeFipsMode(configure, minifiHome, logger);
Review Comment:
Updated in https://github.com/apache/nifi-minifi-cpp/pull/1925/commits/a893f09f0598bcc9d38bcf2a23bee3f06868c985
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm closed pull request #1925: MINIFICPP-2524 Add FIPS compliancy support
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983268004
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
Why are all these removed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983287116
##########
minifi_main/MiNiFiMain.cpp:
##########
Review Comment:
Moved to `minifi_main` in https://github.com/apache/nifi-minifi-cpp/pull/1925/commits/cbf74e02f1f7e12855229bdfbdde1ede810a2aa6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983320226
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
It's fine with fips, but these are the library options, not the fips target options.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2524 Add FIPS compliancy support [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #1925:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1925#discussion_r1983343787
##########
cmake/BundledOpenSSL.cmake:
##########
@@ -51,15 +57,12 @@ function(use_openssl SOURCE_DIR BINARY_DIR)
set(OPENSSL_EXTRA_FLAGS
no-tests # Disable tests
- no-apps # disable executables
no-capieng # disable CAPI engine (legacy)
- no-dso # disable dynamic libraries
no-docs # disable docs and manpages
no-legacy # disable legacy modules
- no-module # disable dynamically loadable engines
- no-pinshared # don't pin shared libraries in the process memory
Review Comment:
But these options are meant for the library options for loading the FIPS module not for the FIPS target.
- we need the binary of the latest OpenSSL target, not the FIPS target
- Without the options to be able to load shared libraries loading of the FIPS module fails with the following error:
```
Failed to load FIPS module
INSTALL FAILED
4077F3DFF87D0000:error:07880025:common libcrypto routines:provider_init:reason(37):crypto/provider_core.c:949:name=fips
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org