Skip to content

Commit

Permalink
[KYUUBI #3930] [Fix] fix ut in SessionSigningSuite by getting session…
Browse files Browse the repository at this point in the history
… public key and user sign from LocalProperty

### _Why are the changes needed?_

Followed by issue #3930 and pr #3932.

- correct ut in SessionSigningSuite by fetching session public key and user sign via scala scripts instead of `SET` command in SQL , to prevent accidently redacted by `spark.redaction.regex`
- removed `syncronized` in SignUtils introduced in pr #3932 , with confirming no thread safe problem during the signing and verification

### _How was this patch tested?_
- [x] 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 #3974 from bowenliang123/3932-fix2.

Closes #3930

c5f07b9 [liangbowen] comment
1f4ace3 [liangbowen] use KyuubiReservedKeys.KYUUBI_SESSION_USER_SIGN/KYUUBI_SESSION_SIGN_PUBLICKEY for property name
a034cbe [liangbowen] style
1f127c1 [liangbowen] remove `synchronized` from `signWithPrivateKey` and `verifySignWithECDSA` in SignUtils
ff5a21d [liangbowen] fix ut by getting `kyuubi.session.sign.publickey` and `kyuubi.session.user.sign` via `spark.sparkContext.getLocalProperty` in scala scripts.

Authored-by: liangbowen <liangbowen@gf.com.cn>
Signed-off-by: ulysses-you <ulyssesyou@apache.org>
  • Loading branch information
bowenliang123 authored and ulysses-you committed Dec 13, 2022
1 parent 827ae40 commit 36a2d33
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ object SignUtils {
def signWithPrivateKey(
plainText: String,
privateKey: PrivateKey,
algorithm: String = "SHA256withECDSA"): String = synchronized {
algorithm: String = "SHA256withECDSA"): String = {
val privateSignature = Signature.getInstance(algorithm)
privateSignature.initSign(privateKey)
privateSignature.update(plainText.getBytes(StandardCharsets.UTF_8))
Expand All @@ -57,7 +57,7 @@ object SignUtils {
def verifySignWithECDSA(
plainText: String,
signatureBase64: String,
publicKeyBase64: String): Boolean = synchronized {
publicKeyBase64: String): Boolean = {
try {
val publicKeyBytes = Base64.getDecoder.decode(publicKeyBase64)
val publicKey: PublicKey = KeyFactory.getInstance(KEYPAIR_ALGORITHM_EC)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.apache.kyuubi.session

import org.apache.commons.lang3.StringUtils

import org.apache.kyuubi.WithKyuubiServer
import org.apache.kyuubi.config.KyuubiConf
import org.apache.kyuubi.config.KyuubiConf._
Expand Down Expand Up @@ -68,19 +70,29 @@ class SessionSigningSuite extends WithKyuubiServer with HiveJDBCTestHelper {
test("KYUUBI #3839 session user sign - check session user sign") {
withSessionConf()()() {
withJdbcStatement() { statement =>
val rs1 = statement.executeQuery(s"SET $KYUUBI_SESSION_SIGN_PUBLICKEY")
val value = s"SET ${OPERATION_LANGUAGE.key}=scala"
statement.executeQuery(value)

val rs1 = statement.executeQuery(
s"""
|spark.sparkContext.getLocalProperty("$KYUUBI_SESSION_SIGN_PUBLICKEY")
|""".stripMargin)
assert(rs1.next())
assert(rs1.getString("value").nonEmpty)

val rs2 = statement.executeQuery(s"SET $KYUUBI_SESSION_USER_SIGN")
val rs2 = statement.executeQuery(
s"""
|spark.sparkContext.getLocalProperty("$KYUUBI_SESSION_USER_SIGN")
|""".stripMargin)
assert(rs2.next())
assert(rs2.getString("value").nonEmpty)

val publicKeyStr = rs1.getString("value")
val sessionUserSign = rs2.getString("value")
// skipping prefix "res0: String = " of returned scala result
val publicKeyStr = rs1.getString(1).substring(15)
val sessionUserSign = rs2.getString(1).substring(15)

assert(StringUtils.isNotBlank(publicKeyStr))
assert(StringUtils.isNotBlank(sessionUserSign))
assert(SignUtils.verifySignWithECDSA(user, sessionUserSign, publicKeyStr))
}
}
}

}

0 comments on commit 36a2d33

Please sign in to comment.