mirror of https://github.com/sbt/sbt.git
[2.x] fix: Gate LSP calls behind auth
**Problem** Some custom LSP calls do not check initialize-handshake, which over TCP includes token-based authentication. **Solution** This adds checkAuthenticated check around sbt/exec etc.
This commit is contained in:
parent
b0c840105b
commit
e6ac4ecffc
|
|
@ -630,6 +630,7 @@ lazy val commandProj = (project in file("main-command"))
|
||||||
contrabandSettings,
|
contrabandSettings,
|
||||||
mimaSettings,
|
mimaSettings,
|
||||||
mimaBinaryIssueFilters ++= Vector(
|
mimaBinaryIssueFilters ++= Vector(
|
||||||
|
exclude[ReversedMissingMethodProblem]("sbt.internal.server.ServerCallback.*"),
|
||||||
exclude[MissingClassProblem]("sbt.internal.util.JoinThread"),
|
exclude[MissingClassProblem]("sbt.internal.util.JoinThread"),
|
||||||
exclude[MissingClassProblem]("sbt.internal.util.JoinThread$"),
|
exclude[MissingClassProblem]("sbt.internal.util.JoinThread$"),
|
||||||
exclude[MissingClassProblem]("sbt.internal.util.ReadJsonFromInputStream"),
|
exclude[MissingClassProblem]("sbt.internal.util.ReadJsonFromInputStream"),
|
||||||
|
|
|
||||||
|
|
@ -81,6 +81,7 @@ trait ServerCallback {
|
||||||
|
|
||||||
private[sbt] def authOptions: Set[ServerAuthentication]
|
private[sbt] def authOptions: Set[ServerAuthentication]
|
||||||
private[sbt] def authenticate(token: String): Boolean
|
private[sbt] def authenticate(token: String): Boolean
|
||||||
|
private[sbt] def isAuthenticated: Boolean
|
||||||
private[sbt] def setInitialized(value: Boolean): Unit
|
private[sbt] def setInitialized(value: Boolean): Unit
|
||||||
private[sbt] def setInitializeOption(opts: InitializeOption): Unit
|
private[sbt] def setInitializeOption(opts: InitializeOption): Unit
|
||||||
private[sbt] def onSettingQuery(execId: Option[String], req: Q): Unit
|
private[sbt] def onSettingQuery(execId: Option[String], req: Q): Unit
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,7 @@ object LintUnused {
|
||||||
sbt.nio.Keys.outputFileStamper,
|
sbt.nio.Keys.outputFileStamper,
|
||||||
sbt.nio.Keys.watchTriggers,
|
sbt.nio.Keys.watchTriggers,
|
||||||
serverConnectionType,
|
serverConnectionType,
|
||||||
|
serverPort,
|
||||||
serverIdleTimeout,
|
serverIdleTimeout,
|
||||||
shellPrompt,
|
shellPrompt,
|
||||||
sLog,
|
sLog,
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,16 @@ private[sbt] object LanguageServerProtocol {
|
||||||
|
|
||||||
def handler(converter: FileConverter): ServerHandler = ServerHandler { callback =>
|
def handler(converter: FileConverter): ServerHandler = ServerHandler { callback =>
|
||||||
import callback.*
|
import callback.*
|
||||||
|
|
||||||
|
def checkAuthenticated(r: JsonRpcRequestMessage)(f: => Unit): Unit =
|
||||||
|
if !isAuthenticated then
|
||||||
|
jsonRpcRespondError(
|
||||||
|
Some(r.id),
|
||||||
|
ErrorCodes.InvalidRequest,
|
||||||
|
s"'${r.method}' is not allowed before authentication."
|
||||||
|
)
|
||||||
|
else f
|
||||||
|
|
||||||
ServerIntent(
|
ServerIntent(
|
||||||
onRequest = {
|
onRequest = {
|
||||||
case r: JsonRpcRequestMessage if r.method == "initialize" =>
|
case r: JsonRpcRequestMessage if r.method == "initialize" =>
|
||||||
|
|
@ -69,31 +79,44 @@ private[sbt] object LanguageServerProtocol {
|
||||||
jsonRpcRespond(InitializeResult(serverCapabilities), Some(r.id))
|
jsonRpcRespond(InitializeResult(serverCapabilities), Some(r.id))
|
||||||
|
|
||||||
case r: JsonRpcRequestMessage if r.method == "textDocument/definition" =>
|
case r: JsonRpcRequestMessage if r.method == "textDocument/definition" =>
|
||||||
val _ = Definition.lspDefinition(json(r), r.id, CommandSource(name), converter, log)(using
|
checkAuthenticated(r) {
|
||||||
StandardMain.executionContext
|
val _ =
|
||||||
)
|
Definition.lspDefinition(json(r), r.id, CommandSource(name), converter, log)(using
|
||||||
|
StandardMain.executionContext
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
case r: JsonRpcRequestMessage if r.method == "sbt/exec" =>
|
case r: JsonRpcRequestMessage if r.method == "sbt/exec" =>
|
||||||
val param = Converter.fromJson[SbtExecParams](json(r)).get
|
checkAuthenticated(r) {
|
||||||
val _ = appendExec(param.commandLine, Some(r.id))
|
val param = Converter.fromJson[SbtExecParams](json(r)).get
|
||||||
|
val _ = appendExec(param.commandLine, Some(r.id))
|
||||||
|
}
|
||||||
|
|
||||||
case r: JsonRpcRequestMessage if r.method == "sbt/setting" =>
|
case r: JsonRpcRequestMessage if r.method == "sbt/setting" =>
|
||||||
val param = Converter.fromJson[Q](json(r)).get
|
checkAuthenticated(r) {
|
||||||
onSettingQuery(Option(r.id), param)
|
val param = Converter.fromJson[Q](json(r)).get
|
||||||
|
onSettingQuery(Option(r.id), param)
|
||||||
|
}
|
||||||
|
|
||||||
case r: JsonRpcRequestMessage if r.method == "sbt/cancelRequest" =>
|
case r: JsonRpcRequestMessage if r.method == "sbt/cancelRequest" =>
|
||||||
val param = Converter.fromJson[CancelRequestParams](json(r)).get
|
checkAuthenticated(r) {
|
||||||
onCancellationRequest(Option(r.id), param)
|
val param = Converter.fromJson[CancelRequestParams](json(r)).get
|
||||||
|
onCancellationRequest(Option(r.id), param)
|
||||||
|
}
|
||||||
|
|
||||||
case r: JsonRpcRequestMessage if r.method == "sbt/completion" =>
|
case r: JsonRpcRequestMessage if r.method == "sbt/completion" =>
|
||||||
val param = Converter.fromJson[CP](json(r)).get
|
checkAuthenticated(r) {
|
||||||
onCompletionRequest(Option(r.id), param)
|
val param = Converter.fromJson[CP](json(r)).get
|
||||||
|
onCompletionRequest(Option(r.id), param)
|
||||||
|
}
|
||||||
|
|
||||||
},
|
},
|
||||||
onResponse = PartialFunction.empty,
|
onResponse = PartialFunction.empty,
|
||||||
onNotification = {
|
onNotification = {
|
||||||
case n: JsonRpcNotificationMessage if n.method == "textDocument/didSave" =>
|
case n: JsonRpcNotificationMessage if n.method == "textDocument/didSave" =>
|
||||||
val _ = appendExec(";Test/compile; collectAnalyses", None)
|
if (isAuthenticated) {
|
||||||
|
val _ = appendExec(";Test/compile; collectAnalyses", None)
|
||||||
|
} else log.warn(s"ignoring '${n.method}' before authentication")
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -81,6 +81,7 @@ final class NetworkChannel(
|
||||||
private val delimiter: Byte = '\n'.toByte
|
private val delimiter: Byte = '\n'.toByte
|
||||||
private val out = connection.getOutputStream
|
private val out = connection.getOutputStream
|
||||||
private var initialized = false
|
private var initialized = false
|
||||||
|
private var authenticated = false
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Reference to the client-side custom options
|
* Reference to the client-side custom options
|
||||||
|
|
@ -145,6 +146,7 @@ final class NetworkChannel(
|
||||||
def name: String = self.name
|
def name: String = self.name
|
||||||
private[sbt] def authOptions: Set[ServerAuthentication] = self.authOptions
|
private[sbt] def authOptions: Set[ServerAuthentication] = self.authOptions
|
||||||
private[sbt] def authenticate(token: String): Boolean = self.authenticate(token)
|
private[sbt] def authenticate(token: String): Boolean = self.authenticate(token)
|
||||||
|
private[sbt] def isAuthenticated: Boolean = self.isAuthenticated
|
||||||
private[sbt] def setInitialized(value: Boolean): Unit = self.setInitialized(value)
|
private[sbt] def setInitialized(value: Boolean): Unit = self.setInitialized(value)
|
||||||
private[sbt] def setInitializeOption(opts: InitializeOption): Unit =
|
private[sbt] def setInitializeOption(opts: InitializeOption): Unit =
|
||||||
self.setInitializeOption(opts)
|
self.setInitializeOption(opts)
|
||||||
|
|
@ -169,7 +171,14 @@ final class NetworkChannel(
|
||||||
private[sbt] def subscribeToAll: Boolean =
|
private[sbt] def subscribeToAll: Boolean =
|
||||||
Option(initializeOption.get).flatMap(_.subscribeToAll).getOrElse(false)
|
Option(initializeOption.get).flatMap(_.subscribeToAll).getOrElse(false)
|
||||||
|
|
||||||
protected def authenticate(token: String): Boolean = instance.authenticate(token)
|
protected def authenticate(token: String): Boolean = {
|
||||||
|
val result = instance.authenticate(token)
|
||||||
|
if result then authenticated = true
|
||||||
|
result
|
||||||
|
}
|
||||||
|
|
||||||
|
private[sbt] def isAuthenticated: Boolean =
|
||||||
|
authenticated || authOptions.isEmpty
|
||||||
|
|
||||||
protected def setInitialized(value: Boolean): Unit = initialized = value
|
protected def setInitialized(value: Boolean): Unit = initialized = value
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,7 @@ package server
|
||||||
|
|
||||||
import java.util.concurrent.{ ArrayBlockingQueue, ConcurrentHashMap }
|
import java.util.concurrent.{ ArrayBlockingQueue, ConcurrentHashMap }
|
||||||
import java.util.UUID
|
import java.util.UUID
|
||||||
|
import sbt.internal.langserver.ErrorCodes
|
||||||
import sbt.internal.protocol.{
|
import sbt.internal.protocol.{
|
||||||
JsonRpcNotificationMessage,
|
JsonRpcNotificationMessage,
|
||||||
JsonRpcRequestMessage,
|
JsonRpcRequestMessage,
|
||||||
|
|
@ -196,13 +197,20 @@ object VirtualTerminal {
|
||||||
private val requestHandler: Handler[JsonRpcRequestMessage] =
|
private val requestHandler: Handler[JsonRpcRequestMessage] =
|
||||||
callback => {
|
callback => {
|
||||||
case r if r.method == attach =>
|
case r if r.method == attach =>
|
||||||
val isInteractive = r.params
|
if (callback.isAuthenticated) {
|
||||||
.flatMap(Converter.fromJson[Attach](_).toOption.map(_.interactive))
|
val isInteractive = r.params
|
||||||
.exists(identity)
|
.flatMap(Converter.fromJson[Attach](_).toOption.map(_.interactive))
|
||||||
StandardMain.exchange.channelForName(callback.name) match {
|
.exists(identity)
|
||||||
case Some(nc: NetworkChannel) => nc.setInteractive(r.id, isInteractive)
|
StandardMain.exchange.channelForName(callback.name) match {
|
||||||
case _ =>
|
case Some(nc: NetworkChannel) => nc.setInteractive(r.id, isInteractive)
|
||||||
}
|
case _ =>
|
||||||
|
}
|
||||||
|
} else
|
||||||
|
callback.jsonRpcRespondError(
|
||||||
|
Some(r.id),
|
||||||
|
ErrorCodes.InvalidRequest,
|
||||||
|
s"'$attach' is not allowed before authentication."
|
||||||
|
)
|
||||||
}
|
}
|
||||||
private val responseHandler: Handler[JsonRpcResponseMessage] =
|
private val responseHandler: Handler[JsonRpcResponseMessage] =
|
||||||
callback => {
|
callback => {
|
||||||
|
|
@ -265,12 +273,14 @@ object VirtualTerminal {
|
||||||
private val notificationHandler: Handler[JsonRpcNotificationMessage] =
|
private val notificationHandler: Handler[JsonRpcNotificationMessage] =
|
||||||
callback => {
|
callback => {
|
||||||
case n if n.method == systemIn =>
|
case n if n.method == systemIn =>
|
||||||
import sjsonnew.BasicJsonProtocol.*
|
if (callback.isAuthenticated) {
|
||||||
n.params.flatMap(Converter.fromJson[Byte](_).toOption).foreach { byte =>
|
import sjsonnew.BasicJsonProtocol.*
|
||||||
StandardMain.exchange.channelForName(callback.name) match {
|
n.params.flatMap(Converter.fromJson[Byte](_).toOption).foreach { byte =>
|
||||||
case Some(nc: NetworkChannel) => nc.write(byte)
|
StandardMain.exchange.channelForName(callback.name) match {
|
||||||
case _ =>
|
case Some(nc: NetworkChannel) => nc.write(byte)
|
||||||
|
case _ =>
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
} else callback.log.warn(s"ignoring '$systemIn' before authentication")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,9 @@
|
||||||
|
scalaVersion := "3.8.4"
|
||||||
|
|
||||||
|
Global / serverConnectionType := ConnectionType.Tcp
|
||||||
|
Global / serverPort := 5002
|
||||||
|
|
||||||
|
lazy val root = (project in file("."))
|
||||||
|
.settings(
|
||||||
|
name := "tcp",
|
||||||
|
)
|
||||||
|
|
@ -0,0 +1,136 @@
|
||||||
|
/*
|
||||||
|
* sbt
|
||||||
|
* Copyright 2011 - 2018, Lightbend, Inc.
|
||||||
|
* Copyright 2008 - 2010, Mark Harrah
|
||||||
|
* Licensed under Apache License 2.0 (see LICENSE)
|
||||||
|
*/
|
||||||
|
|
||||||
|
package testpkg
|
||||||
|
|
||||||
|
import java.io.File
|
||||||
|
import java.nio.file.{ Files, Path }
|
||||||
|
import scala.concurrent.duration.*
|
||||||
|
|
||||||
|
import sbt.internal.langserver.{ CancelRequestParams, ErrorCodes, SbtExecParams }
|
||||||
|
import sbt.internal.langserver.codec.JsonProtocol.given
|
||||||
|
import sbt.protocol.{ Attach, CompletionParams, SettingQuery }
|
||||||
|
import sbt.protocol.codec.JsonProtocol.given
|
||||||
|
import sbt.io.IO
|
||||||
|
import sbt.io.syntax.*
|
||||||
|
import sbt.protocol.ServerSession
|
||||||
|
import sbt.{ ForkOptions, OutputStrategy, RunFromSourceMain }
|
||||||
|
import sjsonnew.JsonWriter
|
||||||
|
|
||||||
|
import org.scalatest.funsuite.AnyFunSuite
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reproduces the reported vulnerability: a TCP server configured with token auth
|
||||||
|
* (the default whenever `serverConnectionType` is Tcp) must reject requests that
|
||||||
|
* mutate or read through the server (`sbt/exec`, `textDocument/definition`) from a
|
||||||
|
* client that never completed a token-authenticated `initialize`. Unlike the other
|
||||||
|
* tests in this suite, these deliberately skip `ServerSession#initialize` to play
|
||||||
|
* the part of an attacker who can reach the socket but does not know the token.
|
||||||
|
*/
|
||||||
|
class ExecRequiresInitializeTest extends AnyFunSuite {
|
||||||
|
private val testDirectory = "tcp"
|
||||||
|
|
||||||
|
private val serverTestBase: File = {
|
||||||
|
val p0 = new File(".").getAbsoluteFile / "server-test" / "src" / "server-test"
|
||||||
|
val p1 = new File(".").getAbsoluteFile / "src" / "server-test"
|
||||||
|
if (p0.exists) p0 else p1
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Forks a real sbt server for `testDirectory`, connects a raw (un-initialized) session. */
|
||||||
|
private def withUnauthenticatedSession(f: ServerSession => Unit): Unit = {
|
||||||
|
val base: Path = Files.createTempDirectory(Path.of("/tmp"), "sbt-tcp-poc")
|
||||||
|
val buildDir = base.toFile / testDirectory
|
||||||
|
IO.copyDirectory(serverTestBase / testDirectory, buildDir)
|
||||||
|
info(s"test project created at: $buildDir")
|
||||||
|
|
||||||
|
val classpath = TestProperties.classpath.split(File.pathSeparator).map(new File(_))
|
||||||
|
val process = RunFromSourceMain.fork(
|
||||||
|
ForkOptions()
|
||||||
|
.withOutputStrategy(OutputStrategy.StdoutOutput)
|
||||||
|
.withRunJVMOptions(
|
||||||
|
Vector(
|
||||||
|
"-Djline.terminal=none",
|
||||||
|
"-Dsbt.io.virtual=false",
|
||||||
|
"-Dsbt.banner=false",
|
||||||
|
)
|
||||||
|
),
|
||||||
|
buildDir,
|
||||||
|
TestProperties.scalaVersion,
|
||||||
|
TestProperties.version,
|
||||||
|
classpath.toSeq
|
||||||
|
)
|
||||||
|
|
||||||
|
try {
|
||||||
|
val portfile = buildDir / "project" / "target" / "active.json"
|
||||||
|
ServerSession.waitForPortfile(portfile, process.isAlive())
|
||||||
|
|
||||||
|
val session = ServerSession.connect(portfile)
|
||||||
|
try
|
||||||
|
// Deliberately do NOT call session.initialize(...): this simulates an
|
||||||
|
// attacker who can reach the TCP socket but never authenticates.
|
||||||
|
f(session)
|
||||||
|
finally session.close()
|
||||||
|
} finally {
|
||||||
|
if (process.isAlive()) process.destroy()
|
||||||
|
IO.delete(base.toFile)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Sends `method`/`params` on `session` and asserts the server rejected it pre-auth. */
|
||||||
|
private def assertRejected[A: JsonWriter](session: ServerSession, method: String, params: A): Unit = {
|
||||||
|
val id = session.nextId()
|
||||||
|
session.sendJsonRpc(id, method, params).get
|
||||||
|
val response = session.waitForResponseMsg(30.seconds, id).get
|
||||||
|
|
||||||
|
assert(
|
||||||
|
response.error.isDefined,
|
||||||
|
s"$method should have been rejected before initialize, but got: $response"
|
||||||
|
)
|
||||||
|
assertResult(ErrorCodes.InvalidRequest)(response.error.get.code)
|
||||||
|
}
|
||||||
|
|
||||||
|
test("sbt/exec is rejected over TCP before a token-authenticated initialize") {
|
||||||
|
withUnauthenticatedSession { session =>
|
||||||
|
assertRejected(session, "sbt/exec", SbtExecParams("compile"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
test("textDocument/definition is rejected over TCP before a token-authenticated initialize") {
|
||||||
|
withUnauthenticatedSession { session =>
|
||||||
|
// The gate runs before params are ever parsed, so a bogus payload is enough
|
||||||
|
// to prove the request never reaches Definition.lspDefinition's file reads.
|
||||||
|
assertRejected(session, "textDocument/definition", "")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
test("sbt/setting is rejected over TCP before a token-authenticated initialize") {
|
||||||
|
withUnauthenticatedSession { session =>
|
||||||
|
assertRejected(session, "sbt/setting", SettingQuery("root/name"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
test("sbt/cancelRequest is rejected over TCP before a token-authenticated initialize") {
|
||||||
|
withUnauthenticatedSession { session =>
|
||||||
|
assertRejected(session, "sbt/cancelRequest", CancelRequestParams("some-id"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
test("sbt/completion is rejected over TCP before a token-authenticated initialize") {
|
||||||
|
withUnauthenticatedSession { session =>
|
||||||
|
assertRejected(session, "sbt/completion", CompletionParams("comp", None))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
test("sbt/attach is rejected over TCP before a token-authenticated initialize") {
|
||||||
|
// Regression for the interactive-attach bypass: without this gate, a client could
|
||||||
|
// attach and feed raw command bytes via sbt/systemIn, running commands without
|
||||||
|
// ever completing the token handshake that sbt/exec itself requires.
|
||||||
|
withUnauthenticatedSession { session =>
|
||||||
|
assertRejected(session, "sbt/attach", Attach(interactive = true))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue