Skip to content

Commit 1fee068

Browse files
cxzl25ulysses-you
authored andcommitted
[KYUUBI #2591] Redact secret information from ProcBuilder log
### _Why are the changes needed?_ Now the user can see the command to start the engine, which may have some sensitive information. Introduce a configuration item to support replacing sensitive information. For example, if you use the `kyuubi.ha.zookeeper.auth.digest` configuration, you can configure `kyuubi.server.redaction.regex` `(?i)zookeeper.auth.digest` close #2591 ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [x] 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 #2592 from cxzl25/KYUUBI-2591. Closes #2591 67567a2 [sychen] update doc eb1ec9a [sychen] redact kv fbcba2d [sychen] Merge branch 'master' into KYUUBI-2591 346e21b [sychen] kyuubi.server.redaction.regex Authored-by: sychen <sychen@ctrip.com> Signed-off-by: ulysses-you <ulyssesyou@apache.org>
1 parent 802890a commit 1fee068

File tree

10 files changed

+131
-6
lines changed

10 files changed

+131
-6
lines changed

docs/deployment/settings.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ Key | Default | Meaning | Type | Since
350350
<code>kyuubi.server.limit.connections.per.user</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Maximum kyuubi server connections per user. Any user exceeding this limit will not be allowed to connect.</div>|<div style='width: 30pt'>int</div>|<div style='width: 20pt'>1.6.0</div>
351351
<code>kyuubi.server.limit.connections.per.user.ipaddress</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Maximum kyuubi server connections per user:ipaddress combination. Any user-ipaddress exceeding this limit will not be allowed to connect.</div>|<div style='width: 30pt'>int</div>|<div style='width: 20pt'>1.6.0</div>
352352
<code>kyuubi.server.name</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>The name of Kyuubi Server.</div>|<div style='width: 30pt'>string</div>|<div style='width: 20pt'>1.5.0</div>
353+
<code>kyuubi.server.redaction.regex</code>|<div style='width: 65pt;word-wrap: break-word;white-space: normal'>&lt;undefined&gt;</div>|<div style='width: 170pt;word-wrap: break-word;white-space: normal'>Regex to decide which Kyuubi contain sensitive information. When this regex matches a property key or value, the value is redacted from the various logs.</div>|<div style='width: 30pt'></div>|<div style='width: 20pt'>1.6.0</div>
353354

354355

355356
### Session

kyuubi-common/src/main/scala/org/apache/kyuubi/Utils.scala

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import java.util.{Properties, TimeZone, UUID}
2525

2626
import scala.collection.JavaConverters._
2727
import scala.util.control.NonFatal
28+
import scala.util.matching.Regex
2829

2930
import org.apache.commons.lang3.SystemUtils
3031
import org.apache.commons.lang3.time.DateFormatUtils
@@ -288,4 +289,53 @@ object Utils extends Logging {
288289
}
289290
}
290291
}
292+
293+
val REDACTION_REPLACEMENT_TEXT = "*********(redacted)"
294+
295+
private val PATTERN_FOR_KEY_VALUE_ARG = "(.+?)=(.+)".r
296+
297+
def redactCommandLineArgs(conf: KyuubiConf, commands: Array[String]): Array[String] = {
298+
val redactionPattern = conf.get(SERVER_SECRET_REDACTION_PATTERN)
299+
var nextKV = false
300+
commands.map {
301+
case PATTERN_FOR_KEY_VALUE_ARG(key, value) if nextKV =>
302+
val (_, newValue) = redact(redactionPattern, Seq((key, value))).head
303+
nextKV = false
304+
s"$key=$newValue"
305+
306+
case cmd if cmd == "--conf" =>
307+
nextKV = true
308+
cmd
309+
310+
case cmd =>
311+
cmd
312+
}
313+
}
314+
315+
/**
316+
* Redact the sensitive values in the given map. If a map key matches the redaction pattern then
317+
* its value is replaced with a dummy text.
318+
*/
319+
def redact[K, V](regex: Option[Regex], kvs: Seq[(K, V)]): Seq[(K, V)] = {
320+
regex match {
321+
case None => kvs
322+
case Some(r) => redact(r, kvs)
323+
}
324+
}
325+
326+
private def redact[K, V](redactionPattern: Regex, kvs: Seq[(K, V)]): Seq[(K, V)] = {
327+
kvs.map {
328+
case (key: String, value: String) =>
329+
redactionPattern.findFirstIn(key)
330+
.orElse(redactionPattern.findFirstIn(value))
331+
.map { _ => (key, REDACTION_REPLACEMENT_TEXT) }
332+
.getOrElse((key, value))
333+
case (key, value: String) =>
334+
redactionPattern.findFirstIn(value)
335+
.map { _ => (key, REDACTION_REPLACEMENT_TEXT) }
336+
.getOrElse((key, value))
337+
case (key, value) =>
338+
(key, value)
339+
}.asInstanceOf[Seq[(K, V)]]
340+
}
291341
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
package org.apache.kyuubi.config
1919

2020
import java.time.Duration
21+
import java.util.regex.PatternSyntaxException
2122

2223
import scala.util.{Failure, Success, Try}
24+
import scala.util.matching.Regex
2325

2426
private[kyuubi] case class ConfigBuilder(key: String) {
2527

@@ -119,6 +121,18 @@ private[kyuubi] case class ConfigBuilder(key: String) {
119121
_onCreate.foreach(_(entry))
120122
entry
121123
}
124+
125+
def regexConf: TypedConfigBuilder[Regex] = {
126+
def regexFromString(str: String, key: String): Regex = {
127+
try str.r
128+
catch {
129+
case e: PatternSyntaxException =>
130+
throw new IllegalArgumentException(s"$key should be a regex, but was $str", e)
131+
}
132+
}
133+
134+
new TypedConfigBuilder(this, regexFromString(_, this.key), _.toString)
135+
}
122136
}
123137

124138
private[kyuubi] case class TypedConfigBuilder[T](

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import java.util.concurrent.ConcurrentHashMap
2323
import java.util.regex.Pattern
2424

2525
import scala.collection.JavaConverters._
26+
import scala.util.matching.Regex
2627

2728
import org.apache.kyuubi.{Logging, Utils}
2829
import org.apache.kyuubi.config.KyuubiConf._
@@ -1490,4 +1491,12 @@ object KyuubiConf {
14901491
.version("1.6.0")
14911492
.booleanConf
14921493
.createWithDefault(false)
1494+
1495+
val SERVER_SECRET_REDACTION_PATTERN: OptionalConfigEntry[Regex] =
1496+
buildConf("kyuubi.server.redaction.regex")
1497+
.doc("Regex to decide which Kyuubi contain sensitive information. When this regex matches " +
1498+
"a property key or value, the value is redacted from the various logs.")
1499+
.version("1.6.0")
1500+
.regexConf
1501+
.createOptional
14931502
}

kyuubi-common/src/test/scala/org/apache/kyuubi/UtilsSuite.scala

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,12 @@ import java.nio.file.{Files, Paths}
2323
import java.security.PrivilegedExceptionAction
2424
import java.util.Properties
2525

26+
import scala.collection.mutable.ArrayBuffer
27+
2628
import org.apache.hadoop.security.UserGroupInformation
2729

2830
import org.apache.kyuubi.config.KyuubiConf
31+
import org.apache.kyuubi.config.KyuubiConf.SERVER_SECRET_REDACTION_PATTERN
2932

3033
class UtilsSuite extends KyuubiFunSuite {
3134

@@ -160,4 +163,51 @@ class UtilsSuite extends KyuubiFunSuite {
160163
val exception2 = intercept[IllegalArgumentException](Utils.fromCommandLineArgs(args2, conf))
161164
assert(exception2.getMessage.contains("Illegal argument: a"))
162165
}
166+
167+
test("redact sensitive information in command line args") {
168+
val conf = new KyuubiConf()
169+
conf.set(SERVER_SECRET_REDACTION_PATTERN, "(?i)secret|password".r)
170+
171+
val buffer = new ArrayBuffer[String]()
172+
buffer += "main"
173+
buffer += "--conf"
174+
buffer += "kyuubi.my.password=sensitive_value"
175+
buffer += "--conf"
176+
buffer += "kyuubi.regular.property1=regular_value"
177+
buffer += "--conf"
178+
buffer += "kyuubi.my.secret=sensitive_value"
179+
buffer += "--conf"
180+
buffer += "kyuubi.regular.property2=regular_value"
181+
182+
val commands = buffer.toArray
183+
184+
// Redact sensitive information
185+
val redactedCmdArgs = Utils.redactCommandLineArgs(conf, commands)
186+
187+
val expectBuffer = new ArrayBuffer[String]()
188+
expectBuffer += "main"
189+
expectBuffer += "--conf"
190+
expectBuffer += "kyuubi.my.password=" + Utils.REDACTION_REPLACEMENT_TEXT
191+
expectBuffer += "--conf"
192+
expectBuffer += "kyuubi.regular.property1=regular_value"
193+
expectBuffer += "--conf"
194+
expectBuffer += "kyuubi.my.secret=" + Utils.REDACTION_REPLACEMENT_TEXT
195+
expectBuffer += "--conf"
196+
expectBuffer += "kyuubi.regular.property2=regular_value"
197+
198+
assert(expectBuffer.toArray === redactedCmdArgs)
199+
}
200+
201+
test("redact sensitive information") {
202+
val secretKeys = Some("my.password".r)
203+
assert(Utils.redact(secretKeys, Seq(("kyuubi.my.password", "12345"))) ===
204+
Seq(("kyuubi.my.password", Utils.REDACTION_REPLACEMENT_TEXT)))
205+
assert(Utils.redact(secretKeys, Seq(("anything", "kyuubi.my.password=12345"))) ===
206+
Seq(("anything", Utils.REDACTION_REPLACEMENT_TEXT)))
207+
assert(Utils.redact(secretKeys, Seq((999, "kyuubi.my.password=12345"))) ===
208+
Seq((999, Utils.REDACTION_REPLACEMENT_TEXT)))
209+
// Do not redact when value type is not string
210+
assert(Utils.redact(secretKeys, Seq(("my.password", 12345))) ===
211+
Seq(("my.password", 12345)))
212+
}
163213
}

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,8 @@ private[kyuubi] class EngineRef(
185185

186186
MetricsSystem.tracing(_.incCount(ENGINE_TOTAL))
187187
try {
188-
info(s"Launching engine:\n$builder")
188+
val redactedCmd = builder.toString
189+
info(s"Launching engine:\n$redactedCmd")
189190
val process = builder.start
190191
var exitValue: Option[Int] = None
191192
while (engineRef.isEmpty) {
@@ -205,7 +206,7 @@ private[kyuubi] class EngineRef(
205206
process.destroyForcibly()
206207
MetricsSystem.tracing(_.incCount(MetricRegistry.name(ENGINE_TIMEOUT, appUser)))
207208
throw KyuubiSQLException(
208-
s"Timeout($timeout ms) to launched $engineType engine with $builder. $killMessage",
209+
s"Timeout($timeout ms) to launched $engineType engine with $redactedCmd. $killMessage",
209210
builder.getError)
210211
}
211212
engineRef = discoveryClient.getEngineByRefId(engineSpace, engineRefId)

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/ProcBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ trait ProcBuilder {
270270
if (commands == null) {
271271
super.toString()
272272
} else {
273-
commands.map {
273+
Utils.redactCommandLineArgs(conf, commands).map {
274274
case arg if arg.startsWith("--") => s"\\\n\t$arg"
275275
case arg => arg
276276
}.mkString(" ")

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ class HiveProcessBuilder(
105105
buffer.toArray
106106
}
107107

108-
override def toString: String = commands.mkString("\n")
108+
override def toString: String = Utils.redactCommandLineArgs(conf, commands).mkString("\n")
109109

110110
override def shortName: String = "hive"
111111
}

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class SparkProcessBuilder(
5555

5656
var allConf = conf.getAll
5757

58-
// if enable sasl kerberos authentication for zookeeper, need to upload the server ketab file
58+
// if enable sasl kerberos authentication for zookeeper, need to upload the server keytab file
5959
if (AuthTypes.withName(conf.get(HighAvailabilityConf.HA_ZK_ENGINE_AUTH_TYPE))
6060
== AuthTypes.KERBEROS) {
6161
allConf = allConf ++ zkAuthKeytabFileConf(allConf)

kyuubi-server/src/main/scala/org/apache/kyuubi/engine/trino/TrinoProcessBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,5 +98,5 @@ class TrinoProcessBuilder(
9898

9999
override def shortName: String = "trino"
100100

101-
override def toString: String = commands.mkString("\n")
101+
override def toString: String = Utils.redactCommandLineArgs(conf, commands).mkString("\n")
102102
}

0 commit comments

Comments
 (0)