[HBASE-27526] NettyHBaseSaslRpcServerHandler.channelRead0 forget to record "AUTH_FAILED_FOR" auditlog for an exception.

• Type: Brainstorming
• Status: Resolved
• Priority: Minor
• Resolution: Not A Problem
• Assignee: Unassigned
• Reporter: Beibei Zhao
• Created: 09/Dec/22 13:29
• Updated: 08/Jan/23 15:43
• Resolved: 23/Dec/22 09:17

Description

In other methods such as SimpleServerRpcConnection.saslReadAndProcess, they always record "AUTH_FAILED_FOR" for an exception, and "AUTH_SUCCESSFUL_FOR" after task is completed like this:

  private void saslReadAndProcess(ByteBuff saslToken) throws IOException, InterruptedException {
    ......
      } catch (IOException e) {
        ......
        // attempting user could be null
        RpcServer.AUDITLOG.warn("{}{}: {}", RpcServer.AUTH_FAILED_FOR, clientIP,
          saslServer.getAttemptingUser());
        throw e;
      }
      ......
      if (saslServer.isComplete()) {
        ......
        RpcServer.AUDITLOG.info(RpcServer.AUTH_SUCCESSFUL_FOR + ugi);
        ......
      }
    }
  }

but NettyHBaseSaslRpcServerHandler.channelRead0 only record "AUTH_SUCCESSFUL_FOR" in finishSaslNegotiation, and just throw Exception without record "AUTH_FAILED_FOR":

protected void channelRead0(ChannelHandlerContext ctx, ByteBuf msg) throws Exception {
      ......
      if (saslServer.isComplete()) {
        conn.finishSaslNegotiation();
        ......
      }
  }

void finishSaslNegotiation() throws IOException {
    ......
    RpcServer.AUDITLOG.info(RpcServer.AUTH_SUCCESSFUL_FOR + ugi);
  }

So I think an exceptionCaught should be called here:

  public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
    LOG.error("Error when doing SASL handshade, provider={}", conn.provider, cause);
    Throwable sendToClient = HBaseSaslRpcServer.unwrap(cause);
    doResponse(ctx, SaslStatus.ERROR, null, sendToClient.getClass().getName(),
      sendToClient.getLocalizedMessage());
    rpcServer.metrics.authenticationFailure();
    String clientIP = this.toString();
    // attempting user could be null
    RpcServer.AUDITLOG.warn("{}{}: {}", RpcServer.AUTH_FAILED_FOR, clientIP,
      conn.saslServer != null ? conn.saslServer.getAttemptingUser() : "Unknown");
    NettyFutureUtils.safeClose(ctx);
  }

Issue Links

• links to: GitHub Pull Request #4920