From 507fc90c3a98fae559646ef89277366fd5de01a4 Mon Sep 17 00:00:00 2001 From: Cynthia Date: Tue, 27 Jun 2017 11:18:33 -0400 Subject: [PATCH 1/5] fixed unsafe 'decode' issue --- sshkey_paramiko.r2py | 193 +++++++++++++++++++++---------------------- 1 file changed, 95 insertions(+), 98 deletions(-) diff --git a/sshkey_paramiko.r2py b/sshkey_paramiko.r2py index 8135176..acb7994 100644 --- a/sshkey_paramiko.r2py +++ b/sshkey_paramiko.r2py @@ -7,20 +7,20 @@ To be used by sshkey.r2py, this is not a stand alone module. sshkey.r2py - is the wrapper that developers should use. This file contains the code - that has been modified or taken from paramiko. It is licensed under a - different license then the rest of the code and to avoid any conflict it + is the wrapper that developers should use. This file contains the code + that has been modified or taken from paramiko. It is licensed under a + different license then the rest of the code and to avoid any conflict it has been separated into its own module. - - It makes heavy use of the paramiko code base, I have + + It makes heavy use of the paramiko code base, I have deconstructed their scheme in an attempt to port only the necessary code into repy. - - + + Modified by Anthony Honstain - - Paramiko + + Paramiko paramiko/util.py paramiko/message.py paramiko/rsakey.py were the source for the functions: _sshkey_paramiko_get_bytes @@ -33,14 +33,14 @@ _sshkey_paramiko_read_private_key Copyright (C) 2003-2007 Robey Pointer - + This file is part of paramiko. - + Paramiko is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version. - + Paramiko is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more @@ -60,7 +60,7 @@ class sshkey_paramiko_BERException (Exception): pass - + class sshkey_paramiko_SSHException(Exception): """This exception indicates that the ssh key was unable to be decoded""" pass @@ -77,14 +77,14 @@ class _sshkey_paramiko_BER(object): """ Perform BER decoding. - + None - + ber_obj _sshkey_paramiko_BER(data) - list = ber_obj.decode() - + list = ber_obj.decode_next() + """ """ @@ -95,9 +95,9 @@ class _sshkey_paramiko_BER(object): self.content = content self.idx = 0 - def decode(self): - return self.decode_next() - + # def decode(self): + # return self.decode_next() + def decode_next(self): if self.idx >= len(self.content): return None @@ -158,18 +158,18 @@ class _sshkey_paramiko_BER(object): def _sshkey_paramiko_get_bytes(packet, n): """ - ValueError if I/O operation on closed _sshkey_StringIO object. This is raised - by the _sshkey_StringIO object, but it is never closed in this code so + ValueError if I/O operation on closed _sshkey_StringIO object. This is raised + by the _sshkey_StringIO object, but it is never closed in this code so this is unlikely. - + MODIFIED FROM paramiko/message.py - + Return the next C{n} bytes of the Message, without decomposing into an int, string, etc. Just the raw bytes are returned. - + @return: a string of the next C{n} bytes of the Message, or a string of C{n} zero bytes, if there aren't C{n} bytes remaining. - + @rtype: string """ b = packet.read(n) @@ -183,10 +183,10 @@ def _sshkey_paramiko_inflate_long(s, always_positive=False): """ TAKEN FROM paramiko/util.py - turns a normalized byte string into a + turns a normalized byte string into a long-int (adapted from Crypto.Util.number) """ - + out = 0L negative = 0 if not always_positive and (len(s) > 0) and (ord(s[0]) >= 0x80): @@ -199,12 +199,12 @@ def _sshkey_paramiko_inflate_long(s, always_positive=False): for i in range(0, len(s), 4): # Anthony - replaces struct functionality #out = (out << 32) + struct.unpack('>I', s[i:i+4])[0] - out = (out << 32) + out = (out << 32) out += (ord(s[i]) << 24) out += ord(s[i+1]) << 16 out += ord(s[i+2]) << 8 out += ord(s[i+3]) - + if negative: out -= (1L << (8 * len(s))) return out @@ -216,49 +216,49 @@ def _sshkey_paramiko_get_string(packet): Takes a StringIO object and reads off 4 bytes and converts them to an integer, that integer is used to decide how many bytes are to be read off of the packet. - - Example: If the byte string in packet (StringIO object) were + + Example: If the byte string in packet (StringIO object) were '\x00\x00\x00\x01A' Stage1 Then the first 4 bytes will be '\x00\x00\x00\x01' which will be converted into the integer num_bytes=1. Stage2 - Then num_bytes=1 will be read from packet and returned. - - + Then num_bytes=1 will be read from packet and returned. + + Heavily modified to remove object used in paramiko/message.py - + A StringIO object that is not closed. - + - ValueError if I/O operation on closed _sshkey_StringIO object. This is raised - by the _sshkey_StringIO object, but it is never closed in this code so + ValueError if I/O operation on closed _sshkey_StringIO object. This is raised + by the _sshkey_StringIO object, but it is never closed in this code so this is unlikely. None - - + + The string encoded in the packet. - + """ - + # Get 4 bytes from packet which will tell us how many bytes to # extract for stage 2. stage1 = _sshkey_paramiko_get_bytes(packet, 4) - + # Replacing struct module - #num_bytes = struct.unpack('>I', stage1)[0] + #num_bytes = struct.unpack('>I', stage1)[0] num_bytes = 0 for byte in stage1: num_bytes = num_bytes << 8 - num_bytes += ord(byte) - - # This is the desired string from the packet. + num_bytes += ord(byte) + + # This is the desired string from the packet. stage2 = _sshkey_paramiko_get_bytes(packet, num_bytes) return stage2 - + # Anthony modified to force use of our md5 instead pycrypto md5 @@ -268,7 +268,7 @@ def _sshkey_paramiko_generate_key_bytes(salt, key, nbytes): Used to generate the DES3 key so that the private key can be decrypted. - + Given a password, passphrase, or other human-source key, scramble it through a secure hash into some keyworthy bytes. This specific algorithm is used for encrypting/decrypting private key files. @@ -315,7 +315,7 @@ def _sshkey_paramiko_read_public_key(openfile): data = data.split() # Example value for data at this point # data = ['ssh-rsa', 'AAAAB3NzaC1yc2...' , 'root@' ] - + # We discard data[0], the string 'ssh-rsa' is encoded in data[1] in base64 data = data[1] @@ -324,64 +324,64 @@ def _sshkey_paramiko_read_public_key(openfile): packet = _sshkey_StringIO(base64_b64decode(data)) keyname = _sshkey_paramiko_get_string(packet) - + if keyname != 'ssh-rsa': - raise sshkey_paramiko_SSHException("Invalid SSH-rsa key") + raise sshkey_paramiko_SSHException("Invalid SSH-rsa key") # The exponent is encoded first e_exp = _sshkey_paramiko_inflate_long(_sshkey_paramiko_get_string(packet)) n_modulus = _sshkey_paramiko_inflate_long(_sshkey_paramiko_get_string(packet)) packet.close() - + return e_exp, n_modulus - - + + def _sshkey_paramiko_read_private_key(tag, openfile, password=None): """ - + sshkey_paramiko_SSHException: - not a valid RSA private key file + not a valid RSA private key file A header with "-----BEGIN DSA PRIVATE KEY-----" would cause this. - + base64 decoding error - + Unknown private key structure - + Can't parse DEK-info in private key file If a header was found but did not contain 'DEK-Info: DES-EDE3-CBC,57987BCBC21F738A' - + Unable to parse key file - + Not a valid RSA private key file (bad ber encoding) - + sshkey_paramiko_EncryptionException: Unknown private key cipher If a unsported cipher was used to encrypt the ssh key, only DES3 is supported. A key encrypted with AES would raise this exception. - + Private key file is encrypted, password needed. No password was provided to decrypt the key. - + """ keydata = _sshkey_paramiko_decode_private_key(tag, openfile, password) - - # private key file contains: - # keylist = { version = 0, n, e, d, p, q, d mod p-1, d mod q-1, q**-1 mod p } + + # private key file contains: + # keylist = { version = 0, n, e, d, p, q, d mod p-1, d mod q-1, q**-1 mod p } try: - keylist = _sshkey_paramiko_BER(keydata).decode() + keylist = _sshkey_paramiko_BER(keydata).decode_next() except sshkey_paramiko_BERException: raise sshkey_paramiko_SSHException('Unable to parse key file') - + if (type(keylist) is not list) or (len(keylist) < 4) or (keylist[0] != 0): raise sshkey_paramiko_SSHException('Not a valid RSA private key file (bad ber encoding)') - + return keylist - + def _sshkey_paramiko_decode_private_key(tag, file, password=None): @@ -389,44 +389,44 @@ def _sshkey_paramiko_decode_private_key(tag, file, password=None): Take a file process the headers and decrypt if needed, it will then return the encoded data. - - + + file.readlines() returns a list like the following - ['-----BEGIN RSA PRIVATE KEY-----\n', - 'MIIEowIBAAKCAQEAq6Sbj5wJWmDbyQnyACihkpwttRG57u9MGiB59jT/Nl96Q0Lc\n', - 'kMACD45GB+JUSzMvBpT0R9Dp+e83Jk12sV756wD9Qn5x4uKvVp4aFea2k6EPf/2x\n', - 'c/QHtzBR6YugFrzeuHaeQZLtvXzKZsaQJMYQwR8Njn+kP/oM6gvIBaWV6FUScAmF\n', + ['-----BEGIN RSA PRIVATE KEY-----\n', + 'MIIEowIBAAKCAQEAq6Sbj5wJWmDbyQnyACihkpwttRG57u9MGiB59jT/Nl96Q0Lc\n', + 'kMACD45GB+JUSzMvBpT0R9Dp+e83Jk12sV756wD9Qn5x4uKvVp4aFea2k6EPf/2x\n', + 'c/QHtzBR6YugFrzeuHaeQZLtvXzKZsaQJMYQwR8Njn+kP/oM6gvIBaWV6FUScAmF\n', . . - 'oqBUsB6Bfp+NZGCxwICn+OV9N8z2bFWENYwx0Ubr7UlnETe05IqO\n', + 'oqBUsB6Bfp+NZGCxwICn+OV9N8z2bFWENYwx0Ubr7UlnETe05IqO\n', '-----END RSA PRIVATE KEY-----\n'] currentline will be set to the line number where the key data begins, marked by the line '-----BEGIN RSA PRIVATE KEY-----\n' - + Header information will be removed, I do know the details because my key files do not contain this information. - + end will be set to currentline and incremented untill the string '-----END RSA PRIVATE KEY-----\n' is found. - + In the case of the sample data, it was not encrypted and therefore base64.decodestring(''.join(lines[currentline:end])) will be returned. - + """ _CIPHER_TABLE = { 'DES-EDE3-CBC': { 'cipher': pydes_triple_des, 'keysize': 24, 'blocksize': 8, 'mode': pydes_CBC } } - + lines = file.readlines() - + currentline = 0 # File must contain the string '-----BEGIN RSA PRIVATE KEY-----\n' targetstring = '-----BEGIN ' + tag + ' PRIVATE KEY-----' while (currentline < len(lines)) and (lines[currentline].strip() != targetstring): currentline += 1 - if currentline >= len(lines): + if currentline >= len(lines): # String not found, this must an invalid key file. raise sshkey_paramiko_SSHException('not a valid ' + tag + ' private key file') @@ -446,7 +446,7 @@ def _sshkey_paramiko_decode_private_key(tag, file, password=None): # Modified to remove new line lines[end] = lines[end].strip('\n') end += 1 - + # if we trudged to the end of the file, just try to cope. try: data = base64_b64decode(''.join(lines[currentline:end])) @@ -457,8 +457,8 @@ def _sshkey_paramiko_decode_private_key(tag, file, password=None): if 'proc-type' not in headers: # unencryped: done return data - - + + # Key was encrypted so it will need to go through the decryption # process. # encrypted keyfile: will need a password @@ -473,25 +473,22 @@ def _sshkey_paramiko_decode_private_key(tag, file, password=None): # if no password was passed in, raise an exception pointing out that we need one if password is None: raise sshkey_paramiko_EncryptionException('Private key file is encrypted, password needed.') - + cipher = _CIPHER_TABLE[encryption_type]['cipher'] keysize = _CIPHER_TABLE[encryption_type]['keysize'] mode = _CIPHER_TABLE[encryption_type]['mode'] - + # Anthony - Replaced binascii #salt = binascii.unhexlify(saltstr) salt = binascii_a2b_hex(saltstr) - + # Anthony modified to use hashlib instead of pycrypto hash key = _sshkey_paramiko_generate_key_bytes(salt, password, keysize) #key = generate_key_bytes(MD5, salt, password, keysize) - + # Anthony - modified to use pydes instead of pycrypto DES3 temp_des_obj = pydes_triple_des(key, mode, salt) decrypted = temp_des_obj.decrypt(data) #decrypted = cipher.new(key, mode, salt).decrypt(data) - - return decrypted - - + return decrypted From 2e6ddd04f3c9de00ae7badc1045f44376058e6b5 Mon Sep 17 00:00:00 2001 From: Cynthia Date: Tue, 27 Jun 2017 11:35:00 -0400 Subject: [PATCH 2/5] replaced unsafe 'decode' call with 'decode_next' --- sshkey_paramiko.r2py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sshkey_paramiko.r2py b/sshkey_paramiko.r2py index 8135176..6ae2d49 100644 --- a/sshkey_paramiko.r2py +++ b/sshkey_paramiko.r2py @@ -83,7 +83,7 @@ class _sshkey_paramiko_BER(object): ber_obj _sshkey_paramiko_BER(data) - list = ber_obj.decode() + list = ber_obj.decode_next() """ @@ -95,8 +95,8 @@ class _sshkey_paramiko_BER(object): self.content = content self.idx = 0 - def decode(self): - return self.decode_next() + # def decode(self): + # return self.decode_next() def decode_next(self): if self.idx >= len(self.content): @@ -373,7 +373,7 @@ def _sshkey_paramiko_read_private_key(tag, openfile, password=None): # private key file contains: # keylist = { version = 0, n, e, d, p, q, d mod p-1, d mod q-1, q**-1 mod p } try: - keylist = _sshkey_paramiko_BER(keydata).decode() + keylist = _sshkey_paramiko_BER(keydata).decode_next() except sshkey_paramiko_BERException: raise sshkey_paramiko_SSHException('Unable to parse key file') From a4c603969a0f34fc2129c65b84ad5b73a90edd1b Mon Sep 17 00:00:00 2001 From: Cynthia Date: Tue, 27 Jun 2017 11:52:04 -0400 Subject: [PATCH 3/5] remove decode function --- sshkey_paramiko.r2py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sshkey_paramiko.r2py b/sshkey_paramiko.r2py index 6ae2d49..5c65866 100644 --- a/sshkey_paramiko.r2py +++ b/sshkey_paramiko.r2py @@ -94,9 +94,6 @@ class _sshkey_paramiko_BER(object): def __init__(self, content=''): self.content = content self.idx = 0 - - # def decode(self): - # return self.decode_next() def decode_next(self): if self.idx >= len(self.content): From 5ba4d22d34dc2f87ed35a542cabad973c30a9dc6 Mon Sep 17 00:00:00 2001 From: Cynthia Date: Tue, 27 Jun 2017 12:45:50 -0400 Subject: [PATCH 4/5] added comment for removing decode --- sshkey_paramiko.r2py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sshkey_paramiko.r2py b/sshkey_paramiko.r2py index 5c65866..f4a06c5 100644 --- a/sshkey_paramiko.r2py +++ b/sshkey_paramiko.r2py @@ -95,6 +95,8 @@ class _sshkey_paramiko_BER(object): self.content = content self.idx = 0 + # June 27, 2017: removed decode function, as it is not allowed by safe.py + def decode_next(self): if self.idx >= len(self.content): return None From 3387cb5527044394d6b3d087361c50ef90bd7f91 Mon Sep 17 00:00:00 2001 From: Cynthia Date: Thu, 29 Jun 2017 09:11:59 -0400 Subject: [PATCH 5/5] remove date info --- sshkey_paramiko.r2py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sshkey_paramiko.r2py b/sshkey_paramiko.r2py index f4a06c5..043e0d7 100644 --- a/sshkey_paramiko.r2py +++ b/sshkey_paramiko.r2py @@ -95,7 +95,7 @@ class _sshkey_paramiko_BER(object): self.content = content self.idx = 0 - # June 27, 2017: removed decode function, as it is not allowed by safe.py + # Removed decode function, as it is not allowed by safe.py def decode_next(self): if self.idx >= len(self.content):