From e295bfaef53d5f99a190219e3cb984bc89922f02 Mon Sep 17 00:00:00 2001 From: Jo Rabin Date: Mon, 25 Sep 2023 18:31:00 +0100 Subject: [PATCH 1/6] Suggested KeyFile processing --- .../linguafranca/pwdb/kdbx/KdbxKeyFile.java | 98 +++++++++++++++++-- 1 file changed, 89 insertions(+), 9 deletions(-) diff --git a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java index 9d2b52e5..23a03df0 100644 --- a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java +++ b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java @@ -16,16 +16,28 @@ package org.linguafranca.pwdb.kdbx; +import org.apache.commons.codec.DecoderException; import org.apache.commons.codec.binary.Base64; import org.apache.commons.codec.binary.Hex; +import org.linguafranca.pwdb.security.Encryption; import org.w3c.dom.Document; +import org.xml.sax.SAXException; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; +import java.io.FilterInputStream; +import java.io.IOException; import java.io.InputStream; +import java.io.PushbackInputStream; +import java.security.DigestInputStream; +import java.security.MessageDigest; +import java.util.Arrays; +import java.util.Objects; /** * Class has a static method to load a key from a KDBX XML Key File @@ -35,28 +47,96 @@ @SuppressWarnings("WeakerAccess") public class KdbxKeyFile { - private static XPath xpath = XPathFactory.newInstance().newXPath(); + private static final XPath xpath = XPathFactory.newInstance().newXPath(); + private static final int BUFFER_SIZE = 65; + private static final int KEY_LEN_32 = 32; + private static final int KEY_LEN_64 = 64; + + private static class HashMismatchException extends Exception {} /** - * Load a key from an InputStream with a KDBX XML key file. - * @param inputStream the input stream holding the key + * Load a key from an InputStream + *

+ * The InputStream can represent ... TODO write about the formats + * + * @param inputStream the input stream holding the key, caller should close * @return the key */ public static byte[] load(InputStream inputStream) { + DigestInputStream digestInputStream = new DigestInputStream(inputStream, Encryption.getSha256MessageDigestInstance()); + PushbackInputStream pis = new PushbackInputStream(digestInputStream, BUFFER_SIZE); + try { + byte[] buffer = new byte[65]; + int bytesRead = pis.read(buffer); + if (bytesRead == KEY_LEN_32) { + // if length 32 assume binary key file + return buffer; + } else if (bytesRead == KEY_LEN_64) { + // if length 64 assume hex encoded key file + return Hex.decodeHex(new String(Arrays.copyOf(buffer, bytesRead))); + } else { + // if length not 32 or 64 either an XML key file or just a file to get digest + pis.unread(buffer); + try { + // see if it's an XML key file + pis.unread(buffer); + return tryComputeXmlKeyFile(new FilterInputStream(pis){ + @Override + public void close() { + // suppress ability to close, so we can carry on reading on exception + } + }); + } catch (HashMismatchException e) { + throw new IllegalArgumentException("Invalid key in signature file"); + } catch (Exception e) { + byte [] sink = new byte[1024]; + // xml file was invalid so read the remainder of file + //noinspection StatementWithEmptyBody + while (digestInputStream.read(sink) > 0) { + // just reading file to get its digest + } + return digestInputStream.getMessageDigest().digest(); + } + } + } catch (Exception e) { + throw new IllegalArgumentException(e); + } + } + + + /** + * Read the InputStream (kdbx xml keyfile) and compute the hash (SHA-256) to build a key + * + * @param is The KeyFile as an InputStream + * @return the computed byte array (keyFile) to compute the MasterKey + */ + private static byte[] tryComputeXmlKeyFile(InputStream is) throws HashMismatchException { + try { DocumentBuilder documentBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - Document doc = documentBuilder.parse(inputStream); + Document doc = documentBuilder.parse(is); String version = (String) xpath.evaluate("//KeyFile/Meta/Version/text()", doc, XPathConstants.STRING); String data = (String) xpath.evaluate("//KeyFile/Key/Data/text()", doc, XPathConstants.STRING); if (data == null) { return null; } - if (version.equals("2.0")) { - return Hex.decodeHex(data.replaceAll("\\s","")); + if (Objects.isNull(version) || !version.equals("2.0")){ + return Base64.decodeBase64(data.getBytes()); } - return Base64.decodeBase64(data.getBytes()); - } catch (Exception e) { - throw new RuntimeException("Key File input stream cannot be null"); + + byte[] hexData = Hex.decodeHex(data.replaceAll("\\s", "")); + MessageDigest md = Encryption.getSha256MessageDigestInstance(); + byte[] computedHash = md.digest(hexData); + + String hashToCheck = (String) xpath.evaluate("//KeyFile/Key/Data/@Hash", doc, XPathConstants.STRING); + byte[] verifiedHash = Hex.decodeHex(hashToCheck); + + if (!Arrays.equals(Arrays.copyOf(computedHash, verifiedHash.length), verifiedHash)) { + throw new HashMismatchException(); + } + return hexData; + } catch(IOException | SAXException | ParserConfigurationException | XPathExpressionException | DecoderException e) { + throw new IllegalArgumentException("An error occurred during XML parsing: " + e.getMessage()); } } } From 291365a4028c0475398126339918bb78ec9775aa Mon Sep 17 00:00:00 2001 From: Jo Rabin Date: Mon, 25 Sep 2023 18:33:19 +0100 Subject: [PATCH 2/6] Suggested KeyFile processing (edit class header) --- kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java index 23a03df0..b16fab74 100644 --- a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java +++ b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java @@ -40,7 +40,7 @@ import java.util.Objects; /** - * Class has a static method to load a key from a KDBX XML Key File + * Class has a static method to load a key from an {@link InputStream} * * @author jo */ From 264d3dda6ee8dd1f1f0a35e615f7688720996657 Mon Sep 17 00:00:00 2001 From: Jo Rabin Date: Mon, 25 Sep 2023 20:24:39 +0100 Subject: [PATCH 3/6] correct base64 decode, avoid string creation for 64 bit file --- .../main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java index b16fab74..8d41fc50 100644 --- a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java +++ b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java @@ -34,6 +34,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.PushbackInputStream; +import java.nio.ByteBuffer; import java.security.DigestInputStream; import java.security.MessageDigest; import java.util.Arrays; @@ -72,11 +73,10 @@ public static byte[] load(InputStream inputStream) { // if length 32 assume binary key file return buffer; } else if (bytesRead == KEY_LEN_64) { - // if length 64 assume hex encoded key file - return Hex.decodeHex(new String(Arrays.copyOf(buffer, bytesRead))); + // if length 64 assume hex encoded key file (avoid creating a String) + return Hex.decodeHex(ByteBuffer.wrap(buffer).asCharBuffer().array()); } else { // if length not 32 or 64 either an XML key file or just a file to get digest - pis.unread(buffer); try { // see if it's an XML key file pis.unread(buffer); @@ -121,7 +121,7 @@ private static byte[] tryComputeXmlKeyFile(InputStream is) throws HashMismatchEx return null; } if (Objects.isNull(version) || !version.equals("2.0")){ - return Base64.decodeBase64(data.getBytes()); + return Base64.decodeBase64(data); } byte[] hexData = Hex.decodeHex(data.replaceAll("\\s", "")); From c80df9eb20440c358f16023f21ff422d4100d49a Mon Sep 17 00:00:00 2001 From: Jo Rabin Date: Mon, 25 Sep 2023 22:11:32 +0100 Subject: [PATCH 4/6] tidying --- .../linguafranca/pwdb/kdbx/KdbxKeyFile.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java index 8d41fc50..ec920fac 100644 --- a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java +++ b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java @@ -42,8 +42,6 @@ /** * Class has a static method to load a key from an {@link InputStream} - * - * @author jo */ @SuppressWarnings("WeakerAccess") public class KdbxKeyFile { @@ -53,21 +51,20 @@ public class KdbxKeyFile { private static final int KEY_LEN_32 = 32; private static final int KEY_LEN_64 = 64; - private static class HashMismatchException extends Exception {} - /** * Load a key from an InputStream *

- * The InputStream can represent ... TODO write about the formats + * The InputStream can represent ... TODO write about the formats * * @param inputStream the input stream holding the key, caller should close * @return the key */ public static byte[] load(InputStream inputStream) { - DigestInputStream digestInputStream = new DigestInputStream(inputStream, Encryption.getSha256MessageDigestInstance()); + DigestInputStream digestInputStream = new DigestInputStream(inputStream, + Encryption.getSha256MessageDigestInstance()); PushbackInputStream pis = new PushbackInputStream(digestInputStream, BUFFER_SIZE); try { - byte[] buffer = new byte[65]; + byte[] buffer = new byte[BUFFER_SIZE]; int bytesRead = pis.read(buffer); if (bytesRead == KEY_LEN_32) { // if length 32 assume binary key file @@ -80,30 +77,29 @@ public static byte[] load(InputStream inputStream) { try { // see if it's an XML key file pis.unread(buffer); - return tryComputeXmlKeyFile(new FilterInputStream(pis){ + return tryComputeXmlKeyFile(new FilterInputStream(pis) { + // suppress ability to close, so we can carry on reading on exception @Override - public void close() { - // suppress ability to close, so we can carry on reading on exception - } + public void close() { /* nothing */ } }); } catch (HashMismatchException e) { throw new IllegalArgumentException("Invalid key in signature file"); - } catch (Exception e) { - byte [] sink = new byte[1024]; - // xml file was invalid so read the remainder of file - //noinspection StatementWithEmptyBody - while (digestInputStream.read(sink) > 0) { - // just reading file to get its digest - } - return digestInputStream.getMessageDigest().digest(); + } catch (Exception ignored) { + // fall through } + // xml file was invalid so read the remainder of file + byte[] sink = new byte[1024]; + // read file to get its digest + //noinspection StatementWithEmptyBody + while (digestInputStream.read(sink) > 0) { /* nothing */ } + return digestInputStream.getMessageDigest().digest(); + } } catch (Exception e) { throw new IllegalArgumentException(e); } } - /** * Read the InputStream (kdbx xml keyfile) and compute the hash (SHA-256) to build a key * @@ -120,7 +116,7 @@ private static byte[] tryComputeXmlKeyFile(InputStream is) throws HashMismatchEx if (data == null) { return null; } - if (Objects.isNull(version) || !version.equals("2.0")){ + if (Objects.isNull(version) || !version.equals("2.0")) { return Base64.decodeBase64(data); } @@ -135,8 +131,12 @@ private static byte[] tryComputeXmlKeyFile(InputStream is) throws HashMismatchEx throw new HashMismatchException(); } return hexData; - } catch(IOException | SAXException | ParserConfigurationException | XPathExpressionException | DecoderException e) { + } catch (IOException | SAXException | ParserConfigurationException | XPathExpressionException | + DecoderException e) { throw new IllegalArgumentException("An error occurred during XML parsing: " + e.getMessage()); } } + + private static class HashMismatchException extends Exception { + } } From 2d930f472c0cc16e76673195b5730aa2c8f53240 Mon Sep 17 00:00:00 2001 From: Jo Rabin Date: Tue, 26 Sep 2023 09:40:13 +0100 Subject: [PATCH 5/6] more improvements --- .../linguafranca/pwdb/kdbx/KdbxKeyFile.java | 85 +++++++++++-------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java index ec920fac..5f543306 100644 --- a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java +++ b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java @@ -60,42 +60,48 @@ public class KdbxKeyFile { * @return the key */ public static byte[] load(InputStream inputStream) { + // wrap the stream to get its digest (in case we need it) DigestInputStream digestInputStream = new DigestInputStream(inputStream, Encryption.getSha256MessageDigestInstance()); + // wrap the stream, so we can test reading from it but then push back to get original stream PushbackInputStream pis = new PushbackInputStream(digestInputStream, BUFFER_SIZE); try { byte[] buffer = new byte[BUFFER_SIZE]; int bytesRead = pis.read(buffer); + + // if length 32 assume binary key file if (bytesRead == KEY_LEN_32) { - // if length 32 assume binary key file return buffer; - } else if (bytesRead == KEY_LEN_64) { - // if length 64 assume hex encoded key file (avoid creating a String) - return Hex.decodeHex(ByteBuffer.wrap(buffer).asCharBuffer().array()); - } else { - // if length not 32 or 64 either an XML key file or just a file to get digest + } + + // if length 64 may be hex encoded key file + if (bytesRead == KEY_LEN_64) { try { - // see if it's an XML key file - pis.unread(buffer); - return tryComputeXmlKeyFile(new FilterInputStream(pis) { - // suppress ability to close, so we can carry on reading on exception - @Override - public void close() { /* nothing */ } - }); - } catch (HashMismatchException e) { - throw new IllegalArgumentException("Invalid key in signature file"); - } catch (Exception ignored) { - // fall through + return Hex.decodeHex(ByteBuffer.wrap(buffer).asCharBuffer().array()); // (avoid creating a String) + } catch (DecoderException ignored) { + // fall through it may be an XML file or just a file whose digest we want } - // xml file was invalid so read the remainder of file - byte[] sink = new byte[1024]; - // read file to get its digest - //noinspection StatementWithEmptyBody - while (digestInputStream.read(sink) > 0) { /* nothing */ } - return digestInputStream.getMessageDigest().digest(); + } + // restore stream + pis.unread(buffer); + // if length not 32 or 64 either an XML key file or just a file to get digest + try { + // see if it's an XML key file + return tryComputeXmlKeyFile(pis); + } catch (HashMismatchException e) { + throw new IllegalArgumentException("Invalid key in signature file"); + } catch (Exception ignored) { + // fall through to get file digest } - } catch (Exception e) { + + // is not a valid xml file, so read the remainder of file + byte[] sink = new byte[1024]; + // read file to get its digest + //noinspection StatementWithEmptyBody + while (digestInputStream.read(sink) > 0) { /* nothing */ } + return digestInputStream.getMessageDigest().digest(); + } catch (IOException e) { throw new IllegalArgumentException(e); } } @@ -103,34 +109,43 @@ public void close() { /* nothing */ } /** * Read the InputStream (kdbx xml keyfile) and compute the hash (SHA-256) to build a key * - * @param is The KeyFile as an InputStream + * @param is The KeyFile as an InputStream, must return with stream open on error * @return the computed byte array (keyFile) to compute the MasterKey */ private static byte[] tryComputeXmlKeyFile(InputStream is) throws HashMismatchException { - + // DocumentBuilder closes input stream so wrap inputStream to inhibit this in case of failure + InputStream unCloseable = new FilterInputStream(is) { + @Override + public void close() { /* nothing */ } + }; try { DocumentBuilder documentBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); - Document doc = documentBuilder.parse(is); - String version = (String) xpath.evaluate("//KeyFile/Meta/Version/text()", doc, XPathConstants.STRING); + Document doc = documentBuilder.parse(unCloseable); + // get the key String data = (String) xpath.evaluate("//KeyFile/Key/Data/text()", doc, XPathConstants.STRING); if (data == null) { - return null; + throw new IllegalArgumentException("Key file does not contain a key"); } + // get the file version + String version = (String) xpath.evaluate("//KeyFile/Meta/Version/text()", doc, XPathConstants.STRING); + // if not 2.0 then key is base64 encoded if (Objects.isNull(version) || !version.equals("2.0")) { return Base64.decodeBase64(data); } - byte[] hexData = Hex.decodeHex(data.replaceAll("\\s", "")); - MessageDigest md = Encryption.getSha256MessageDigestInstance(); - byte[] computedHash = md.digest(hexData); + // key data may contain white space + byte[] decodedData = Hex.decodeHex(data.replaceAll("\\s", "")); + byte[] decodedDataHash = Encryption.getSha256MessageDigestInstance().digest(decodedData); + // hash used to verify the data String hashToCheck = (String) xpath.evaluate("//KeyFile/Key/Data/@Hash", doc, XPathConstants.STRING); - byte[] verifiedHash = Hex.decodeHex(hashToCheck); + byte[] decodedHashToCheck = Hex.decodeHex(hashToCheck); - if (!Arrays.equals(Arrays.copyOf(computedHash, verifiedHash.length), verifiedHash)) { + // hashToCheck is a truncated version of the actual hash + if (!Arrays.equals(Arrays.copyOf(decodedDataHash, decodedHashToCheck.length), decodedHashToCheck)) { throw new HashMismatchException(); } - return hexData; + return decodedData; } catch (IOException | SAXException | ParserConfigurationException | XPathExpressionException | DecoderException e) { throw new IllegalArgumentException("An error occurred during XML parsing: " + e.getMessage()); From bb7c5e2febd5938f44309e63ffd95aa046b79b64 Mon Sep 17 00:00:00 2001 From: Jo Rabin Date: Tue, 26 Sep 2023 09:55:58 +0100 Subject: [PATCH 6/6] nits --- kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java | 1 - 1 file changed, 1 deletion(-) diff --git a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java index 5f543306..010ecd7a 100644 --- a/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java +++ b/kdbx/src/main/java/org/linguafranca/pwdb/kdbx/KdbxKeyFile.java @@ -36,7 +36,6 @@ import java.io.PushbackInputStream; import java.nio.ByteBuffer; import java.security.DigestInputStream; -import java.security.MessageDigest; import java.util.Arrays; import java.util.Objects;