Skip to content

Commit bc094f8

Browse files
committed
Allow option to use passwords with IAM for transition
Signed-off-by: mkhullar <mohit.khullar@gmail.com>
1 parent 99f399e commit bc094f8

File tree

9 files changed

+226
-46
lines changed

9 files changed

+226
-46
lines changed

db/comdb2.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ int gbl_vtab_externalauth = 1;
242242
int gbl_uses_simpleauth = 0;
243243
int gbl_uses_externalauth_connect = 0;
244244
int gbl_externalauth_warn = 0;
245+
int gbl_passwords_with_externalauth = 0;
245246
int gbl_identity_cache_max = 500;
246247
int gbl_authorization_cache_max = 2000;
247248
int gbl_authentication_cache_ageout = 900;

db/comdb2.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,6 +1654,7 @@ extern int gbl_vtab_externalauth;
16541654
extern int gbl_uses_simpleauth;
16551655
extern int gbl_uses_externalauth_connect;
16561656
extern int gbl_externalauth_warn;
1657+
extern int gbl_passwords_with_externalauth;
16571658
extern int gbl_identity_cache_max;
16581659
extern int gbl_authorization_cache_max;
16591660
extern int gbl_authentication_cache_ageout;

db/db_access.c

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,21 @@ void get_client_origin(char *out, size_t outlen, struct sqlclntstate *clnt) {
6969
clnt->argv0 ? clnt->argv0 : "?",
7070
clnt->origin ? clnt->origin: "?",
7171
clnt->conninfo.pid);
72-
}
72+
}
73+
74+
static void report_access_denied(const char *action, const char *table, const char *user, int bdberr, errstat_t *err)
75+
{
76+
char msg[1024];
77+
if (bdberr)
78+
snprintf(msg, sizeof(msg), "%s access denied to %s for user %s bdberr=%d", action, table, user, bdberr);
79+
else
80+
snprintf(msg, sizeof(msg), "%s access denied to table %s for user %s", action, table, user);
81+
logmsg(LOGMSG_INFO, "%s\n", msg);
82+
if (err) {
83+
errstat_set_rc(err, SQLITE_ACCESS);
84+
errstat_set_str(err, msg);
85+
}
86+
}
7387

7488
int gbl_fdb_auth_error = 0;
7589

@@ -126,7 +140,8 @@ int check_user_password(struct sqlclntstate *clnt)
126140
clnt->allow_make_request = 1;
127141
ATOMIC_ADD64(gbl_num_auth_allowed, 1);
128142
}
129-
return rc;
143+
if (rc || !gbl_passwords_with_externalauth)
144+
return rc;
130145
}
131146

132147
if ((!remsql_warned || gbl_fdb_auth_error) && (!gbl_uses_password && !gbl_uses_externalauth) &&
@@ -265,11 +280,7 @@ int access_control_check_sql_write(struct BtCursor *pCur,
265280
if ((authdata = get_authdata(clnt)) != NULL)
266281
clnt->authdata = authdata;
267282
char client_info[1024];
268-
snprintf(client_info, sizeof(client_info),
269-
"%s:origin:%s:pid:%d",
270-
clnt->argv0 ? clnt->argv0 : "?",
271-
clnt->origin ? clnt->origin: "?",
272-
clnt->conninfo.pid);
283+
get_client_origin(client_info, sizeof(client_info), clnt);
273284
if (!clnt->authdata && clnt->secure && !gbl_allow_anon_id_for_spmux) {
274285
return reject_anon_id(clnt);
275286
}
@@ -278,31 +289,20 @@ int access_control_check_sql_write(struct BtCursor *pCur,
278289
clnt->argv0 ? clnt->argv0 : "???", clnt->conninfo.pid, clnt->conninfo.node);
279290
} else if (externalComdb2AuthenticateUserWrite(clnt->authdata, table_name, client_info)) {
280291
ATOMIC_ADD64(gbl_num_auth_denied, 1);
281-
char msg[1024];
282-
snprintf(msg, sizeof(msg), "Write access denied to table %s for user %s",
283-
table_name, clnt->externalAuthUser ? clnt->externalAuthUser : "");
284-
logmsg(LOGMSG_INFO, "%s\n", msg);
285-
errstat_set_rc(&thd->clnt->osql.xerr, SQLITE_ACCESS);
286-
errstat_set_str(&thd->clnt->osql.xerr, msg);
292+
report_access_denied("Write", table_name, clnt->externalAuthUser ? clnt->externalAuthUser : "", 0,
293+
&thd->clnt->osql.xerr);
287294
return SQLITE_ABORT;
288295
}
289-
} else {
290-
/* Check read access if its not user schema. */
296+
}
297+
if ((gbl_passwords_with_externalauth && !clnt->admin) || !gbl_uses_externalauth) {
298+
/* Check write access via password ACLs. */
291299
/* Check it only if engine is open already. */
292300
if (gbl_uses_password && !clnt->current_user.bypass_auth && (thd->clnt->in_sqlite_init == 0)) {
293-
rc = bdb_check_user_tbl_access(
294-
pCur->db->dbenv->bdb_env, thd->clnt->current_user.name,
295-
pCur->db->tablename, ACCESS_WRITE, &bdberr);
301+
rc = bdb_check_user_tbl_access(pCur->db->dbenv->bdb_env, thd->clnt->current_user.name, pCur->db->tablename,
302+
ACCESS_WRITE, &bdberr);
296303
if (rc != 0) {
297304
ATOMIC_ADD64(gbl_num_auth_denied, 1);
298-
char msg[1024];
299-
snprintf(msg, sizeof(msg),
300-
"Write access denied to %s for user %s bdberr=%d",
301-
table_name, thd->clnt->current_user.name, bdberr);
302-
logmsg(LOGMSG_INFO, "%s\n", msg);
303-
errstat_set_rc(&thd->clnt->osql.xerr, SQLITE_ACCESS);
304-
errstat_set_str(&thd->clnt->osql.xerr, msg);
305-
305+
report_access_denied("Write", table_name, thd->clnt->current_user.name, bdberr, &thd->clnt->osql.xerr);
306306
return SQLITE_ABORT;
307307
}
308308
}
@@ -352,40 +352,26 @@ int access_control_check_sql_read(struct BtCursor *pCur, struct sql_thread *thd,
352352
if ((authdata = get_authdata(clnt)) != NULL)
353353
clnt->authdata = authdata;
354354
char client_info[1024];
355-
snprintf(client_info, sizeof(client_info),
356-
"%s:origin:%s:pid:%d",
357-
clnt->argv0 ? clnt->argv0 : "?",
358-
clnt->origin ? clnt->origin: "?",
359-
clnt->conninfo.pid);
355+
get_client_origin(client_info, sizeof(client_info), clnt);
360356
if (!clnt->authdata && clnt->secure && !gbl_allow_anon_id_for_spmux)
361357
return reject_anon_id(clnt);
362358
if (gbl_externalauth_warn && !clnt->authdata) {
363359
logmsg(LOGMSG_INFO, "Client %s pid:%d mach:%d is missing authentication data\n",
364360
clnt->argv0 ? clnt->argv0 : "???", clnt->conninfo.pid, clnt->conninfo.node);
365361
} else if (externalComdb2AuthenticateUserRead(clnt->authdata, table_name, client_info)) {
366362
ATOMIC_ADD64(gbl_num_auth_denied, 1);
367-
char msg[1024];
368-
snprintf(msg, sizeof(msg), "Read access denied to table %s for user %s",
369-
table_name, clnt->externalAuthUser ? clnt->externalAuthUser : "");
370-
logmsg(LOGMSG_INFO, "%s\n", msg);
371-
errstat_set_rc(&thd->clnt->osql.xerr, SQLITE_ACCESS);
372-
errstat_set_str(&thd->clnt->osql.xerr, msg);
363+
report_access_denied("Read", table_name, clnt->externalAuthUser ? clnt->externalAuthUser : "", 0,
364+
&thd->clnt->osql.xerr);
373365
return SQLITE_ABORT;
374366
}
375-
} else {
367+
}
368+
if ((gbl_passwords_with_externalauth && !clnt->admin) || !gbl_uses_externalauth) {
376369
if (gbl_uses_password && !clnt->current_user.bypass_auth && table_name && thd->clnt->in_sqlite_init == 0) {
377370
rc = bdb_check_user_tbl_access(thedb->bdb_env, thd->clnt->current_user.name, (char *)table_name,
378371
ACCESS_READ, &bdberr);
379372
if (rc != 0) {
380373
ATOMIC_ADD64(gbl_num_auth_denied, 1);
381-
char msg[1024];
382-
snprintf(msg, sizeof(msg),
383-
"Read access denied to %s for user %s bdberr=%d",
384-
table_name, thd->clnt->current_user.name, bdberr);
385-
logmsg(LOGMSG_INFO, "%s\n", msg);
386-
errstat_set_rc(&thd->clnt->osql.xerr, SQLITE_ACCESS);
387-
errstat_set_str(&thd->clnt->osql.xerr, msg);
388-
374+
report_access_denied("Read", table_name, thd->clnt->current_user.name, bdberr, &thd->clnt->osql.xerr);
389375
return SQLITE_ABORT;
390376
}
391377
}

db/db_tunables.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2397,6 +2397,8 @@ REGISTER_TUNABLE("externalauth_warn", "Warn instead of returning error in case o
23972397
TUNABLE_BOOLEAN, &gbl_externalauth_warn, NOARG | READEARLY,
23982398
NULL, NULL, NULL, NULL);
23992399

2400+
REGISTER_TUNABLE("passwords_with_externalauth", "Check password auth in addition to externalauth (Default: off)",
2401+
TUNABLE_BOOLEAN, &gbl_passwords_with_externalauth, NOARG | READEARLY, NULL, NULL, NULL, NULL);
24002402

24012403
REGISTER_TUNABLE("view_feature", "Enables support for VIEWs (Default: ON)",
24022404
TUNABLE_BOOLEAN, &gbl_view_feature, 0, NULL, NULL, NULL, NULL);
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
ifeq ($(TESTSROOTDIR),)
2+
include ../testcase.mk
3+
else
4+
include $(TESTSROOTDIR)/testcase.mk
5+
endif
6+
ifeq ($(TEST_TIMEOUT),)
7+
export TEST_TIMEOUT=3m
8+
endif
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
schema
2+
{
3+
byte dummy[1]
4+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
simpleauth
2+
passwords_with_externalauth
3+
table dummy dummy.csc2
4+
create_dba_user off
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
#!/usr/bin/env bash
2+
bash -n "$0" | exit 1
3+
4+
set -e
5+
source ${TESTSROOTDIR}/tools/runit_common.sh
6+
7+
dbnm=$1
8+
9+
if [ "x$dbnm" == "x" ] ; then
10+
failexit "need a DB name"
11+
fi
12+
13+
SA="${TESTSBUILDDIR}/simpleauth_test ${dbnm}"
14+
OP="bpi:procauth:cluster:test:user:op:bpkg:setup"
15+
MOHIT="bpi:procauth:cluster:testcluster:user:mohit:bpkg:myapp"
16+
17+
passed=0
18+
failed=0
19+
20+
run_expect_success() {
21+
local principal="$1"
22+
local sql="$2"
23+
local desc="$3"
24+
if $SA "$principal" "$sql" > /dev/null 2>&1; then
25+
echo " PASS: $desc"
26+
passed=$((passed + 1))
27+
else
28+
echo " FAIL: $desc"
29+
failed=$((failed + 1))
30+
fi
31+
}
32+
33+
run_expect_failure() {
34+
local principal="$1"
35+
local sql="$2"
36+
local desc="$3"
37+
if $SA "$principal" "$sql" > /dev/null 2>&1; then
38+
echo " FAIL: $desc (expected failure but succeeded)"
39+
failed=$((failed + 1))
40+
else
41+
echo " PASS: $desc"
42+
passed=$((passed + 1))
43+
fi
44+
}
45+
46+
# Run SQL via admin connection with OP password user (bypasses IAM).
47+
admin_sql() {
48+
local sql="$1"
49+
${CDB2SQL_EXE} --admin ${CDB2_OPTIONS} ${dbnm} default - <<EOF
50+
set user admin_user
51+
set password adminpass
52+
$sql
53+
EOF
54+
}
55+
56+
#---------------------------------------------------------------
57+
echo "Test: Setup comdb2_simple_auth table and IAM rules"
58+
#---------------------------------------------------------------
59+
run_expect_success "$OP" "create table if not exists comdb2_simple_auth(cluster cstring(20) default '*', user cstring(20) default '*', bpkg cstring(50) default '*', verb cstring(20) default '*', resourcetype cstring(20) default '*', resourcename cstring(50) default '*')" \
60+
"create auth table"
61+
run_expect_success "$OP" "create unique index if not exists comdb2_simple_auth_ix on comdb2_simple_auth(cluster, user, bpkg, verb, resourcetype, resourcename)" \
62+
"create auth index"
63+
# Wildcard rule: allow everyone everything via IAM
64+
run_expect_success "$OP" "insert into comdb2_simple_auth(cluster, user, bpkg, verb, resourcetype, resourcename) values('*', '*', '*', '*', '*', '*') on conflict do nothing" \
65+
"insert wildcard IAM rule"
66+
run_expect_success "$OP" "insert into comdb2_simple_auth(cluster, user, bpkg, verb, resourcetype, resourcename) values('*', '*', '*', 'Connect', '*', '*') on conflict do nothing" \
67+
"insert Connect rule"
68+
# OP needs explicit IAM access to auth table after wildcard removal
69+
run_expect_success "$OP" "insert into comdb2_simple_auth(cluster, user, bpkg, verb, resourcetype, resourcename) values('*', 'op', '*', 'Write', 'table', 'comdb2_simple_auth') on conflict do nothing" \
70+
"grant op IAM write on auth table"
71+
run_expect_success "$OP" "insert into comdb2_simple_auth(cluster, user, bpkg, verb, resourcetype, resourcename) values('*', 'op', '*', 'Read', 'table', 'comdb2_simple_auth') on conflict do nothing" \
72+
"grant op IAM read on auth table"
73+
74+
#---------------------------------------------------------------
75+
echo "Test: Create test table and enable password auth"
76+
#---------------------------------------------------------------
77+
run_expect_success "$OP" "create table if not exists t1(i int)" \
78+
"create test table t1"
79+
# Create password users: 'default' (non-OP) for data tests,
80+
# 'admin_user' (OP) for management via admin connection.
81+
run_expect_success "$OP" "put password '' for 'default'" \
82+
"create default password user"
83+
run_expect_success "$OP" "put password 'adminpass' for 'admin_user'" \
84+
"create admin password user"
85+
run_expect_success "$OP" "grant op to 'admin_user'" \
86+
"grant OP to admin_user"
87+
# Grant table-level password ACLs to default user
88+
run_expect_success "$OP" "grant read on t1 to 'default'" \
89+
"grant password read on t1 to default"
90+
run_expect_success "$OP" "grant write on t1 to 'default'" \
91+
"grant password write on t1 to default"
92+
# Enable password authentication -- from this point both IAM and
93+
# password checks must pass.
94+
# Note: put authentication on uses comdb2AuthenticateOpPassword which
95+
# requires password-based OP credentials (ignores IAM), so we must
96+
# use admin_sql with admin_user/adminpass rather than simpleauth_test.
97+
admin_sql "put authentication on"
98+
echo " password authentication enabled"
99+
100+
#---------------------------------------------------------------
101+
echo "Test: Both IAM and password pass"
102+
#---------------------------------------------------------------
103+
# simpleauth_test provides IAM identity (wildcard allows everything).
104+
# Password user is 'default' with correct password and table ACLs.
105+
run_expect_success "$MOHIT" "select * from t1" \
106+
"read t1: IAM pass + password pass"
107+
run_expect_success "$MOHIT" "insert into t1 values(1)" \
108+
"write t1: IAM pass + password pass"
109+
110+
#---------------------------------------------------------------
111+
echo "Test: IAM passes but password table ACL denied"
112+
#---------------------------------------------------------------
113+
# Revoke password-based table ACLs for default user.
114+
# IAM wildcard still allows, but bdb_check_user_tbl_access will fail
115+
# because 'default' is not OP and has no table grants.
116+
admin_sql "revoke read on t1 from 'default'"
117+
admin_sql "revoke write on t1 from 'default'"
118+
run_expect_failure "$MOHIT" "select * from t1" \
119+
"read t1: IAM pass + password ACL denied"
120+
run_expect_failure "$MOHIT" "insert into t1 values(2)" \
121+
"write t1: IAM pass + password ACL denied"
122+
# Restore password ACLs
123+
admin_sql "grant read on t1 to 'default'"
124+
admin_sql "grant write on t1 to 'default'"
125+
# Verify restored
126+
run_expect_success "$MOHIT" "select * from t1" \
127+
"read t1 after restore: both pass"
128+
129+
#---------------------------------------------------------------
130+
echo "Test: IAM denied but password passes"
131+
#---------------------------------------------------------------
132+
# Remove wildcard IAM rule. Keep the Connect rule so MakeRequest
133+
# still passes (allows connection but not table-level access).
134+
admin_sql "delete from comdb2_simple_auth where cluster='*' and user='*' and bpkg='*' and verb='*' and resourcetype='*' and resourcename='*'"
135+
# MOHIT has no IAM Read/Write rule for t1 -> IAM table check fails
136+
# before reaching the password ACL check.
137+
run_expect_failure "$MOHIT" "select * from t1" \
138+
"read t1: IAM denied + password pass (fails at IAM)"
139+
run_expect_failure "$MOHIT" "insert into t1 values(3)" \
140+
"write t1: IAM denied + password pass (fails at IAM)"
141+
# Restore wildcard IAM rule
142+
admin_sql "insert into comdb2_simple_auth(cluster, user, bpkg, verb, resourcetype, resourcename) values('*', '*', '*', '*', '*', '*') on conflict do nothing"
143+
# Verify restored
144+
run_expect_success "$MOHIT" "select * from t1" \
145+
"read t1 after IAM restore: both pass"
146+
147+
#---------------------------------------------------------------
148+
echo "Test: Neither IAM nor password passes"
149+
#---------------------------------------------------------------
150+
admin_sql "delete from comdb2_simple_auth where cluster='*' and user='*' and bpkg='*' and verb='*' and resourcetype='*' and resourcename='*'"
151+
admin_sql "revoke read on t1 from 'default'"
152+
admin_sql "revoke write on t1 from 'default'"
153+
run_expect_failure "$MOHIT" "select * from t1" \
154+
"read t1: IAM denied + password denied"
155+
run_expect_failure "$MOHIT" "insert into t1 values(4)" \
156+
"write t1: IAM denied + password denied"
157+
158+
#---------------------------------------------------------------
159+
echo "Test: Cleanup"
160+
#---------------------------------------------------------------
161+
admin_sql "insert into comdb2_simple_auth(cluster, user, bpkg, verb, resourcetype, resourcename) values('*', '*', '*', '*', '*', '*') on conflict do nothing"
162+
admin_sql "drop table if exists t1"
163+
admin_sql "delete from comdb2_simple_auth where user != '*'"
164+
165+
echo ""
166+
echo "Results: $passed passed, $failed failed"
167+
168+
if [ $failed -gt 0 ] ; then
169+
failexit "$failed tests failed"
170+
fi
171+
172+
echo "Success"
173+
exit 0

tests/tunables.test/t00_all_tunables.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,7 @@
707707
(name='partition_retroactively_verbose', description='Disable/enable data routing debugging for retroactively time partitioning (Default: OFF)', type='BOOLEAN', value='OFF', read_only='N')
708708
(name='partition_sc_reorder', description='If the schema change is serialized for a partition, run current shard last', type='BOOLEAN', value='ON', read_only='N')
709709
(name='partitioned_table_enabled', description='Allow syntax create/alter table ... partitioned by ...', type='BOOLEAN', value='ON', read_only='N')
710+
(name='passwords_with_externalauth', description='Check password auth in addition to externalauth (Default: off)', type='BOOLEAN', value='OFF', read_only='N')
710711
(name='pause_moveto', description='pause_moveto', type='BOOLEAN', value='OFF', read_only='N')
711712
(name='pbkdf2_iterations', description='Number of iterations of PBKDF2 algorithm for password hashing.', type='INTEGER', value='4096', read_only='N')
712713
(name='penaltyincpercent', description='', type='INTEGER', value='20', read_only='Y')

0 commit comments

Comments
 (0)