Skip to content

Conversation

@line-o
Copy link
Member

@line-o line-o commented Jun 11, 2021

crypto lib compatible with the coming version of eXist-db (5.3.0)

This version is built on top of the community project base but includes the latest changes and fixes made by @claudius108

Thanks to @chakl for tying up the loose ends.

This PR supersedes #36 (resolving conflicts, fixing file permissions and git history)

claudius108 and others added 5 commits June 3, 2021 09:38
@joewiz
Copy link
Member

joewiz commented Jun 11, 2021

@line-o Thank you! Unfortunately, I'm having trouble building this:

joe@choskimac-iii crypto-exist-java-lib % mvn clean package
[INFO] Scanning for projects...
[INFO] 
[INFO] -----< org.exist-db.xquery.extensions.expath:expath-crypto-module >-----
[INFO] Building eXist-db EXPath Cryptographic library 6.0.0-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
Downloading from maven-default-http-blocker: http://0.0.0.0/org/exist-db/exist-core/5.3.0-SNAPSHOT/maven-metadata.xml
[WARNING] Could not transfer metadata org.exist-db:exist-core:5.3.0-SNAPSHOT/maven-metadata.xml from/to maven-default-http-blocker (http://0.0.0.0/): transfer failed for http://0.0.0.0/org/exist-db/exist-core/5.3.0-SNAPSHOT/maven-metadata.xml
[WARNING] org.exist-db:exist-core:5.3.0-SNAPSHOT/maven-metadata.xmlfailed to transfer from http://0.0.0.0/ during a previous attempt. This failure was cached in the local repository and resolution will not be reattempted until the update interval of maven-default-http-blocker has elapsed or updates are forced. Original error: Could not transfer metadata org.exist-db:exist-core:5.3.0-SNAPSHOT/maven-metadata.xml from/to maven-default-http-blocker (http://0.0.0.0/): transfer failed for http://0.0.0.0/org/exist-db/exist-core/5.3.0-SNAPSHOT/maven-metadata.xml
Downloading from maven-default-http-blocker: http://0.0.0.0/org/exist-db/exist-parent/5.3.0-SNAPSHOT/maven-metadata.xml
[WARNING] Could not transfer metadata org.exist-db:exist-parent:5.3.0-SNAPSHOT/maven-metadata.xml from/to maven-default-http-blocker (http://0.0.0.0/): transfer failed for http://0.0.0.0/org/exist-db/exist-parent/5.3.0-SNAPSHOT/maven-metadata.xml
Downloading from maven-default-http-blocker: http://0.0.0.0/org/exist-db/exist-start/5.3.0-SNAPSHOT/maven-metadata.xml
[WARNING] Could not transfer metadata org.exist-db:exist-start:5.3.0-SNAPSHOT/maven-metadata.xml from/to maven-default-http-blocker (http://0.0.0.0/): transfer failed for http://0.0.0.0/org/exist-db/exist-start/5.3.0-SNAPSHOT/maven-metadata.xml
[WARNING] org.exist-db:exist-start:5.3.0-SNAPSHOT/maven-metadata.xmlfailed to transfer from http://0.0.0.0/ during a previous attempt. This failure was cached in the local repository and resolution will not be reattempted until the update interval of maven-default-http-blocker has elapsed or updates are forced. Original error: Could not transfer metadata org.exist-db:exist-start:5.3.0-SNAPSHOT/maven-metadata.xml from/to maven-default-http-blocker (http://0.0.0.0/): transfer failed for http://0.0.0.0/org/exist-db/exist-start/5.3.0-SNAPSHOT/maven-metadata.xml
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.827 s
[INFO] Finished at: 2021-06-11T12:16:39-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project expath-crypto-module: Could not resolve dependencies for project org.exist-db.xquery.extensions.expath:expath-crypto-module:jar:6.0.0-SNAPSHOT: Failed to collect dependencies at ro.kuberam.libs.java:crypto:jar:1.7: Failed to read artifact descriptor for ro.kuberam.libs.java:crypto:jar:1.7: Could not transfer artifact ro.kuberam.libs.java:crypto:pom:1.7 from/to maven-default-http-blocker (http://0.0.0.0/): Blocked mirror for repositories: [exist (http://repo.exist-db.org/repository/exist-db/, default, releases+snapshots)] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException

If there are any special steps required for building, perhaps we could add them to the README.

@line-o
Copy link
Member Author

line-o commented Jun 14, 2021

@joewiz I wonder where 0.0.0.0 is coming from.

@line-o
Copy link
Member Author

line-o commented Jun 14, 2021

By the looks of things https should be used ...
What is maven-default-http-blocker?

@line-o
Copy link
Member Author

line-o commented Jun 15, 2021

The leftover IDE settings files were removed and are now ignored.

@line-o
Copy link
Member Author

line-o commented Jun 15, 2021

@joewiz were you able to build the package now?

@joewiz
Copy link
Member

joewiz commented Jun 15, 2021

@line-o No, I still have the same issue as earlier, I think because my local build of crypto-java-lib via mvn clean install fails too (it was only mvn clean package that worked for me, but this wasn't depositing the jars into m2). Here's where mvn clean install fails for me in my clone of crypto-java-lib master:

[INFO] Building jar: /Users/joe/workspace/crypto-java-lib/target/crypto-1.7-javadoc.jar
[INFO] 
[INFO] --- maven-gpg-plugin:1.6:sign (sign-artifacts) @ crypto ---
gpg: all values passed to '--default-key' ignored
gpg: no default secret key: No secret key
gpg: signing failed: No secret key
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  17.955 s
[INFO] Finished at: 2021-06-15T12:41:59-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-gpg-plugin:1.6:sign (sign-artifacts) on project crypto: Exit code: 2 -> [Help 1]

I hope this helps toward the goal of finding the magic formula for a reproducible build of this library, whether in a README or in CI, etc.

@adamretter
Copy link
Contributor

adamretter commented Jun 16, 2021

@joewiz I think GPG should only be needed for publishing (releasing) the library as all artifacts uploaded to Maven Central (and most other repositories) have to be signed first. If you need it for other phases, it is likely that the gpg plugin is misconfigured in the pom.xml.

If you want to work-around the issue, just make sure you have GPG installed and at least one public/private keypair.

@joewiz
Copy link
Member

joewiz commented Jun 16, 2021

@line-o I'm happy to report that having fixed my GPG issue (thanks to hints from @windauer), I can now use mvn install to install crypto-java-lib into my local maven cache. And thus I can now use mvn package to successfully build crypto-exist-java-lib.

@adamretter I agree that crypto-java-lib shouldn't be requiring GPG for mvn install.

For me, steps to build crypto-exist-java-lib are:

  1. Be sure a default GPG secret key ~/.gnupg/gpg.conf is set to a current, non-expired key (gpg --list-keys; renew expired keys via https://gist.github.com/krisleech/760213ed287ea9da85521c7c9aac1df0).

  2. Clone crypto-java-lib and install:

    git clone https://github.com/claudius108/crypto-java-lib.git
    cd crypto-java-lib
    mvn clean install
    
  3. Clone crypto-exist-java-lib and build the xar package:

    git clone https://github.com/eXist-db/crypto-exist-java-lib.git
    cd crypto-exist-java-lib
    mvn clean package
    

@joewiz
Copy link
Member

joewiz commented Jun 27, 2021

Now that crypto-java-lib has been transferred to the expath organization, the updated steps to build crypto-exist-java-lib would be:

  1. Be sure a default GPG secret key ~/.gnupg/gpg.conf is set to a current, non-expired key (gpg --list-keys; renew expired keys via https://gist.github.com/krisleech/760213ed287ea9da85521c7c9aac1df0).

  2. Clone crypto-java-lib and install:

    git clone https://github.com/expath/crypto-java-lib.git
    cd crypto-java-lib
    mvn clean install
    
  3. Clone crypto-exist-java-lib and build the xar package:

    git clone https://github.com/eXist-db/crypto-exist-java-lib.git
    cd crypto-exist-java-lib
    mvn clean package
    

Unfortunately, when I start with a clean ~/.m2 directory, step 3 fails:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.580 s
[INFO] Finished at: 2021-06-27T12:39:17-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project expath-crypto-module: Could not resolve dependencies for project org.exist-db.xquery.extensions.expath:expath-crypto-module:jar:6.0.0-SNAPSHOT: Failed to collect dependencies at org.exist-db:exist-core:jar:5.3.0: Failed to read artifact descriptor for org.exist-db:exist-core:jar:5.3.0: Could not transfer artifact org.exist-db:exist-core:pom:5.3.0 from/to maven-default-http-blocker (http://0.0.0.0/): Blocked mirror for repositories: [exist (http://repo.exist-db.org/repository/exist-db/, default, releases+snapshots)] -> [Help 1]

I'd be happy to back away if testing is premature, or to retry the build using other arguments or steps if anyone has suggestions. I'm just reporting on the current status from my machine (macOS 11.4, liberica-jdk8-full OpenJDK 1.8.0_292-b10), since "merge open PR and release crypto-lib 6.0.0" is an outstanding action item in the master issue for the 5.3.0 release eXist-db/exist#3914.

@windauer
Copy link
Member

@joewiz I missed the release step for Sonatype for 5.3.0 and thats why the build failed, should work fine now. The next step would be to have a https://github.com/expath/crypto-java-lib.git 1.7 release which is not available on Sonatype (Maven Central) yet. Problem is, that the pom.xml there is already 1.7 but without any release. @adamretter what do you think is the best way, bring out a crypto-java-lib.git 1.7 as it is but got never released or switch to "1.7.1-SNAPSHOT" and then do a release.. Even a bit more dirty but correct from my perspective would be to put the pom.xml to 1.7.0-SNAPSHOT and then run a 1.7.0 release. Once this was done crypto-exist-java-lib can be easily build only by running mvn clean package.

@windauer
Copy link
Member

I started a Draft PR on expath/crypto-java-lib: expath/crypto-java#9

- add github-release-plugin
- add developers and contributors
- fix scm.url
- change <organisation>
@windauer
Copy link
Member

windauer commented Jun 28, 2021

I switched the dependencies in pom.xml and xar-assembly.xml to org.expath.crypto crypto-java v.1.8.0 and it worked perfectly fine. @joewiz so only step 3 from your documentation here is required:

$ git clone https://github.com/eXist-db/crypto-exist-java-lib
$ cd crypto-exist-java-lib
$ mvn clean package

I added it this to the README.md but I did not add step 1 @joewiz cause I'm not sure if we wan't to mention this in the README.md.. then we have to even do it in the eXist-db documentation app and many more as well I guess. And what if people don't know GPG do we need to explain how to create one? Perhaps we could add a "valid GPG" key as a requirement to be able to build from source, next to Java and Maven..

@joewiz
Copy link
Member

joewiz commented Jun 28, 2021

@windauer Thank you! The new version built just fine with these instructions and installs into eXist 5.3.0 without error.

Strangely, though, when I run a simple query (adapted from the crypto-java test suite's test for hashing strings, I get an ArrayIndexOutOfBoundsException:

xquery version "3.1";

import module namespace crypto="http://expath.org/ns/crypto";

(: the default encoding is for crypto:hash is "base64", 
 : so we expect the result for this input to be "use1oAoe8vIgnFgygz2OKw==" :)

crypto:hash("Short string for tests.", "MD5")

The error from exist.log:

2021-06-28 17:28:42,093 [qtp310986416-69] ERROR (XQueryServlet.java [process]:550) - 2 
java.lang.ArrayIndexOutOfBoundsException: 2
	at org.expath.exist.crypto.digest.HashFunction.eval(HashFunction.java:76) ~[expath-crypto-module-exist-java-lib-6.0.0-SNAPSHOT.jar:6.0.0-SNAPSHOT]
	at org.exist.xquery.BasicFunction.eval(BasicFunction.java:73) ~[exist-core-5.3.0.jar:5.3.0]
	at org.exist.xquery.InternalFunctionCall.eval(InternalFunctionCall.java:62) ~[exist-core-5.3.0.jar:5.3.0]
	at org.exist.xquery.DynamicCardinalityCheck.eval(DynamicCardinalityCheck.java:73) ~[exist-core-5.3.0.jar:5.3.0]
	at org.exist.xquery.functions.fn.FunExists.eval(FunExists.java:77) ~[exist-core-5.3.0.jar:5.3.0]

Adding a 3rd parameter to specify the encoding returns the expected result without no error thrown:

xquery version "3.1";

import module namespace crypto="http://expath.org/ns/crypto";

crypto:hash("Short string for tests.", "MD5", "hex") eq "bac7b5a00a1ef2f2209c5832833d8e2b",
crypto:hash("Short string for tests.", "MD5", "base64") eq "use1oAoe8vIgnFgygz2OKw=="

The identical test passes in crypto-java, so clearly we have some issue is in crypto-exist-java-lib. I guess the question is: merge now and open an issue about this for fixing later, or investigate this first?

@adamretter
Copy link
Contributor

@joewiz I see there are a lot of XQuery files in src/test/xquery but they are not yet factored as XQSuite tests. Perhaps a first step should be to convert these into XQSuite tests and then introduce CI?

- remove import of Optional in HashFunction and HmacFunction
- refactor for readability
@line-o
Copy link
Member Author

line-o commented Jun 29, 2021

@joewiz the issue you reported was fixed by 270bad2

@windauer
Copy link
Member

I see a need at eXist-db 5.3.0 users for a crypto lib 6.0.0 asap and therefore I would vote for releasing it asap. This code is running on several production systems I'm aware of perfectly fine. And although IT people can easily build a XAR of this PR others don't even know about this repo and will only notice that crypto lib is not working with 5.3.0 and in worst case start to write their own crypto ;) And I'm all in for getting the tests to work, cleaning up the pom, adding it to CI .. afterwards.

@joewiz
Copy link
Member

joewiz commented Jun 29, 2021

@line-o Thank you!

@windauer That's fair! Given the unexpected discovery of the bug in crypto:hash, I thought it would be prudent to perform a fuller test - if only to help prospective users of the new version decide whether they can expect their existing code to work. Rather than reinvent the wheel, I took @adamretter's suggestion to adapt Claudius's tests from https://github.com/eXist-db/crypto-exist-java-lib/tree/master/src/test/java/org/expath/exist/crypto/xquery into xqsuite, and have attached the xqsuite module here: test-crypto.xq.zip.

Below are the current results (using this PR with eXist 5.3.0).

<testsuite package="http://exist-db.org/xquery/test" timestamp="2021-06-29T11:28:22.639-04:00"
    tests="46" failures="13" errors="11" pending="0" time="PT0.032S">
    <testcase name="AWS REST request" class="t:AwsRestRequest">
        <failure message="assertEquals failed." type="failure-error-code-1"
            >jZNOcbfWmD/A/f3hSvVzXZjM2HU=</failure>
        <output>ggrrPVimvAlDa9xalO4+S87TKJY=</output>
    </testcase>
    <testcase name="AWS REST request, default format" class="t:AwsRestRequestWithDefaultFormat">
        <failure
            message="assertEquals failed: wrong number of items returned by function. Expected: 1. Got: 20"
            type="failure-error-code-1">jZNOcbfWmD/A/f3hSvVzXZjM2HU=</failure>
        <output>-115 -109 78 113 -73 -42 -104 63 -64 -3 -3 -31 74 -11 115 93 -104 -52 -40
            117</output>
    </testcase>
    <testcase name="Symmetric decryption of string, AES/CBC/PKCS5Padding"
        class="t:decryptStringWithAesSymmetricKeyCbcMode">
        <error type="java:java.lang.IllegalArgumentException" message="Illegal base64 character 2d"
        />
    </testcase>
    <testcase name="Symmetric decryption of string, AES/CBC/PKCS5Padding, default provider"
        class="t:decryptStringWithAesSymmetricKeyCbcModeDefaultProvider">
        <error type="java:java.lang.IllegalArgumentException" message="Illegal base64 character 2d"
        />
    </testcase>
    <testcase name="Symmetric decryption of string, AES"
        class="t:decryptStringWithAesSymmetricKeyEcbMode">
        <error type="java:java.lang.IllegalArgumentException" message="Illegal base64 character 2d"
        />
    </testcase>
    <testcase name="Symmetric encryption of string, AES/CBC/PKCS5Padding"
        class="t:encryptStringWithAesSymmetricKeyCbcMode">
        <failure message="assertEquals failed." type="failure-error-code-1"
            >51-143-171-200-187-20-34-252-231-243-254-42-36-13-9-123-191-251-243-42-3-238-193-13-155-168-139-67-135-3-143-54</failure>
        <output>M4+ryLsUIvzn8/4qJA0Je7/78yoD7sENm6iLQ4cDjzY=</output>
    </testcase>
    <testcase name="Symmetric encryption of string, AES/CBC/PKCS5Padding, default provider"
        class="t:encryptStringWithAesSymmetricKeyCbcModeDefaultProvider">
        <failure message="assertEquals failed." type="failure-error-code-1"
            >51-143-171-200-187-20-34-252-231-243-254-42-36-13-9-123-191-251-243-42-3-238-193-13-155-168-139-67-135-3-143-54</failure>
        <output>M4+ryLsUIvzn8/4qJA0Je7/78yoD7sENm6iLQ4cDjzY=</output>
    </testcase>
    <testcase name="Symmetric encryption of string, AES"
        class="t:encryptStringWithAesSymmetricKeyEcbMode">
        <failure message="assertEquals failed." type="failure-error-code-1"
            >222-157-20-54-132-99-46-30-73-43-253-148-61-155-86-141-51-56-40-42-31-168-189-56-236-102-58-237-175-171-9-87</failure>
        <output>3p0UNoRjLh5JK/2UPZtWjTM4KCofqL047GY67a+rCVc=</output>
    </testcase>
    <testcase name="Symmetric encryption of string, AES/CBC/PKCS5Padding, wrong key"
        class="t:encryptStringWithAesWrongSymmetricKeyCbcMode">
        <failure
            message="Expected error err:CX19: The secret key is invalid, got: crypto:crypto:invalid-crypto-key crypto:invalid-crypto-key, The cryptographic key is invalid."
            type="failure-error-code-1"/>
    </testcase>
    <testcase
        name="Symmetric encryption of string, AES/CBC/PKCS5Padding, wrong key, default provider"
        class="t:encryptStringWithAesWrongSymmetricKeyCbcModeDefaultProvider">
        <error type="crypto:crypto:invalid-crypto-key"
            message="crypto:invalid-crypto-key, The cryptographic key is invalid."/>
    </testcase>
    <testcase name="Generate enveloped digital signature"
        class="t:generateEnvelopedDigitalSignature">
        <error type="java:java.lang.NullPointerException" message=""/>
    </testcase>
    <testcase name="'MD5' hashing for binary" class="t:hashBinaryWithMd5"/>
    <testcase name="'MD5' hashing for binary, default format"
        class="t:hashBinaryWithMd5AndDefaultFormat">
        <error type="err:XPTY0004"
            message="It is a type error if, during the static analysis phase, an expression is found to have a static type that is not appropriate for the context in which the expression occurs, or during the dynamic evaluation phase, the dynamic type of a value does not match a required type as specified by the matching rules in 2.5.4 SequenceType Matching. XPTY0004: Argument 3:  evaluates to an empty sequence, but an empty argument is not allowed here."
        />
    </testcase>
    <testcase name="'SHA-1' hashing for binary" class="t:hashBinaryWithSha1"/>
    <testcase name="'SHA-1' hashing for binary, default format"
        class="t:hashBinaryWithSha1AndDefaultFormat">
        <error type="err:XPTY0004"
            message="It is a type error if, during the static analysis phase, an expression is found to have a static type that is not appropriate for the context in which the expression occurs, or during the dynamic evaluation phase, the dynamic type of a value does not match a required type as specified by the matching rules in 2.5.4 SequenceType Matching. XPTY0004: Argument 3:  evaluates to an empty sequence, but an empty argument is not allowed here."
        />
    </testcase>
    <testcase name="'SHA-256' hashing for binary" class="t:hashBinaryWithSha256"/>
    <testcase name="'SHA-256' hashing for binary, default format"
        class="t:hashBinaryWithSha256AndDefaultFormat">
        <error type="err:XPTY0004"
            message="It is a type error if, during the static analysis phase, an expression is found to have a static type that is not appropriate for the context in which the expression occurs, or during the dynamic evaluation phase, the dynamic type of a value does not match a required type as specified by the matching rules in 2.5.4 SequenceType Matching. XPTY0004: Argument 3:  evaluates to an empty sequence, but an empty argument is not allowed here."
        />
    </testcase>
    <testcase name="'SHA-384' hashing for binary" class="t:hashBinaryWithSha384"/>
    <testcase name="'SHA-384' hashing for binary, default format"
        class="t:hashBinaryWithSha384AndDefaultFormat">
        <error type="err:XPTY0004"
            message="It is a type error if, during the static analysis phase, an expression is found to have a static type that is not appropriate for the context in which the expression occurs, or during the dynamic evaluation phase, the dynamic type of a value does not match a required type as specified by the matching rules in 2.5.4 SequenceType Matching. XPTY0004: Argument 3:  evaluates to an empty sequence, but an empty argument is not allowed here."
        />
    </testcase>
    <testcase name="'SHA-512' hashing for binary" class="t:hashBinaryWithSha512"/>
    <testcase name="'SHA-512' hashing for binary, default format"
        class="t:hashBinaryWithSha512AndDefaultFormat">
        <error type="err:XPTY0004"
            message="It is a type error if, during the static analysis phase, an expression is found to have a static type that is not appropriate for the context in which the expression occurs, or during the dynamic evaluation phase, the dynamic type of a value does not match a required type as specified by the matching rules in 2.5.4 SequenceType Matching. XPTY0004: Argument 3:  evaluates to an empty sequence, but an empty argument is not allowed here."
        />
    </testcase>
    <testcase name="Hash binary with wrong algorithm" class="t:hashBinaryWithWrongAlgorithm">
        <failure
            message="Expected error err:CX21: The algorithm is not supported., got: crypto:crypto:unknown-algorithm crypto:unknown-algorithm, The specified algorithm is not supported."
            type="failure-error-code-1"/>
    </testcase>
    <testcase name="Hash binary with wrong algorithm, default format"
        class="t:hashBinaryWithWrongAlgorithmAndDefaultFormat">
        <failure
            message="Expected error err:CX21: The algorithm is not supported., got: err:XPTY0004 It is a type error if, during the static analysis phase, an expression is found to have a static type that is not appropriate for the context in which the expression occurs, or during the dynamic evaluation phase, the dynamic type of a value does not match a required type as specified by the matching rules in 2.5.4 SequenceType Matching. XPTY0004: Argument 3:  evaluates to an empty sequence, but an empty argument is not allowed here."
            type="failure-error-code-1"/>
    </testcase>
    <testcase name="'MD5' hashing for string" class="t:hashStringWithMd5"/>
    <testcase name="'MD5' hashing for string, default format"
        class="t:hashStringWithMd5AndDefaultFormat"/>
    <testcase name="'SHA-1' hashing for string" class="t:hashStringWithSha1"/>
    <testcase name="'SHA-1' hashing for string, default format"
        class="t:hashStringWithSha1AndDefaultFormat"/>
    <testcase name="'SHA-256' hashing for string" class="t:hashStringWithSha256"/>
    <testcase name="'SHA-256' hashing for string, default format"
        class="t:hashStringWithSha256AndDefaultFormat"/>
    <testcase name="'SHA-384' hashing for string" class="t:hashStringWithSha384"/>
    <testcase name="'SHA-384' hashing for string, default format"
        class="t:hashStringWithSha384AndDefaultFormat"/>
    <testcase name="'SHA-512' hashing for string" class="t:hashStringWithSha512"/>
    <testcase name="'SHA-512' hashing for string, default format"
        class="t:hashStringWithSha512AndDefaultFormat"/>
    <testcase name="'MD5' hashing for XML file" class="t:hashXmlWithMd5"/>
    <testcase name="'MD5' hashing for XML file, default format"
        class="t:hashXmlWithMd5AndDefaultFormat"/>
    <testcase name="'MD5' HMAC for string" class="t:hmacStringWithMd5"/>
    <testcase name="'MD5' HMAC for string, default format"
        class="t:hmacStringWithMd5AndDefaultFormat">
        <failure
            message="assertEquals failed: wrong number of items returned by function. Expected: 1. Got: 16"
            type="failure-error-code-1">l4MY6Yosjo7W60VJeXB/PQ==</failure>
        <output>-105 -125 24 -23 -118 44 -114 -114 -42 -21 69 73 121 112 127 61</output>
    </testcase>
    <testcase name="'SHA-1' HMAC for string" class="t:hmacStringWithSha1"/>
    <testcase name="'SHA-1' HMAC for string, default format"
        class="t:hmacStringWithSha1AndDefaultFormat">
        <failure
            message="assertEquals failed: wrong number of items returned by function. Expected: 1. Got: 20"
            type="failure-error-code-1">55LyDq7GFnqijauK4CQWR4AqyZk=</failure>
        <output>-25 -110 -14 14 -82 -58 22 122 -94 -115 -85 -118 -32 36 22 71 -128 42 -55
            -103</output>
    </testcase>
    <testcase name="'SHA-256' HMAC for string" class="t:hmacStringWithSha256"/>
    <testcase name="'SHA-256' HMAC for string, default format"
        class="t:hmacStringWithSha256AndDefaultFormat">
        <failure
            message="assertEquals failed: wrong number of items returned by function. Expected: 1. Got: 32"
            type="failure-error-code-1">FfZidcLEUg4oJLIZfw6xHlPMz8KPHxo2liaBKgLfcOE=</failure>
        <output>21 -10 98 117 -62 -60 82 14 40 36 -78 25 127 14 -79 30 83 -52 -49 -62 -113 31 26 54
            -106 38 -127 42 2 -33 112 -31</output>
    </testcase>
    <testcase name="'SHA-384' HMAC for string" class="t:hmacStringWithSha384"/>
    <testcase name="'SHA-384' HMAC for string, default format"
        class="t:hmacStringWithSha384AndDefaultFormat">
        <failure
            message="assertEquals failed: wrong number of items returned by function. Expected: 1. Got: 48"
            type="failure-error-code-1"
            >RRirKZTmx+cG8EXvgrRnpYFPEPYXaZBirY+LFmiUBAK61LCryDsL4clFRG5/BcBr</failure>
        <output>69 24 -85 41 -108 -26 -57 -25 6 -16 69 -17 -126 -76 103 -91 -127 79 16 -10 23 105
            -112 98 -83 -113 -117 22 104 -108 4 2 -70 -44 -80 -85 -56 59 11 -31 -55 69 68 110 127 5
            -64 107</output>
    </testcase>
    <testcase name="'SHA-512' HMAC for string" class="t:hmacStringWithSha512"/>
    <testcase name="'SHA-512' HMAC for string, default format"
        class="t:hmacStringWithSha512AndDefaultFormat">
        <failure
            message="assertEquals failed: wrong number of items returned by function. Expected: 1. Got: 64"
            type="failure-error-code-1"
            >z9MtEpBXxO5bKmsXJWfKsZ4v+RduKU89Y95H2HMGQEwHGefWmewNNQ7urZVuWEU5aeRRdO7G7j0QlcLYv1pkrg==</failure>
        <output>-49 -45 45 18 -112 87 -60 -18 91 42 107 23 37 103 -54 -79 -98 47 -7 23 110 41 79 61
            99 -34 71 -40 115 6 64 76 7 25 -25 -42 -103 -20 13 53 14 -18 -83 -107 110 88 69 57 105
            -28 81 116 -18 -58 -18 61 16 -107 -62 -40 -65 90 100 -82</output>
    </testcase>
    <testcase name="Validate enveloped digital signature"
        class="t:validateEnvelopedDigitalSignature">
        <error type="java:java.lang.NullPointerException" message=""/>
    </testcase>
</testsuite>

I also ran this test suite with eXist 4.7.1 and crypto 5.3.0. The results are similar - indicating either continuing problems in the crypto package and/or problems with the test suite's adaptation of Claudius's tests.

Here are the tests with different results than above (they're all worse in the old code, whereas the new code either passes or supplies real error codes instead of NPEs):

    <testcase name="Symmetric encryption of string, AES/CBC/PKCS5Padding, wrong key"
        class="t:encryptStringWithAesWrongSymmetricKeyCbcMode">
        <failure
            message="Expected error err:CX19: The secret key is invalid, got: java:java.lang.NullPointerException "
            type="failure-error-code-1"/>
    </testcase>
    <testcase
        name="Symmetric encryption of string, AES/CBC/PKCS5Padding, wrong key, default provider"
        class="t:encryptStringWithAesWrongSymmetricKeyCbcModeDefaultProvider">
        <error type="java:java.lang.NullPointerException" message=""/>
    </testcase>

    <testcase name="Hash binary with wrong algorithm" class="t:hashBinaryWithWrongAlgorithm">
        <failure
            message="Expected error err:CX21: The algorithm is not supported., got: java:java.lang.NullPointerException "
            type="failure-error-code-1"/>
    </testcase>

    <testcase name="'MD5' hashing for string, default format"
        class="t:hashStringWithMd5AndDefaultFormat">
        <error type="java:java.lang.ArrayIndexOutOfBoundsException" message="2"/>
    </testcase>
    <testcase name="'SHA-1' hashing for string" class="t:hashStringWithSha1"/>
    <testcase name="'SHA-1' hashing for string, default format"
        class="t:hashStringWithSha1AndDefaultFormat">
        <error type="java:java.lang.ArrayIndexOutOfBoundsException" message="2"/>
    </testcase>
    <testcase name="'SHA-256' hashing for string" class="t:hashStringWithSha256"/>
    <testcase name="'SHA-256' hashing for string, default format"
        class="t:hashStringWithSha256AndDefaultFormat">
        <error type="java:java.lang.ArrayIndexOutOfBoundsException" message="2"/>
    </testcase>
    <testcase name="'SHA-384' hashing for string" class="t:hashStringWithSha384"/>
    <testcase name="'SHA-384' hashing for string, default format"
        class="t:hashStringWithSha384AndDefaultFormat">
        <error type="java:java.lang.ArrayIndexOutOfBoundsException" message="2"/>
    </testcase>
    <testcase name="'SHA-512' hashing for string" class="t:hashStringWithSha512"/>
    <testcase name="'SHA-512' hashing for string, default format"
        class="t:hashStringWithSha512AndDefaultFormat">
        <error type="java:java.lang.ArrayIndexOutOfBoundsException" message="2"/>
    </testcase>
    <testcase name="'MD5' hashing for XML file" class="t:hashXmlWithMd5"/>
    <testcase name="'MD5' hashing for XML file, default format"
        class="t:hashXmlWithMd5AndDefaultFormat">
        <error type="java:java.lang.ArrayIndexOutOfBoundsException" message="2"/>
    </testcase>

So, the good news is that there are fewer failures now with this PR than before (11 now, compared to 17 before), and there are no new failures. So I'd say this PR is absolutely an improvement over the previous release, and should provide users a stable upgrade path for users.

@line-o
Copy link
Member Author

line-o commented Jun 29, 2021

@joewiz 🚀Stellar job!
If we already have tests that can run, then I think that changes things. I will try and incorporate them right away.

@line-o
Copy link
Member Author

line-o commented Jun 29, 2021

But we could also have them in added in a separate commit or PR @joewiz ? That way you will get the honors. 😃

@joewiz
Copy link
Member

joewiz commented Jun 29, 2021

@line-o I'd be happy to add a commit here or submit a PR. Three questions I'd be grateful for your guidance on:

  1. Where should I place the test module?
  2. Should I delete the old tests?
  3. I have one hard-coded path to the directory where I cloned the repository - so that the test suite's setUp step can store the keystore.ks binary file from https://github.com/eXist-db/crypto-exist-java-lib/tree/master/src/test/resources/org/expath/exist/crypto into the database. I'd like to avoid a reference to my filesystem, but I'm not sure how else to expose this binary file to the test suite. If you have suggestions, I can adjust the test module.

@adamretter
Copy link
Contributor

adamretter commented Jun 29, 2021

@joewiz Nice

  1. Where should I place the test module?

If you create a PR that moves your XQSuite tests into src/test/xquery/crypto then this follows the convention in eXist-db, and I can add a stub to have them executed via XQSuite from Maven and CI.

  1. Should I delete the old tests?

I would think so if they are migrated to XQSuite.

  1. I have one hard-coded path to the...

Do you need that binary file, or does the test work with any binary file? If any fixed file will suffice, then I would suggest to xmldb:store a binary file of some constant string like "hello world" in the test setup, and then use that. If you need a very specific file, you should store it into src/test/resources and then we can figure out how to reference it when I setup the instruments for Maven and CI testing.

@joewiz
Copy link
Member

joewiz commented Jun 29, 2021

@adamretter Ok, I'll prepare a PR along these lines. Thanks!

@joewiz
Copy link
Member

joewiz commented Jun 29, 2021

@adamretter I submitted the PR at #39. In the end I kept the keystore.ks binary file since I thought it was used in the test for crypto:generate-signature in https://github.com/eXist-db/crypto-exist-java-lib/pull/39/files#diff-ed513a1e2f516c2c2808b598d7c364220a1ef0d950606ffa5fe2bc60a3e57bedR173. Now I see it's not actually used in the function... Besides, though, for the other tests that reference it in calculating hashes on binaries, I didn't want to recalculate these, lest I introduce new errors into the PR.

@line-o
Copy link
Member Author

line-o commented Jun 29, 2021

Would someone not invested in this PR merge it please. Assuming that #39 is pulled in later @joewiz

@joewiz
Copy link
Member

joewiz commented Jun 29, 2021

Yes, I think the results above show that this PR is ready to be merged, and the work on getting the xqsuite can proceed in #39.

@line-o @chakl @adamretter Thank you all for establishing this new foundation for the crypto project!

And of course thanks to @claudius108 for creating it in the first place and in assisting us in these most recent steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants