Improve SSH private key validation (#713)

* Improve SSH private key validation

* Address review comment
This commit is contained in:
Fabian Henneke 2020-04-16 15:04:55 +02:00 committed by GitHub
parent f269bc7d28
commit 8b4751f825
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 57 deletions

View file

@ -522,49 +522,32 @@ class UserPreference : AppCompatActivity() {
@Throws(IllegalArgumentException::class, IOException::class) @Throws(IllegalArgumentException::class, IOException::class)
private fun copySshKey(uri: Uri) { private fun copySshKey(uri: Uri) {
// See metadata from document to validate SSH key // First check whether the content at uri is likely an SSH private key.
contentResolver.query(uri, null, null, null, null, null)?.use { cursor -> val fileSize = contentResolver.query(uri, arrayOf(OpenableColumns.SIZE), null, null, null)
val sizeIndex = cursor.getColumnIndex(OpenableColumns.SIZE) ?.use { cursor ->
// cursor returns only 1 row // Cursor returns only a single row.
cursor.moveToFirst() cursor.moveToFirst()
// see file's metadata cursor.getInt(0)
val fileSize = cursor.getInt(sizeIndex) } ?: throw IOException(getString(R.string.ssh_key_does_not_exist))
// We assume that an SSH key's ideal size is > 0 bytes && < 100 kilobytes.
if (fileSize > 100000 || fileSize == 0) { // We assume that an SSH key's ideal size is > 0 bytes && < 100 kilobytes.
throw IllegalArgumentException("Wrong file type selected") if (fileSize > 100_000 || fileSize == 0)
} else { throw IllegalArgumentException(getString(R.string.ssh_key_import_error_not_an_ssh_key_message))
// Validate BEGIN and END markers
val lines = contentResolver.openInputStream(uri)?.bufferedReader()?.readLines()
// The file must have more than 2 lines, and the first and last line must have
// OpenSSH key markers.
if (lines != null &&
lines.size > 2 &&
!lines[0].contains("BEGIN OPENSSH PRIVATE KEY") &&
!lines[lines.size - 1].contains("END OPENSSH PRIVATE KEY")) {
throw IllegalArgumentException("Wrong file type selected")
}
}
}
val sshKeyInputStream = contentResolver.openInputStream(uri) val sshKeyInputStream = contentResolver.openInputStream(uri)
if (sshKeyInputStream != null) { ?: throw IOException(getString(R.string.ssh_key_does_not_exist))
val lines = sshKeyInputStream.bufferedReader().readLines()
val internalKeyFile = File("""$filesDir/.ssh_key""") // The file must have more than 2 lines, and the first and last line must have private key
// markers.
if (lines.size < 2 ||
!Regex("BEGIN .* PRIVATE KEY").containsMatchIn(lines.first()) ||
!Regex("END .* PRIVATE KEY").containsMatchIn(lines.last())
)
throw IllegalArgumentException(getString(R.string.ssh_key_import_error_not_an_ssh_key_message))
if (internalKeyFile.exists()) { // Canonicalize line endings to '\n'.
internalKeyFile.delete() File("$filesDir/.ssh_key").writeText(lines.joinToString("\n"))
internalKeyFile.createNewFile()
}
val sshKeyOutputSteam = internalKeyFile.outputStream()
sshKeyInputStream.copyTo(sshKeyOutputSteam, 1024)
sshKeyInputStream.close()
sshKeyOutputSteam.close()
} else {
Toast.makeText(this, getString(R.string.ssh_key_does_not_exist), Toast.LENGTH_LONG).show()
}
} }
private val isAccessibilityServiceEnabled: Boolean private val isAccessibilityServiceEnabled: Boolean
@ -622,23 +605,11 @@ class UserPreference : AppCompatActivity() {
finish() finish()
} catch (e: Exception) { } catch (e: Exception) {
when (e) { MaterialAlertDialogBuilder(this)
is IOException, .setTitle(resources.getString(R.string.ssh_key_error_dialog_title))
is IllegalArgumentException -> { .setMessage(e.message)
MaterialAlertDialogBuilder(this) .setPositiveButton(resources.getString(R.string.dialog_ok), null)
.setTitle(resources.getString(R.string.ssh_key_error_dialog_title)) .show()
.setMessage(getString(R.string.ssh_key_import_error_not_an_ssh_key_message))
.setPositiveButton(resources.getString(R.string.dialog_ok), null)
.show()
}
else -> {
MaterialAlertDialogBuilder(this)
.setTitle(resources.getString(R.string.ssh_key_error_dialog_title))
.setMessage(resources.getString(R.string.ssh_key_error_dialog_text) + e.message)
.setPositiveButton(resources.getString(R.string.dialog_ok), null)
.show()
}
}
} }
} }
EDIT_GIT_INFO -> { EDIT_GIT_INFO -> {

View file

@ -148,6 +148,7 @@
<string name="pref_show_time_summary">Set the time (in seconds) you want the password to be in clipboard. 0 means forever. Current value: %1$s</string> <string name="pref_show_time_summary">Set the time (in seconds) you want the password to be in clipboard. 0 means forever. Current value: %1$s</string>
<string name="pref_copy_title">Automatically copy password</string> <string name="pref_copy_title">Automatically copy password</string>
<string name="pref_copy_dialog_title">Automatically copy the password to the clipboard after decryption was successful.</string> <string name="pref_copy_dialog_title">Automatically copy the password to the clipboard after decryption was successful.</string>
<string name="ssh_key_import_error_not_an_ssh_key_message">Selected file does not appear to be an SSH private key.</string>
<string name="ssh_key_success_dialog_title">SSH-key imported</string> <string name="ssh_key_success_dialog_title">SSH-key imported</string>
<string name="ssh_key_error_dialog_title">Key import error</string> <string name="ssh_key_error_dialog_title">Key import error</string>
<string name="ssh_key_error_dialog_text">Message : \n</string> <string name="ssh_key_error_dialog_text">Message : \n</string>
@ -348,5 +349,4 @@
<string name="theme_dark">Dark</string> <string name="theme_dark">Dark</string>
<string name="theme_battery_saver">Set by Battery Saver</string> <string name="theme_battery_saver">Set by Battery Saver</string>
<string name="theme_follow_system">System default</string> <string name="theme_follow_system">System default</string>
<string name="ssh_key_import_error_not_an_ssh_key_message">Selected file does not appear to be an SSH key</string>
</resources> </resources>