Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INTERNAL: Add authenticated flag to conn struct #801

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

namsic
Copy link
Collaborator

@namsic namsic commented Nov 8, 2024

⌨️ What I did

It was previously possible to bypass authentication due to implicit state management.
Now we explicitly consider ourselves unauthenticated on any new connections and authentication attempts.

@namsic namsic force-pushed the ascii_sasl branch 2 times, most recently from da3a371 to 0dfd60e Compare November 11, 2024 06:20
@namsic namsic changed the title Ascii sasl INTERNAL: Add authenticated flag to conn struct Nov 11, 2024
@namsic namsic marked this pull request as ready for review November 11, 2024 06:29
@jhpark816 jhpark816 merged commit 31a464c into naver:develop Nov 11, 2024
1 check passed
@namsic
Copy link
Collaborator Author

namsic commented Nov 11, 2024

int sasl_getprop(sasl_conn_t *conn, int propnum, const void **pvalue)
{
  // . . .

  switch(propnum)
  {
    // . . .
    case SASL_USERNAME:
      if(! conn->oparams.user)
	  result = SASL_NOTDONE;
      else
	  *((const char **)pvalue) = conn->oparams.user;
      break;
    // . . .
  default: 
      result = SASL_BADPARAM;
  }

sasl_getprop(conn, USERNAME, &uname)은 단순히 conn->oparams.user 값을 반환합니다.

여기서 oparams.user 값은 인증이 완료되기 전 challenge를 주고받는 과정에서 초기화되는 값이기 때문에
USERNAME property로 인증 여부를 판단하는 것이 적절하지 않은 것으로 보입니다.

@namsic namsic deleted the ascii_sasl branch November 11, 2024 07:30
@jhpark816
Copy link
Collaborator

@namsic
완전히 bug fix 이군요.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants