Skip to content

Commit d75f48e

Browse files
committed
[KYUUBI #3052][FOLLOWUP] Do not use the ip in proxy http header for authentication to prevent CVE
### _Why are the changes needed?_ Because the client can specify any ip in http header, to prevent CVE issue, we do not use it for authentication. ### _How was this patch tested?_ - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #3078 from turboFei/http_real_ip_cve. Closes #3052 e7c41ea [Fei Wang] prevent cve Authored-by: Fei Wang <fwang12@ebay.com> Signed-off-by: Fei Wang <fwang12@ebay.com>
1 parent 8f3d789 commit d75f48e

File tree

10 files changed

+50
-18
lines changed

10 files changed

+50
-18
lines changed

docs/deployment/settings.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ kyuubi.frontend.mysql.min.worker.threads|9|Minimum number of threads in the comm
273273
kyuubi.frontend.mysql.netty.worker.threads|&lt;undefined&gt;|Number of thread in the netty worker event loop of MySQL frontend service. Use min(cpu_cores, 8) in default.|int|1.4.0
274274
kyuubi.frontend.mysql.worker.keepalive.time|PT1M|Time(ms) that an idle async thread of the command execution thread pool will wait for a new task to arrive before terminating in MySQL frontend service|duration|1.4.0
275275
kyuubi.frontend.protocols|THRIFT_BINARY|A comma separated list for all frontend protocols <ul> <li>THRIFT_BINARY - HiveServer2 compatible thrift binary protocol.</li> <li>THRIFT_HTTP - HiveServer2 compatible thrift http protocol.</li> <li>REST - Kyuubi defined REST API(experimental).</li> <li>MYSQL - MySQL compatible text protocol(experimental).</li> </ul>|seq|1.4.0
276-
kyuubi.frontend.proxy.http.client.ip.header|X-Real-IP|The http header to record the real client ip address. If your server is behind a load balancer or other proxy, the server will see this load balancer or proxy IP address as the client IP address, to get around this common issue, most load balancers or proxies offer the ability to record the real remote IP address in an HTTP herader that will be added to the request for other devices to use.|string|1.6.0
276+
kyuubi.frontend.proxy.http.client.ip.header|X-Real-IP|The http header to record the real client ip address. If your server is behind a load balancer or other proxy, the server will see this load balancer or proxy IP address as the client IP address, to get around this common issue, most load balancers or proxies offer the ability to record the real remote IP address in an HTTP header that will be added to the request for other devices to use. Note that, because the header value can be specified to any ip address, so it will not be used for authentication.|string|1.6.0
277277
kyuubi.frontend.rest.bind.host|&lt;undefined&gt;|Hostname or IP of the machine on which to run the REST frontend service.|string|1.4.0
278278
kyuubi.frontend.rest.bind.port|10099|Port of the machine on which to run the REST frontend service.|int|1.4.0
279279
kyuubi.frontend.thrift.backoff.slot.length|PT0.1S|Time to back off during login to the thrift frontend service.|duration|1.4.0

kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,8 +585,9 @@ object KyuubiConf {
585585
.doc("The http header to record the real client ip address. If your server is behind a load" +
586586
" balancer or other proxy, the server will see this load balancer or proxy IP address as" +
587587
" the client IP address, to get around this common issue, most load balancers or proxies" +
588-
" offer the ability to record the real remote IP address in an HTTP herader that will be" +
589-
" added to the request for other devices to use.")
588+
" offer the ability to record the real remote IP address in an HTTP header that will be" +
589+
" added to the request for other devices to use. Note that, because the header value can" +
590+
" be specified to any ip address, so it will not be used for authentication.")
590591
.version("1.6.0")
591592
.stringConf
592593
.createWithDefault("X-Real-IP")

kyuubi-server/src/main/scala/org/apache/kyuubi/server/KyuubiRestFrontendService.scala

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,16 @@ class KyuubiRestFrontendService(override val serverable: Serverable)
173173
}
174174

175175
def getUserName(sessionConf: Map[String, String]): String = {
176+
// using the remote ip address instead of that in proxy http header for authentication
177+
val ipAddress = AuthenticationFilter.getUserIpAddress
176178
val realUser: String = ServiceUtils.getShortName(
177179
Option(AuthenticationFilter.getUserName).filter(_.nonEmpty).getOrElse("anonymous"))
178-
getProxyUser(sessionConf, Option(AuthenticationFilter.getUserIpAddress).orNull, realUser)
180+
getProxyUser(sessionConf, ipAddress, realUser)
181+
}
182+
183+
def getIpAddress: String = {
184+
Option(AuthenticationFilter.getUserProxyHeaderIpAddress).getOrElse(
185+
AuthenticationFilter.getUserIpAddress)
179186
}
180187

181188
private def getProxyUser(

kyuubi-server/src/main/scala/org/apache/kyuubi/server/KyuubiTHttpFrontendService.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,14 +261,16 @@ final class KyuubiTHttpFrontendService(
261261
}
262262

263263
override protected def getIpAddress: String = {
264-
SessionManager.getIpAddress
264+
Option(SessionManager.getProxyHttpHeaderIpAddress).getOrElse(SessionManager.getIpAddress)
265265
}
266266

267267
override protected def getUserName(req: TOpenSessionReq): String = {
268268
var userName: String = SessionManager.getUserName
269+
// using the remote ip address instead of that in proxy http header for authentication
270+
val ipAddress: String = SessionManager.getIpAddress
269271
if (userName == null) userName = req.getUsername
270272
userName = getShortName(userName)
271-
val effectiveClientUser: String = getProxyUser(req.getConfiguration, getIpAddress, userName)
273+
val effectiveClientUser: String = getProxyUser(req.getConfiguration, ipAddress, userName)
272274
debug("Client's username: " + effectiveClientUser)
273275
effectiveClientUser
274276
}

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/AdminResource.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import io.swagger.v3.oas.annotations.tags.Tag
2727
import org.apache.kyuubi.{Logging, Utils}
2828
import org.apache.kyuubi.server.KyuubiServer
2929
import org.apache.kyuubi.server.api.ApiRequestContext
30-
import org.apache.kyuubi.server.http.authentication.AuthenticationFilter
3130

3231
@Tag(name = "Admin")
3332
@Produces(Array(MediaType.APPLICATION_JSON))
@@ -44,7 +43,7 @@ private[v1] class AdminResource extends ApiRequestContext with Logging {
4443
@Path("refresh/hadoop_conf")
4544
def refreshFrontendHadoopConf(): Response = {
4645
val userName = fe.getUserName(Map.empty)
47-
val ipAddress = AuthenticationFilter.getUserIpAddress
46+
val ipAddress = fe.getIpAddress
4847
info(s"Receive refresh Kyuubi server hadoop conf request from $userName/$ipAddress")
4948
if (!userName.equals(administrator)) {
5049
throw new NotAllowedException(

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/BatchesResource.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import org.apache.kyuubi.engine.ApplicationOperation._
3838
import org.apache.kyuubi.operation.{BatchJobSubmission, FetchOrientation, OperationState}
3939
import org.apache.kyuubi.server.api.ApiRequestContext
4040
import org.apache.kyuubi.server.api.v1.BatchesResource._
41-
import org.apache.kyuubi.server.http.authentication.AuthenticationFilter
4241
import org.apache.kyuubi.server.metadata.MetadataManager
4342
import org.apache.kyuubi.server.metadata.api.Metadata
4443
import org.apache.kyuubi.service.authentication.KyuubiAuthenticationFactory
@@ -168,7 +167,7 @@ private[v1] class BatchesResource extends ApiRequestContext with Logging {
168167
request.setBatchType(request.getBatchType.toUpperCase(Locale.ROOT))
169168

170169
val userName = fe.getUserName(request.getConf.asScala.toMap)
171-
val ipAddress = AuthenticationFilter.getUserIpAddress
170+
val ipAddress = fe.getIpAddress
172171
request.setConf(
173172
(request.getConf.asScala ++ Map(KYUUBI_CLIENT_IP_KEY -> ipAddress)).asJava)
174173
val sessionHandle = sessionManager.openBatchSession(

kyuubi-server/src/main/scala/org/apache/kyuubi/server/api/v1/SessionsResource.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import org.apache.kyuubi.client.api.v1.dto._
3535
import org.apache.kyuubi.events.KyuubiEvent
3636
import org.apache.kyuubi.operation.OperationHandle
3737
import org.apache.kyuubi.server.api.ApiRequestContext
38-
import org.apache.kyuubi.server.http.authentication.AuthenticationFilter
3938
import org.apache.kyuubi.session.KyuubiSession
4039
import org.apache.kyuubi.session.SessionHandle
4140

@@ -142,7 +141,7 @@ private[v1] class SessionsResource extends ApiRequestContext with Logging {
142141
@Consumes(Array(MediaType.APPLICATION_JSON))
143142
def openSession(request: SessionOpenRequest): dto.SessionHandle = {
144143
val userName = fe.getUserName(request.getConfigs.asScala.toMap)
145-
val ipAddress = AuthenticationFilter.getUserIpAddress
144+
val ipAddress = fe.getIpAddress
146145
val handle = fe.be.openSession(
147146
TProtocolVersion.findByValue(request.getProtocolVersion),
148147
userName,

kyuubi-server/src/main/scala/org/apache/kyuubi/server/http/ThriftHttpServlet.scala

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ class ThriftHttpServlet(
7979
@throws[IOException]
8080
override def doPost(request: HttpServletRequest, response: HttpServletResponse): Unit = {
8181
var clientUserName: String = null
82-
var clientIpAddress: String = null
8382
var requireNewCookie: Boolean = false
8483
try {
8584
if (conf.get(KyuubiConf.FRONTEND_THRIFT_HTTP_XSRF_FILTER_ENABLED)) {
@@ -119,12 +118,16 @@ class ThriftHttpServlet(
119118
val doAsQueryParam = getDoAsQueryParam(request.getQueryString)
120119
if (doAsQueryParam != null) SessionManager.setProxyUserName(doAsQueryParam)
121120

122-
clientIpAddress = Option(request.getHeader(conf.get(FRONTEND_PROXY_HTTP_CLIENT_IP_HEADER)))
123-
.getOrElse(request.getRemoteAddr)
121+
val clientIpAddress = request.getRemoteAddr
124122
debug("Client IP Address: " + clientIpAddress)
125-
// Set the thread local ip address
126123
SessionManager.setIpAddress(clientIpAddress)
127124

125+
Option(request.getHeader(conf.get(FRONTEND_PROXY_HTTP_CLIENT_IP_HEADER))).foreach {
126+
ipAddress =>
127+
debug("Proxy Http Header Client IP Address: " + ipAddress)
128+
SessionManager.setProxyHttpHeaderIpAddress(ipAddress)
129+
}
130+
128131
val forwarded_for = request.getHeader(X_FORWARDED_FOR_HEADER)
129132
if (forwarded_for != null) {
130133
debug(X_FORWARDED_FOR_HEADER + ":" + forwarded_for)
@@ -157,6 +160,7 @@ class ThriftHttpServlet(
157160
// Clear the thread locals
158161
SessionManager.clearUserName()
159162
SessionManager.clearIpAddress()
163+
SessionManager.clearProxyHttpHeaderIpAddress()
160164
SessionManager.clearProxyUserName()
161165
SessionManager.clearForwardedAddresses()
162166
}

kyuubi-server/src/main/scala/org/apache/kyuubi/server/http/authentication/AuthenticationFilter.scala

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,9 @@ class AuthenticationFilter(conf: KyuubiConf) extends Filter with Logging {
114114
HttpServletResponse.SC_UNAUTHORIZED,
115115
s"No auth scheme matched for $authorization")
116116
} else {
117-
HTTP_CLIENT_IP_ADDRESS.set(Option(httpRequest.getHeader(
118-
conf.get(FRONTEND_PROXY_HTTP_CLIENT_IP_HEADER))).getOrElse(httpRequest.getRemoteAddr))
117+
HTTP_CLIENT_IP_ADDRESS.set(httpRequest.getRemoteAddr)
118+
HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS.set(
119+
httpRequest.getHeader(conf.get(FRONTEND_PROXY_HTTP_CLIENT_IP_HEADER)))
119120
try {
120121
val authUser = matchedHandler.authenticate(httpRequest, httpResponse)
121122
if (authUser != null) {
@@ -126,6 +127,7 @@ class AuthenticationFilter(conf: KyuubiConf) extends Filter with Logging {
126127
case e: AuthenticationException =>
127128
HTTP_CLIENT_USER_NAME.remove()
128129
HTTP_CLIENT_IP_ADDRESS.remove()
130+
HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS.remove()
129131
httpResponse.setStatus(HttpServletResponse.SC_FORBIDDEN)
130132
httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, e.getMessage)
131133
}
@@ -163,11 +165,16 @@ object AuthenticationFilter {
163165
final val HTTP_CLIENT_IP_ADDRESS = new ThreadLocal[String]() {
164166
override protected def initialValue: String = null
165167
}
168+
final val HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS = new ThreadLocal[String]() {
169+
override protected def initialValue: String = null
170+
}
166171
final val HTTP_CLIENT_USER_NAME = new ThreadLocal[String]() {
167172
override protected def initialValue: String = null
168173
}
169174

170175
def getUserIpAddress: String = HTTP_CLIENT_IP_ADDRESS.get
171176

177+
def getUserProxyHeaderIpAddress: String = HTTP_PROXY_HEADER_CLIENT_IP_ADDRESS.get()
178+
172179
def getUserName: String = HTTP_CLIENT_USER_NAME.get
173180
}

kyuubi-server/src/main/scala/org/apache/kyuubi/server/http/util/SessionManager.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,20 @@ object SessionManager extends Logging {
3535
threadLocalIpAddress.get
3636
}
3737

38+
private val threadLocalProxyHttpHeaderIpAddress: ThreadLocal[String] = new ThreadLocal[String]
39+
40+
def setProxyHttpHeaderIpAddress(realIpAddress: String): Unit = {
41+
threadLocalProxyHttpHeaderIpAddress.set(realIpAddress)
42+
}
43+
44+
def clearProxyHttpHeaderIpAddress(): Unit = {
45+
threadLocalProxyHttpHeaderIpAddress.remove()
46+
}
47+
48+
def getProxyHttpHeaderIpAddress: String = {
49+
threadLocalProxyHttpHeaderIpAddress.get
50+
}
51+
3852
private val threadLocalForwardedAddresses: ThreadLocal[List[String]] =
3953
new ThreadLocal[List[String]]
4054

0 commit comments

Comments
 (0)