From 5d17dccbba321bc8bd55230975cd2a9080aa5384 Mon Sep 17 00:00:00 2001 From: Caideyipi <87789683+Caideyipi@users.noreply.github.com> Date: Fri, 31 Jul 2026 11:20:51 +0800 Subject: [PATCH 1/2] Use null-safe tree access user comparison --- .../plan/relational/security/TreeAccessCheckVisitor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java index 73d8518a7763..04178b448c76 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java @@ -695,7 +695,7 @@ private TSStatus checkCanUpdateUser(AuthorStatement statement, TreeAccessCheckCo private TSStatus checkCanAlterUser(AuthorStatement statement, TreeAccessCheckContext context) { context.setAuditLogOperation(AuditLogOperation.DDL); - if (statement.getUserName().equals(context.getUsername())) { + if (Objects.equals(statement.getUserName(), context.getUsername())) { // users can change the username and password of themselves AUDIT_LOGGER.recordObjectAuthenticationAuditLog( context.setResult(true), context::getUsername); From 3bbec440e4703d44fcc667289c5d720e256c68b3 Mon Sep 17 00:00:00 2001 From: Caideyipi <87789683+Caideyipi@users.noreply.github.com> Date: Fri, 31 Jul 2026 13:23:19 +0800 Subject: [PATCH 2/2] Fix password audit Sonar issues --- .../apache/iotdb/db/audit/DNAuditLogger.java | 90 ++++++++++++++----- .../db/audit/PasswordChangeAuditContext.java | 12 +-- .../db/audit/PasswordChangeAuditTask.java | 5 +- .../security/AccessControlImpl.java | 1 + .../security/TreeAccessCheckVisitor.java | 1 + 5 files changed, 79 insertions(+), 30 deletions(-) diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/DNAuditLogger.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/DNAuditLogger.java index 02fedbd5e1f1..d0d0f4f5d78d 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/DNAuditLogger.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/DNAuditLogger.java @@ -57,12 +57,9 @@ public class DNAuditLogger extends AbstractAuditLogger { // This text matcher is only a fallback. Password-update semantic nodes must clear sql_string // before any audit entry is generated. - private static final Pattern ALTER_USER_PASSWORD_PATTERN = + private static final Pattern ALTER_USER_PASSWORD_PREFIX_PATTERN = Pattern.compile( - "^(\\s*ALTER\\s+USER\\s+.+?\\s+SET\\s+PASSWORD\\s+)" - + "(?:(?:U&)?'(?:''|[^'])*'|\"(?:\"\"|[^\"])*\")" - + "(\\s*;?\\s*)$", - Pattern.CASE_INSENSITIVE | Pattern.DOTALL); + "^\\s*ALTER\\s+USER\\s+\\S+\\s+SET\\s+PASSWORD\\s+", Pattern.CASE_INSENSITIVE); private static final Pattern VALUES_PATTERN = Pattern.compile("(?i)(values)\\([^)]*\\)"); private Coordinator coordinator; @@ -89,10 +86,10 @@ private static InsertRowStatement generateInsertStatement( @NotNull static InsertRowStatement generateInsertStatement( IAuditEntity auditLogFields, String log, PartialPath logDevice, long logTimestamp) { - String username = auditLogFields.getUsername(); - String address = auditLogFields.getCliHostname(); - AuditEventType type = auditLogFields.getAuditEventType(); - AuditLogOperation operation = auditLogFields.getAuditLogOperation(); + final String username = auditLogFields.getUsername(); + final String address = auditLogFields.getCliHostname(); + final AuditEventType type = auditLogFields.getAuditEventType(); + final AuditLogOperation operation = auditLogFields.getAuditLogOperation(); PrivilegeLevel privilegeLevel = PrivilegeLevel.GLOBAL; if (auditLogFields.getPrivilegeTypes() != null) { for (PrivilegeType privilegeType : auditLogFields.getPrivilegeTypes()) { @@ -164,29 +161,76 @@ static String sanitizeAuditSql(String sqlString) { if (sqlString.regionMatches(true, 0, "CREATE USER", 0, "CREATE USER".length())) { sqlString = String.join(" ", Arrays.asList(sqlString.split(" ")).subList(0, 3)) + " ..."; } - Matcher alterUserMatcher = ALTER_USER_PASSWORD_PATTERN.matcher(sqlString); - if (alterUserMatcher.matches()) { - sqlString = alterUserMatcher.replaceFirst("$1...$2"); + final Matcher passwordPrefixMatcher = ALTER_USER_PASSWORD_PREFIX_PATTERN.matcher(sqlString); + if (passwordPrefixMatcher.find()) { + final int passwordLiteralStart = passwordPrefixMatcher.end(); + final int passwordLiteralEnd = findQuotedTokenEnd(sqlString, passwordLiteralStart); + if (passwordLiteralEnd >= 0 && isStatementSuffix(sqlString, passwordLiteralEnd)) { + sqlString = + sqlString.substring(0, passwordLiteralStart) + + "..." + + sqlString.substring(passwordLiteralEnd); + } } return VALUES_PATTERN.matcher(sqlString).replaceAll("$1(...)"); } + private static int skipWhitespace(final String value, int index) { + while (index < value.length() && Character.isWhitespace(value.charAt(index))) { + index++; + } + return index; + } + + private static int findQuotedTokenEnd(final String value, final int index) { + int quoteIndex = index; + if (value.regionMatches(true, quoteIndex, "U&", 0, 2)) { + quoteIndex += 2; + } + if (quoteIndex >= value.length()) { + return -1; + } + final char quote = value.charAt(quoteIndex); + if (quote != '\'' && quote != '"') { + return -1; + } + for (int cursor = quoteIndex + 1; cursor < value.length(); cursor++) { + if (value.charAt(cursor) != quote) { + continue; + } + if (cursor + 1 < value.length() && value.charAt(cursor + 1) == quote) { + cursor++; + continue; + } + return cursor + 1; + } + return -1; + } + + private static boolean isStatementSuffix(final String value, final int index) { + int suffixIndex = skipWhitespace(value, index); + if (suffixIndex < value.length() && value.charAt(suffixIndex) == ';') { + suffixIndex = skipWhitespace(value, suffixIndex + 1); + } + return suffixIndex == value.length(); + } + private static PrivilegeLevel judgePrivilegeLevel(PrivilegeType type) { if (type == null) { return PrivilegeLevel.GLOBAL; } switch (type) { - case READ_DATA: - case DROP: - case ALTER: - case CREATE: - case DELETE: - case INSERT: - case SELECT: - case MANAGE_DATABASE: - case WRITE_DATA: - case READ_SCHEMA: - case WRITE_SCHEMA: + case READ_DATA, + DROP, + ALTER, + CREATE, + DELETE, + INSERT, + SELECT, + MANAGE_DATABASE, + WRITE_DATA, + READ_SCHEMA, + WRITE_SCHEMA: return PrivilegeLevel.OBJECT; default: return PrivilegeLevel.GLOBAL; diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditContext.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditContext.java index d25064bf9cb4..b6396ff06cce 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditContext.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditContext.java @@ -117,17 +117,17 @@ static PasswordChangeAuditContext forTreeAuthorization( } private static String getTreeTargetUsername(Statement statement) { - return statement instanceof AuthorStatement - && ((AuthorStatement) statement).getAuthorType() == AuthorType.UPDATE_USER - ? ((AuthorStatement) statement).getUserName() + return statement instanceof AuthorStatement authorStatement + && authorStatement.getAuthorType() == AuthorType.UPDATE_USER + ? authorStatement.getUserName() : null; } private static String getTableTargetUsername( org.apache.iotdb.commons.queryengine.plan.relational.sql.ast.Statement statement) { - return statement instanceof RelationalAuthorStatement - && ((RelationalAuthorStatement) statement).getAuthorType() == AuthorRType.UPDATE_USER - ? ((RelationalAuthorStatement) statement).getUserName() + return statement instanceof RelationalAuthorStatement authorStatement + && authorStatement.getAuthorType() == AuthorRType.UPDATE_USER + ? authorStatement.getUserName() : null; } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditTask.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditTask.java index f8080ee4a259..18537546e29b 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditTask.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/audit/PasswordChangeAuditTask.java @@ -69,7 +69,10 @@ public void onFailure(@NotNull Throwable throwable) { }, MoreExecutors.directExecutor()); return future; - } catch (InterruptedException | RuntimeException | Error e) { + } catch (InterruptedException e) { + auditContext.log(null); + throw e; + } catch (RuntimeException e) { auditContext.log(null); throw e; } diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java index bc20a7d972f6..a72b07a205c4 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/AccessControlImpl.java @@ -269,6 +269,7 @@ public void checkCanCreateViewFromTreePath(final PartialPath path, IAuditEntity } @Override + @SuppressWarnings("java:S6541") // Keep all relational authorization branches together. public void checkUserCanRunRelationalAuthorStatement( String userName, RelationalAuthorStatement statement, IAuditEntity auditEntity) { AuthorRType type = statement.getAuthorType(); diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java index 04178b448c76..c1b188df23f5 100644 --- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java +++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java @@ -693,6 +693,7 @@ private TSStatus checkCanUpdateUser(AuthorStatement statement, TreeAccessCheckCo } } + @SuppressWarnings("checkstyle:LineLength") private TSStatus checkCanAlterUser(AuthorStatement statement, TreeAccessCheckContext context) { context.setAuditLogOperation(AuditLogOperation.DDL); if (Objects.equals(statement.getUserName(), context.getUsername())) {