Skip to content

Commit

Permalink
Properly handle pgp keychains generated by Scala CLI (#1987)
Browse files Browse the repository at this point in the history 
* Make config --create-pgp-key require password or explicit 'none' or 'random'

* Merge password and password-value options

* Update documentation regarding PGP

* NIT: change option description

* Remove warning about missing password in config, prevent secret values leaking to publish-conf.scala
  • Loading branch information
@MaciejG604
MaciejG604 committed Apr 5, 2023
1 parent 0226124 commit bcc68de
Show file tree
Hide file tree
Showing 12 changed files with 259 additions and 112 deletions.
10 changes: 10 additions & 0 deletions modules/build/src/main/scala/scala/build/internal/util/WarningMessages.scala
View file
Expand Up @@ -19,4 +19,14 @@ object WarningMessages {

def experimentalConfigKeyUsed(name: String): String =
experimentalFeatureUsed(s"The '$name' configuration key")

def rawValueNotWrittenToPublishFile(
rawValue: String,
valueName: String,
directiveName: String
): String =
s"""The value of $valueName ${Console.BOLD}will not${Console.RESET} be written to a potentially public file!
|Provide it as an option to the publish subcommand with:
| $directiveName value:$rawValue
|""".stripMargin
}
40 changes: 30 additions & 10 deletions modules/cli/src/main/scala/scala/cli/commands/config/Config.scala
View file
Expand Up @@ -51,10 +51,15 @@ object Config extends ScalaCommand[ConfigOptions] {
args.all match {
case Seq() =>
if (options.createPgpKey) {
val coursierCache = options.coursier.coursierCache(logger.coursierLogger(""))
val secKeyEntry = Keys.pgpSecretKey
val secKeyPasswordEntry = Keys.pgpSecretKeyPassword
val pubKeyEntry = Keys.pgpPublicKey
if (options.pgpPassword.isEmpty) {
logger.error(
s"--pgp-password not specified, use 'none' to create an unprotected keychain or 'random' to generate a password"
)
sys.exit(1)
}
val coursierCache = options.coursier.coursierCache(logger.coursierLogger(""))
val secKeyEntry = Keys.pgpSecretKey
val pubKeyEntry = Keys.pgpPublicKey

val mail = options.email
.filter(_.trim.nonEmpty)
Expand All @@ -64,17 +69,23 @@ object Config extends ScalaCommand[ConfigOptions] {
.orExit(logger)
}
.getOrElse {
System.err.println(
s"Error: --email ... not specified, and ${Keys.userEmail.fullName} not set (either is required to generate a PGP key)"
logger.error(
s"--email ... not specified, and ${Keys.userEmail.fullName} not set (either is required to generate a PGP key)"
)
sys.exit(1)
}

val password = ThrowawayPgpSecret.pgpPassPhrase()
val passwordOpt = if (options.pgpPassword.contains("none"))
None
else if (options.pgpPassword.contains("random"))
Some(ThrowawayPgpSecret.pgpPassPhrase())
else
options.pgpPassword.map(scala.cli.signing.shared.Secret.apply)

val (pgpPublic, pgpSecret0) =
ThrowawayPgpSecret.pgpSecret(
mail,
Some(password),
passwordOpt,
logger,
coursierCache,
() =>
Expand All @@ -88,11 +99,20 @@ object Config extends ScalaCommand[ConfigOptions] {
val pgpSecretBase64 = pgpSecret0.map(Base64.getEncoder.encodeToString)

db.set(secKeyEntry, PasswordOption.Value(pgpSecretBase64.toConfig))
db.set(secKeyPasswordEntry, PasswordOption.Value(password.toConfig))
db.set(pubKeyEntry, PasswordOption.Value(pgpPublic.toConfig))
db.save(directories.dbPath.toNIO)
.wrapConfigException
.orExit(logger)

logger.message("PGP keychains written to config")
if (options.pgpPassword.contains("random"))
passwordOpt.foreach { password =>
println(
s"""Password: ${password.value}
|Don't lose it!
|""".stripMargin
)
}
}
else {
System.err.println("No argument passed")
Expand Down Expand Up @@ -122,7 +142,7 @@ object Config extends ScalaCommand[ConfigOptions] {
valueOpt match {
case Some(value) =>
for (v <- value)
if (options.password && entry.isPasswordOption)
if (options.passwordValue && entry.isPasswordOption)
PasswordOption.parse(v) match {
case Left(err) =>
System.err.println(err)
Expand Down
22 changes: 14 additions & 8 deletions modules/cli/src/main/scala/scala/cli/commands/config/ConfigOptions.scala
View file
Expand Up @@ -27,20 +27,26 @@ final case class ConfigOptions(
@Tag(tags.inShortHelp)
dump: Boolean = false,
@Group(HelpGroup.Config.toString)
@HelpMessage("Create PGP key in config")
@HelpMessage("Create PGP keychain in config")
@Tag(tags.inShortHelp)
@Tag(tags.experimental)
createPgpKey: Boolean = false,
@Group(HelpGroup.Config.toString)
@HelpMessage("Email to use to create PGP key in config")
@HelpMessage("A password used to encode the private PGP keychain")
@Tag(tags.experimental)
email: Option[String] = None,
@ValueDescription("YOUR_PASSWORD|random|none")
@ExtraName("passphrase")
pgpPassword: Option[String] = None,
@Group(HelpGroup.Config.toString)
@HelpMessage("If the entry is a password, print the password value rather than how to get the password")
@Tag(tags.restricted)
@Tag(tags.inShortHelp)
password: Boolean = false,
@HelpMessage("Email used to create the PGP keychains in config")
@Tag(tags.experimental)
email: Option[String] = None,
@Group(HelpGroup.Config.toString)
@HelpMessage("If the entry is a password, save the password value rather than how to get the password")
@HelpMessage(
"""When accessing config's content print the password value rather than how to get the password
|When saving an entry in config save the password value rather than how to get the password
|e.g. print/save the value of environment variable ENV_VAR rather than "env:ENV_VAR"
|""".stripMargin)
@Tag(tags.restricted)
@Tag(tags.inShortHelp)
passwordValue: Boolean = false,
Expand Down
51 changes: 33 additions & 18 deletions modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala
View file
Expand Up @@ -11,6 +11,7 @@ import scala.build.EitherCps.{either, value}
import scala.build.Logger
import scala.build.Ops.*
import scala.build.errors.{BuildException, CompositeBuildException, MalformedCliInputError}
import scala.build.internal.util.WarningMessages
import scala.build.options.publish.ConfigPasswordOption
import scala.build.options.publish.ConfigPasswordOption.*
import scala.build.options.PublishOptions as BPublishOptions
Expand Down Expand Up @@ -321,8 +322,6 @@ final case class PgpSecretKeyCheck(

if (keysFromConfig.publicKeyOpt.isEmpty)
logger.message(" warning: no PGP public key found in config")
if (keysFromConfig.secretKeyPasswordOpt.isEmpty)
logger.message(" warning: no PGP secret key password found in config")

(keysFromConfig, true)
}
Expand Down Expand Up @@ -392,34 +391,50 @@ final case class PgpSecretKeyCheck(
scala.util.Try(path.relativeTo(os.pwd))
.map(p => s"file:${p.toString}")
.getOrElse(optionValue)

}
else optionValue
case ConfigOption(fullName) => s"config:$fullName"
}

val passwordDirectives = getDirectiveValue(setupKeys.secretKeyPasswordOpt).map {
"publish.secretKeyPassword" -> _
}.toSeq

val secretKeyDirValue = getDirectiveValue(setupKeys.secretKeyOpt)
val rawValueRegex = "^value:(.*)".r

// Prevent potential leakage of a secret value
val passwordDirectives = getDirectiveValue(setupKeys.secretKeyPasswordOpt)
.flatMap {
case rawValueRegex(rawValue) =>
logger.diagnostic(
WarningMessages.rawValueNotWrittenToPublishFile(
rawValue,
"PGP password",
"--secret-key-password"
)
)
None
case secretOption => Some("publish.secretKeyPassword" -> secretOption)
}
.toSeq

// Prevent potential leakage of a secret value
val secretKeyDirValue = getDirectiveValue(setupKeys.secretKeyOpt).flatMap {
case rawValueRegex(rawValue) =>
logger.diagnostic(
WarningMessages.rawValueNotWrittenToPublishFile(
rawValue,
"PGP secret key",
"--secret-key"
)
)
None
case secretOption => Some(secretOption)
}

// This is safe to be publicly available
val publicKeyDirective = getDirectiveValue(setupKeys.publicKeyOpt).map {
"publish.publicKey" -> _
}.toSeq

val extraDirectives = passwordDirectives ++ publicKeyDirective

if (passwordDirectives.exists(_._2.startsWith("value:")))
logger.diagnostic(
"The secret value of PGP private key password will be written to a potentially public file!"
)

if (secretKeyDirValue.exists(_.startsWith("value:")))
logger.diagnostic(
"The secret value of PGP private key will be written to a potentially public file!"
)

OptionCheck.DefaultValue(
() => uploadKey(publicKeyOpt).map(_ => secretKeyDirValue),
extraDirectives,
Expand Down
113 changes: 82 additions & 31 deletions modules/integration/src/test/scala/scala/cli/integration/ConfigTests.scala
View file
Expand Up @@ -67,7 +67,7 @@ class ConfigTests extends ScalaCliSuite {
res.out.trim()
}
def readDecoded(env: Map[String, String] = Map.empty): String = {
val res = os.proc(TestUtil.cli, "--power", "config", key, "--password")
val res = os.proc(TestUtil.cli, "--power", "config", key, "--password-value")
.call(cwd = root, env = configEnv ++ env)
res.out.trim()
}
Expand Down Expand Up @@ -166,10 +166,12 @@ class ConfigTests extends ScalaCliSuite {
}

if (!TestUtil.isCI || !Properties.isWin)
test("Create a default PGP key") {
createDefaultPgpKeyTest()
}
def createDefaultPgpKeyTest(): Unit = {
for (pgpPasswordOption <- List("none", "random", "MY_CHOSEN_PASSWORD"))
test(s"Create a default PGP key, password: $pgpPasswordOption") {
createDefaultPgpKeyTest(pgpPasswordOption)
}

def createDefaultPgpKeyTest(pgpPasswordOption: String): Unit = {
TestInputs().fromRoot { root =>
val configFile = {
val dir = root / "config"
Expand All @@ -179,22 +181,57 @@ class ConfigTests extends ScalaCliSuite {
val extraEnv = Map(
"SCALA_CLI_CONFIG" -> configFile.toString
)
val checkRes = os.proc(TestUtil.cli, "--power", "config", "--create-pgp-key")
val checkPassword = os.proc(TestUtil.cli, "--power", "config", "--create-pgp-key")
.call(cwd = root, env = extraEnv, check = false, mergeErrIntoOut = true)
expect(checkRes.exitCode != 0)
expect(checkRes.out.text().contains("--email"))
os.proc(TestUtil.cli, "--power", "config", "--create-pgp-key", "--email", "alex@alex.me")
.call(cwd = root, env = extraEnv, stdin = os.Inherit, stdout = os.Inherit)
expect(checkPassword.exitCode != 0)
expect(checkPassword.out.text().contains("--pgp-password"))

val checkEmail = os.proc(
TestUtil.cli,
"--power",
"config",
"--create-pgp-key",
"--pgp-password",
pgpPasswordOption
)
.call(cwd = root, env = extraEnv, check = false, mergeErrIntoOut = true)
expect(checkEmail.exitCode != 0)
expect(checkEmail.out.text().contains("--email"))

val pgpCreated = os.proc(
TestUtil.cli,
"--power",
"config",
"--create-pgp-key",
"--email",
"alex@alex.me",
"--pgp-password",
pgpPasswordOption
)
.call(cwd = root, env = extraEnv, mergeErrIntoOut = true)

val pgpPasswordOpt: Option[String] = pgpCreated.out.text()
.linesIterator
.toSeq
.find(_.startsWith("Password"))
.map(_.stripPrefix("Password:").trim())

if (pgpPasswordOption != "random")
expect(pgpPasswordOpt.isEmpty)
else
expect(pgpPasswordOpt.isDefined)

val passwordInConfig = os.proc(TestUtil.cli, "--power", "config", "pgp.secret-key-password")
.call(cwd = root, env = extraEnv, stderr = os.Pipe)
expect(passwordInConfig.out.text().isEmpty())

val password = os.proc(TestUtil.cli, "--power", "config", "pgp.secret-key-password")
.call(cwd = root, env = extraEnv)
.out.trim()
val secretKey = os.proc(TestUtil.cli, "--power", "config", "pgp.secret-key")
.call(cwd = root, env = extraEnv)
.out.trim()
val rawPublicKey = os.proc(TestUtil.cli, "--power", "config", "pgp.public-key", "--password")
.call(cwd = root, env = extraEnv)
.call(cwd = root, env = extraEnv, stderr = os.Pipe)
.out.trim()
val rawPublicKey =
os.proc(TestUtil.cli, "--power", "config", "pgp.public-key", "--password-value")
.call(cwd = root, env = extraEnv, stderr = os.Pipe)
.out.trim()

val tmpFile = root / "test-file"
val tmpFileAsc = root / "test-file.asc"
Expand All @@ -204,23 +241,37 @@ class ConfigTests extends ScalaCliSuite {
def maybeEscape(arg: String): String =
if (Properties.isWin) q + arg + q
else arg
os.proc(
TestUtil.cli,
"--power",
"pgp",
"sign",
"--password",
maybeEscape(password),
"--secret-key",
maybeEscape(secretKey),
tmpFile
)
.call(cwd = root, stdin = os.Inherit, stdout = os.Inherit, env = extraEnv)
val signProcess = if (pgpPasswordOption != "none")
os.proc(
TestUtil.cli,
"--power",
"pgp",
"sign",
"--password",
s"value:${maybeEscape(pgpPasswordOpt.getOrElse("MY_CHOSEN_PASSWORD"))}",
"--secret-key",
maybeEscape(secretKey),
tmpFile
)
else
os.proc(
TestUtil.cli,
"--power",
"pgp",
"sign",
"--secret-key",
maybeEscape(secretKey),
tmpFile
)
signProcess.call(cwd = root, stdin = os.Inherit, stdout = os.Inherit, env = extraEnv)

val pubKeyFile = root / "key.pub"
os.write(pubKeyFile, rawPublicKey)
os.proc(TestUtil.cli, "--power", "pgp", "verify", "--key", pubKeyFile, tmpFileAsc)
.call(cwd = root, stdin = os.Inherit, stdout = os.Inherit, env = extraEnv)
val verifyResult =
os.proc(TestUtil.cli, "--power", "pgp", "verify", "--key", pubKeyFile, tmpFileAsc)
.call(cwd = root, env = extraEnv, mergeErrIntoOut = true)

expect(verifyResult.out.text().contains("valid signature"))
}
}

Expand Down

0 comments on commit bcc68de

Please sign in to comment.