Skip to content

Commit

Permalink
[Backport 2.6.1] [#4421] [YCQL] Disallow Unauthenticated LDAP binding…
Browse files Browse the repository at this point in the history
… + add handling for ycql_ldap_search_filter

Summary:
LDAP protocol has an "Unauthenticated" bind mechanism which allows successful
bind with a non-empty username but empty password.

As per https://datatracker.ietf.org/doc/html/rfc4513#section-6.3.1, clients and
servers, both, should steer clear of this bind mechanism. For this, a check has
been added to error out on empty passwords.

Another fix in this diff is that the handling and tests for ycql_ldap_search_filter were missing.
They have been added now.

Original commit: https://phabricator.dev.yugabyte.com/D13197, 08ad5c8

Test Plan:
Jenkins: urgent, rebase: 2.6.1
./yb_build.sh --java-test org.yb.cql.TestLDAPAuth

Reviewers: dmitry, alan, mihnea, steve.varnau

Reviewed By: mihnea, steve.varnau

Subscribers: steve.varnau, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D13329
  • Loading branch information
pkj415 committed Oct 7, 2021
1 parent 597aaf2 commit 9231cea
Show file tree
Hide file tree
Showing 9 changed files with 336 additions and 52 deletions.
12 changes: 7 additions & 5 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -710,11 +710,13 @@ endif()
set(CMAKE_CXX_STANDARD_REQUIRED ON)

if (USING_LINUXBREW)
# Add the Linuxbrew library directory last.
ADD_LINKER_FLAGS("-L${LINUXBREW_LIB_DIR}")
ADD_GLOBAL_RPATH_ENTRY("${LINUXBREW_LIB_DIR}")
ADD_LINKER_FLAGS("-L/usr/lib64")
ADD_GLOBAL_RPATH_ENTRY("/usr/lib64")
# We need to add the postgres library directory prior to /usr/lib64 specifically in order to avoid
# using the system libpq.
ADD_GLOBAL_RPATH_ENTRY_AND_LIB_DIR("${YB_BUILD_ROOT}/postgres/lib")

# Add the Linuxbrew library directory and the system library directory last.
ADD_GLOBAL_RPATH_ENTRY_AND_LIB_DIR("${LINUXBREW_LIB_DIR}")
ADD_GLOBAL_RPATH_ENTRY_AND_LIB_DIR("/usr/lib64")
endif()

message("Linker flags: ${CMAKE_EXE_LINKER_FLAGS}")
Expand Down
46 changes: 35 additions & 11 deletions cmake_modules/YugabyteFunctions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -204,28 +204,52 @@ endfunction()
# Linker flags applied to both executables and shared libraries. We append this both to
# CMAKE_EXE_LINKER_FLAGS and CMAKE_SHARED_LINKER_FLAGS after we finish making changes to this.
# These flags apply to both YB and RocksDB parts of the codebase.
function(ADD_LINKER_FLAGS FLAGS)
#
# This is an internal macro that modifies variables at the parent scope, which is really the parent
# scope of the functions calling it, i.e. function caller's scope.
macro(_ADD_LINKER_FLAGS_MACRO FLAGS)
if ($ENV{YB_VERBOSE})
message("Adding to linker flags: ${FLAGS}")
endif()

# We must set these variables in both current and parent scope, because this macro can be called
# multiple times from the same function.
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${FLAGS}")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${FLAGS}" PARENT_SCOPE)

set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${FLAGS}")
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${FLAGS}" PARENT_SCOPE)
endfunction()
endmacro()

function(ADD_GLOBAL_RPATH_ENTRY RPATH_ENTRY)
if (RPATH_ENTRY STREQUAL "")
message(FATAL_ERROR "Trying to add an empty rpath entry.")
# Check if the given directory is not an empty string and also warn if it does not exist.
function(_CHECK_LIB_DIR DIR_PATH DESCRIPTION)
if (DIR_PATH STREQUAL "")
message(FATAL_ERROR "Trying to add an empty ${DESCRIPTION}.")
endif()
if(NOT EXISTS "${RPATH_ENTRY}")
if(NOT EXISTS "${DIR_PATH}")
message(
WARNING
"Adding a non-existent rpath directory '${RPATH_ENTRY}'. This might be OK in case the "
"directory is created during the build.")
"Adding a non-existent ${DESCRIPTION} '${DIR_PATH}'. "
"This might be OK in case the directory is created during the build.")
endif()
endfunction()

function(ADD_LINKER_FLAGS FLAGS)
_ADD_LINKER_FLAGS_MACRO("${FLAGS}")
endfunction()

function(ADD_GLOBAL_RPATH_ENTRY RPATH_ENTRY)
_CHECK_LIB_DIR("${RPATH_ENTRY}" "rpath entry")
message("Adding a global rpath entry: ${RPATH_ENTRY}")
set(FLAGS "-Wl,-rpath,${RPATH_ENTRY}")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${FLAGS}" PARENT_SCOPE)
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} ${FLAGS}" PARENT_SCOPE)
_ADD_LINKER_FLAGS_MACRO("-Wl,-rpath,${RPATH_ENTRY}")
endfunction()

# This is similar to ADD_GLOBAL_RPATH_ENTRY but also adds an -L<dir> linker flag.
function(ADD_GLOBAL_RPATH_ENTRY_AND_LIB_DIR DIR_PATH)
_CHECK_LIB_DIR("${DIR_PATH}" "library directory and rpath entry")
message("Adding a library directory and global rpath entry: ${DIR_PATH}")
_ADD_LINKER_FLAGS_MACRO("-L${DIR_PATH}")
_ADD_LINKER_FLAGS_MACRO("-Wl,-rpath,${DIR_PATH}")
endfunction()

# CXX_YB_COMMON_FLAGS are flags that are common across the 'src/yb' portion of the codebase (but do
Expand Down
2 changes: 1 addition & 1 deletion docs/content/latest/secure/enable-authentication/ycql.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ Connecting to the cluster with the default password would no longer work:
```
$ bin/ycqlsh -u cassandra -p cassandra
Connection error:
... Provided username cassandra and/or password are incorrect ...
... Provided username 'cassandra' and/or password are incorrect ...
```

You can now connect to the cluster using the new password:
Expand Down
2 changes: 1 addition & 1 deletion docs/content/stable/secure/enable-authentication/ycql.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ Connecting to the cluster with the default password would no longer work:
```
$ bin/ycqlsh -u cassandra -p cassandra
Connection error:
... Provided username cassandra and/or password are incorrect ...
... Provided username 'cassandra' and/or password are incorrect ...
```

You can now connect to the cluster using the new password:
Expand Down
2 changes: 1 addition & 1 deletion java/yb-cql/src/test/java/org/yb/cql/TestAudit.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void auth() throws Exception {
assertAudit(
new AuditLogEntry('E', "null", "LOGIN_ERROR", "AUTH",
null /* batchId */, null /* keyspace */, null /* scope */,
"LOGIN FAILURE; Provided username user1 and/or password are incorrect"));
"LOGIN FAILURE; Provided username 'user1' and/or password are incorrect"));
} catch (Exception ex) {
throw ex;
}
Expand Down
Loading

0 comments on commit 9231cea

Please sign in to comment.