Errors when parsing the preamble lines before the private key

[hoylen]

There's a bug with the handling of the preamble in PEM formatted private keys. That is, any lines that appear before the -----BEGIN <tag> PRIVATE KEY----- line.

The bug causes _read_private_key_pem to throw a SSHException with an error message that says, "base64 decoding error: Incorrect padding", which then causes an exception giving this misleading message, "encountered RSA key, expected OPENSSH key", as in #1612.

The code in version 2.7.0 and 2.7.1 attempts to treat the preamble as a set of headers.

Headers incorrectly parsed

Firstly, the first line is always discarded.

For example, consider this example:

First: this value is lost
Second: this is the only value detected
-----BEGIN RSA PRIVATE KEY-----

paramiko/paramiko/pkey.py

Lines 350 to 363 in ae3d0fe

	def _read_private_key_pem(self, lines, end, password):
	start = 0
	# parse any headers first
	headers = {}
	start += 1
	while start < len(lines):
	line = lines[start].split(": ")
	if len(line) == 1:
	break
	headers[line[0].lower()] = line[1].strip()
	start += 1
	# if we trudged to the end of the file, just try to cope.
	try:
	data = decodebytes(b("".join(lines[start:end])))

A simple fix is to move the first "start += 1" from before the while loop (line 354) to after the while loop (line 361). But that won't solve the second problem...

Non-header lines causes an error

No colon on this line causes an error
-----BEGIN RSA PRIVATE KEY-----

Secondly, any lines that are not formatted as key-colon-value will still cause an error. For example, the "bag attributes" in #1601 and the blank lines in #1580.

PEM is a loosely specified format with many variants, but RFC 7468 Textual Encodings of PKIX, PKCS, and CMS Structures says "Data before the encapsulation boundaries are permitted, and parsers MUST NOT malfunction when processing such data". If that is followed, while the code may try to parse the preamble as a set of key-value headers, it must not reject keys with preamble it does not understand.

Pull request #1615 proposes to only pass in the lines between and including the encapsulation boundaries to _read_private_key_pem. That will stop the errors, but it will also discard any headers in the preamble. The header information is lost, since any header lines won't be passed to _read_private_key_pem.

I don't know what headers paramiko expects or how they are used, so can't comment on whether the header parsing code is important or not. But it feels like all the preamble lines should be kept for applications to examine, if they want to; but not parsed as headers, since they could be in some proprietary format.

[Update] Actually, the current code doesn't even parse headers correctly: it only works if there are no header lines. So it is safe to simply ignore the entire preamble (including ignoring any header lines), since no functionality will be lost by doing that. The header functionality was never used.

[LiamClarkeNZ]

I've just hit this in an integration test. There's a test key defined as so:

PRIVATE_KEY_STRING = """
-----BEGIN RSA PRIVATE KEY-----
...base 64 stuff...
-----END RSA PRIVATE KEY-----
"""

Please note that this whitespace preserving string literal starts with a new line, for formatting/readability whatever.

Upgrading to paramiko 2.7.* from 2.5 causes tests to fail with the following error:

Traceback (most recent call last):
  File "/opt/\bla.py", line 80, in to_rsa_key
    return RSAKey.from_private_key(StringIO(private_key))
  File "/usr/local/lib/python3.6/dist-packages/paramiko/pkey.py", line 256, in from_private_key
    key = cls(file_obj=file_obj, password=password)
  File "/usr/local/lib/python3.6/dist-packages/paramiko/rsakey.py", line 52, in __init__
    self._from_private_key(file_obj, password)
  File "/usr/local/lib/python3.6/dist-packages/paramiko/rsakey.py", line 179, in _from_private_key
    data = self._read_private_key("RSA", file_obj, password)
  File "/usr/local/lib/python3.6/dist-packages/paramiko/pkey.py", line 334, in _read_private_key
    data = self._read_private_key_pem(lines, end, password)
  File "/usr/local/lib/python3.6/dist-packages/paramiko/pkey.py", line 365, in _read_private_key_pem
    raise SSHException("base64 decoding error: {}".format(e))
paramiko.ssh_exception.SSHException: base64 decoding error: Incorrect padding

The fix is... ...to trim the newline from the start of the triple quoted string that preserves whitespace.

PRIVATE_KEY_STRING = """
-----BEGIN RSA PRIVATE KEY-----
...base 64 stuff...
-----END RSA PRIVATE KEY-----
""".lstrip()  # Yep, this fixes it.

Maybe this is compliant with the spec, but at the very least, the error message could be more precise about the failure. I will try to come back with a PR if I have time, but just wanted to +1 this bug, and add my observations.

[bskinn]

Flagging this to include in the key/auth rework of #387