Skip to content

Omit the account session from Hello until EAA is checked - #640

Open
SebTardif wants to merge 2 commits into
ivpn:developmentfrom
SebTardif:bugfix/hello-session-eaa
Open

SebTardif wants to merge 2 commits into
ivpn:developmentfrom
SebTardif:bugfix/hello-session-eaa

Conversation

@SebTardif

Copy link
Copy Markdown

PR type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other... Please describe:

PR checklist

  • I have read the CONTRIBUTING.md doc
  • The Git workflow follows our guidelines: CONTRIBUTING.md#git
  • I have added necessary documentation (if appropriate)

What is the current behavior?

Hello is on the Enhanced App Authentication skip list, so the password is not checked. The Hello reply still includes the account id and the API session token.

The UI reads that session from the first Hello, before the user has entered the EAA password.

Issue number: N/A

What is the new behavior?

Until this connection has passed the EAA check, a Hello reply sent to it has an empty account id and an empty session token. Paranoid-mode status, version, and settings are unchanged, so the UI can still show the password prompt.

After the first successful password check, the daemon sends a full Hello to that connection. The UI then loads the session the same way it does today.

When EAA is off, Hello is unchanged.

TestHelloForClientOmitsSessionUntilEAA covers the redaction.

Does this PR introduce a breaking change?

  • Yes
  • No

With EAA off, clients see the same Hello. With EAA on, the session arrives on the Hello that follows a successful password check instead of on the first Hello.

Other information

Base is development.

Hello is on the EAA skip list, and the reply included the API
session token. Drop that token and the account id for a connection
that has not passed the password check. After the first successful
check, send a full Hello so the UI can load the session.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
An unauthenticated connection receives a Hello whose account id
and session token are empty on the wire. The unit test of
helloForClient does not call notifyClients.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
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.

1 participant