Skip to content

Commit 9f517fe

Browse files
committed
make unprotect errors noisier, fix problem saving protected tree, add test
fix protected->Protected attribute add PyKeePass.reload() dont modify protected value while parsing XML
1 parent 98b323e commit 9f517fe

File tree

4 files changed

+32
-14
lines changed

4 files changed

+32
-14
lines changed

pykeepass/entry.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def __init__(self, title=None, username=None, password=None, url=None,
4949
self._element.append(E.String(E.Key('Title'), E.Value(title)))
5050
self._element.append(E.String(E.Key('UserName'), E.Value(username)))
5151
self._element.append(
52-
E.String(E.Key('Password'), E.Value(password, protected="False"))
52+
E.String(E.Key('Password'), E.Value(password, Protected="True"))
5353
)
5454
if url:
5555
self._element.append(E.String(E.Key('URL'), E.Value(url)))

pykeepass/kdbx_parsing/common.py

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from lxml import etree
99
from copy import deepcopy
1010
import base64
11+
from binascii import Error as BinasciiError
1112
import unicodedata
1213
import zlib
1314
import re
@@ -191,7 +192,6 @@ class UnprotectedStream(Adapter):
191192
provided by get_cipher"""
192193

193194
protected_xpath = '//Value[@Protected=\'True\']'
194-
unprotected_xpath = '//Value[@Protected=\'False\']'
195195

196196
def __init__(self, protected_stream_key, subcon):
197197
super(UnprotectedStream, self).__init__(subcon)
@@ -201,29 +201,33 @@ def _decode(self, tree, con, path):
201201
cipher = self.get_cipher(self.protected_stream_key(con))
202202
for elem in tree.xpath(self.protected_xpath):
203203
if elem.text is not None:
204-
result = cipher.decrypt(base64.b64decode(elem.text)).decode('utf-8')
205-
# strip invalid XML characters - https://stackoverflow.com/questions/8733233
206-
result = re.sub(
207-
u'[^\u0020-\uD7FF\u0009\u000A\u000D\uE000-\uFFFD\U00010000-\U0010FFFF]+',
208-
'',
209-
result
210-
)
211-
elem.text = result
212-
elem.attrib['Protected'] = 'False'
204+
try:
205+
result = cipher.decrypt(base64.b64decode(elem.text)).decode('utf-8')
206+
# strip invalid XML characters - https://stackoverflow.com/questions/8733233
207+
result = re.sub(
208+
u'[^\u0020-\uD7FF\u0009\u000A\u000D\uE000-\uFFFD\U00010000-\U0010FFFF]+',
209+
'',
210+
result
211+
)
212+
elem.text = result
213+
except (UnicodeDecodeError, BinasciiError, ValueError):
214+
# FIXME: this should be a warning eventually, need to fix all databases in tests/ first
215+
log.error(
216+
"Element at {} marked as protected, but could not unprotect".format(tree.getpath(elem))
217+
)
213218
return tree
214219

215220
def _encode(self, tree, con, path):
216221
tree_copy = deepcopy(tree)
217222
cipher = self.get_cipher(self.protected_stream_key(con))
218-
for elem in tree_copy.xpath(self.unprotected_xpath):
223+
for elem in tree_copy.xpath(self.protected_xpath):
219224
if elem.text is not None:
220225
elem.text = base64.b64encode(
221226
cipher.encrypt(
222227
elem.text.encode('utf-8')
223228
)
224229
)
225-
elem.attrib['Protected'] = 'True'
226-
return tree
230+
return tree_copy
227231

228232

229233
class ARCFourVariantStream(UnprotectedStream):

pykeepass/pykeepass.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ def read(self, filename=None, password=None, keyfile=None,
123123
else:
124124
raise
125125

126+
def reload(self):
127+
"""Reload current database using previous credentials """
128+
129+
self.read(self.filename, self.password, self.keyfile)
130+
126131
def save(self, filename=None, transformed_key=None):
127132
"""Save current database object to disk.
128133

tests/tests.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,15 @@ def test_issue193(self):
842842
self.assertTrue(g.name is None)
843843
self.assertTrue(g in self.kp.groups)
844844

845+
def test_issue194(self):
846+
# entries with Protected=True aren't being protected properly
847+
848+
self.kp_tmp.add_entry(self.kp_tmp.root_group, 'protect_test', 'user', 'pass')
849+
self.kp_tmp.save()
850+
self.kp_tmp.reload()
851+
e = self.kp_tmp.find_entries(title='protect_test', first=True)
852+
self.assertEqual(e.password, 'pass')
853+
845854

846855

847856
class EntryFindTests4(KDBX4Tests, EntryFindTests3):

0 commit comments

Comments
 (0)