Skip to content

Commit

Permalink
Adds emittion of user authentication success event.
Browse files Browse the repository at this point in the history
Motivation:
Currently there is now way to wait for successful SSH session
establishment, the only way to continue is to try to create a child
channel. Given that most clients are expecting the session to be active
when both key exchange and user authenticaiton complete, I propose to
emit UserAuthSuccessEvent.  That way client can wait for that event or
fail if there is an error in the pipeline.

Modifications:
 - Adds UserAuthSuccessEvent event
 - Adds .event case to StateMachineInboundProcessResult
 - Adds emittion of event if StateMachineInboundProcessResult is .event
 - Adds two tests

Result:
Closes #39
  • Loading branch information
artemredkin committed Mar 1, 2021
1 parent 1f95cb0 commit b05da8d
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ protocol AcceptsUserAuthMessages {
var userAuthStateMachine: UserAuthenticationStateMachine { get set }
}

/// This event indicates that server accepted our response to authentication challenge. SSH session can be considered active after that.
public struct UserAuthSuccessEvent {
}

extension AcceptsUserAuthMessages {
mutating func receiveServiceRequest(_ message: SSHMessage.ServiceRequestMessage) throws -> SSHConnectionStateMachine.StateMachineInboundProcessResult {
let result = try self.userAuthStateMachine.receiveServiceRequest(message)
Expand Down Expand Up @@ -52,7 +56,7 @@ extension AcceptsUserAuthMessages {
/// If this method completes without throwing, user auth has completed.
mutating func receiveUserAuthSuccess() throws -> SSHConnectionStateMachine.StateMachineInboundProcessResult {
try self.userAuthStateMachine.receiveUserAuthSuccess()
return .noMessage
return .event(UserAuthSuccessEvent())
}

mutating func receiveUserAuthFailure(_ message: SSHMessage.UserAuthFailureMessage) throws -> SSHConnectionStateMachine.StateMachineInboundProcessResult {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,7 @@ extension SSHConnectionStateMachine {
case globalRequest(SSHMessage.GlobalRequestMessage)
case disconnect
case noMessage
case event(Any)

enum GlobalRequestResponse {
case success(SSHMessage.RequestSuccessMessage)
Expand Down
2 changes: 2 additions & 0 deletions Sources/NIOSSH/NIOSSHHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ extension NIOSSHHandler: ChannelDuplexHandler {
case .disconnect:
// Welp, we immediately have to close.
context.close(promise: nil)
case .event(let event):
context.fireUserInboundEventTriggered(event)
}
}
}
Expand Down
73 changes: 73 additions & 0 deletions Tests/NIOSSHTests/EndToEndTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -608,4 +608,77 @@ class EndToEndTests: XCTestCase {
XCTAssertNotNil(err)
XCTAssertEqual((err as? NIOSSHError)?.type, .creatingChannelAfterClosure)
}

func testHandshakeSuccess() throws {
class ClientHandshakeHandler: ChannelInboundHandler {
typealias InboundIn = Any

let promise: EventLoopPromise<Void>

init(promise: EventLoopPromise<Void>) {
self.promise = promise
}

func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) {
if event is UserAuthSuccessEvent {
self.promise.succeed(())
}
}
}

let promise = self.channel.client.eventLoop.makePromise(of: Void.self)
let handshaker = ClientHandshakeHandler(promise: promise)

let harness = TestHarness()

// Set up the connection, validate all is well.
XCTAssertNoThrow(try self.channel.configureWithHarness(harness))
XCTAssertNoThrow(try self.channel.client.pipeline.addHandler(handshaker).wait())
XCTAssertNoThrow(try self.channel.activate())
XCTAssertNoThrow(try self.channel.interactInMemory())

XCTAssertNoThrow(try promise.futureResult.wait())
}

func testHandshakeFailure() throws {
class ClientHandshakeHandler: ChannelInboundHandler {
typealias InboundIn = Any

let promise: EventLoopPromise<Void>

init(promise: EventLoopPromise<Void>) {
self.promise = promise
}

func errorCaught(context: ChannelHandlerContext, error: Error) {
self.promise.fail(error)
}
}

enum TestError: Error {
case bang
}

struct BadPasswordDelegate: NIOSSHClientUserAuthenticationDelegate {
func nextAuthenticationType(availableMethods: NIOSSHAvailableUserAuthenticationMethods, nextChallengePromise: EventLoopPromise<NIOSSHUserAuthenticationOffer?>) {
nextChallengePromise.fail(TestError.bang)
}
}

var harness = TestHarness()
harness.clientAuthDelegate = BadPasswordDelegate()

let promise = self.channel.client.eventLoop.makePromise(of: Void.self)
let handshaker = ClientHandshakeHandler(promise: promise)

// Set up the connection, validate all is well.
XCTAssertNoThrow(try self.channel.configureWithHarness(harness))
XCTAssertNoThrow(try self.channel.client.pipeline.addHandler(handshaker).wait())
XCTAssertNoThrow(try self.channel.activate())
XCTAssertNoThrow(try self.channel.interactInMemory())

XCTAssertThrowsError(try promise.futureResult.wait()) { error in
XCTAssertEqual(error as? TestError, TestError.bang)
}
}
}
14 changes: 7 additions & 7 deletions Tests/NIOSSHTests/SSHConnectionStateMachineTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ final class SSHConnectionStateMachineTests: XCTestCase {
}
}
}
case .some(.forwardToMultiplexer), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect):
case .some(.forwardToMultiplexer), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect), .some(.event):
fatalError("Currently unsupported")
}
}
Expand Down Expand Up @@ -120,7 +120,7 @@ final class SSHConnectionStateMachineTests: XCTestCase {
}
}
}
case .some(.forwardToMultiplexer), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect):
case .some(.forwardToMultiplexer), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect), .some(.event):
fatalError("Currently unsupported")
}
}
Expand All @@ -144,7 +144,7 @@ final class SSHConnectionStateMachineTests: XCTestCase {
switch result {
case .some(.forwardToMultiplexer(let forwardedMessage)):
XCTAssertEqual(forwardedMessage, message)
case .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect), .none:
case .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect), .some(.event), .none:
XCTFail("Unexpected result: \(String(describing: result))")
}
}
Expand All @@ -169,7 +169,7 @@ final class SSHConnectionStateMachineTests: XCTestCase {
case .some(.disconnect):
// Good
break
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequest), .some(.globalRequestResponse), .none:
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequest), .some(.globalRequestResponse), .some(.event), .none:
XCTFail("Unexpected result: \(String(describing: result))")
}
}
Expand All @@ -186,7 +186,7 @@ final class SSHConnectionStateMachineTests: XCTestCase {
case .some(.globalRequest(let receivedMessage)):
// Good
XCTAssertEqual(.globalRequest(receivedMessage), message)
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequestResponse), .some(.disconnect), .none:
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequestResponse), .some(.disconnect), .some(.event), .none:
XCTFail("Unexpected result: \(String(describing: result))")
}
}
Expand All @@ -203,7 +203,7 @@ final class SSHConnectionStateMachineTests: XCTestCase {
case .some(.globalRequestResponse(let response)):
// Good
return response
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequest), .some(.disconnect), .none:
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.noMessage), .some(.globalRequest), .some(.disconnect), .some(.event), .none:
XCTFail("Unexpected result: \(String(describing: result))")
return nil
}
Expand All @@ -221,7 +221,7 @@ final class SSHConnectionStateMachineTests: XCTestCase {
case .some(.noMessage):
// Good
break
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect), .none:
case .some(.forwardToMultiplexer), .some(.emitMessage), .some(.possibleFutureMessage), .some(.globalRequest), .some(.globalRequestResponse), .some(.disconnect), .some(.event), .none:
XCTFail("Unexpected result: \(String(describing: result))")
}
}
Expand Down

0 comments on commit b05da8d

Please sign in to comment.