You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@santuario.apache.org by "coheigea (via GitHub)" <gi...@apache.org> on 2023/08/28 05:33:10 UTC
[GitHub] [santuario-xml-security-java] coheigea opened a new pull request, #204: Adding a "bouncycastle" provider to run the bouncycastle specific tests
coheigea opened a new pull request, #204:
URL: https://github.com/apache/santuario-xml-security-java/pull/204
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@santuario.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [santuario-xml-security-java] seanjmullan commented on a diff in pull request #204: Adding a "bouncycastle" provider to run the bouncycastle specific tests
Posted by "seanjmullan (via GitHub)" <gi...@apache.org>.
seanjmullan commented on code in PR #204:
URL: https://github.com/apache/santuario-xml-security-java/pull/204#discussion_r1318772045
##########
src/test/java/org/apache/xml/security/test/dom/algorithms/SignatureAlgorithmTest.java:
##########
@@ -104,18 +104,23 @@ void testConstructionWithProvider() throws Exception {
assertFalse(algorithmHash.isEmpty());
Document doc = TestUtils.newDocument();
- Provider provider = new org.bouncycastle.jce.provider.BouncyCastleProvider();
-
- for (String algorithmURI : algorithmHash.keySet()) {
- try {
- AlgorithmParameterSpec spec = algorithmURI.equals(XMLSignature.ALGO_ID_SIGNATURE_RSA_PSS)
- ? new PSSParameterSpec("SHA-256", "MGF1", MGF1ParameterSpec.SHA256, 32, 1)
- : null;
- SignatureAlgorithm signatureAlgorithm = new SignatureAlgorithm(doc, algorithmURI, provider, spec);
- assertEquals(provider.getName(), signatureAlgorithm.getJCEProviderName());
- } catch (XMLSecurityException e) {
- assertEquals("", Arrays.asList(e.getStackTrace()).toString());
+ try {
+ Class<?> bouncyCastleProviderClass = Class.forName("org.bouncycastle.jce.provider.BouncyCastleProvider");
+ Provider provider = (Provider)bouncyCastleProviderClass.getConstructor().newInstance();
+
+ for (String algorithmURI : algorithmHash.keySet()) {
+ try {
+ AlgorithmParameterSpec spec = algorithmURI.equals(XMLSignature.ALGO_ID_SIGNATURE_RSA_PSS)
+ ? new PSSParameterSpec("SHA-256", "MGF1", MGF1ParameterSpec.SHA256, 32, 1)
+ : null;
+ SignatureAlgorithm signatureAlgorithm = new SignatureAlgorithm(doc, algorithmURI, provider, spec);
+ assertEquals(provider.getName(), signatureAlgorithm.getJCEProviderName());
+ } catch (XMLSecurityException e) {
+ assertEquals("", Arrays.asList(e.getStackTrace()).toString());
+ }
}
+ } catch (ReflectiveOperationException e) {
+ // BouncyCastle not installed, ignore
}
Review Comment:
It might be useful to add `Assumptions.assumeNotNull(provider)` here so the test is skipped instead of ignored. (You will need to expand the scope of the `provider` object.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@santuario.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [santuario-xml-security-java] seanjmullan commented on pull request #204: Adding a "bouncycastle" provider to run the bouncycastle specific tests
Posted by "seanjmullan (via GitHub)" <gi...@apache.org>.
seanjmullan commented on PR #204:
URL: https://github.com/apache/santuario-xml-security-java/pull/204#issuecomment-1714524482
Thanks. I think you may have misunderstood my latter comment though. What I was thinking if we could run all of the tests with this profile and be sure that BC was used as the default JCE provider for all of the tests, as this would give us a better assurance that all Santuario APIs worked across the board with different JCE providers. Typically this would be done by editing the `java.security` file and adding it as the first provider. For testing, however, modifying the JDK under test is usually not a viable option, so this could be done instead by specifying an alternate `java.security` file with the `java.security.properties` system property, ex:
`java -Djava.security.properties=my.java.security ...`
where `my.java.security` is a file with an entry with the first provider as BC followed by the existing JDK providers, all renumbered, something like:
```
security.provider.1=org.bouncycastle.jce.provider.BouncyCastleProvider
security.provider.2=SUN
security.provider.3=SunRsaSign
security.provider.4=SunEC
security.provider.5=SunJSSE
security.provider.6=SunJCE
security.provider.7=SunJGSS
security.provider.8=SunSASL
security.provider.9=XMLDSig
security.provider.10=SunPCSC
security.provider.11=JdkLDAP
security.provider.12=JdkSASL
security.provider.13=Apple
security.provider.14=SunPKCS11
```
This might be a bit hard to pull off, but would be a very valuable feature as it would provide comprehensive interoperability testing of at least 2 different JCE providers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@santuario.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [santuario-xml-security-java] coheigea merged pull request #204: Adding a "bouncycastle" provider to run the bouncycastle specific tests
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea merged PR #204:
URL: https://github.com/apache/santuario-xml-security-java/pull/204
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@santuario.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [santuario-xml-security-java] coheigea commented on pull request #204: Adding a "bouncycastle" provider to run the bouncycastle specific tests
Posted by "coheigea (via GitHub)" <gi...@apache.org>.
coheigea commented on PR #204:
URL: https://github.com/apache/santuario-xml-security-java/pull/204#issuecomment-1711689337
Good point, I added the assumptions. Regarding tests, we could just run the tests twice as part of CI,i.e "mvn clean install; mvn clean install -Pbouncycastle"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@santuario.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [santuario-xml-security-java] seanjmullan commented on a diff in pull request #204: Adding a "bouncycastle" provider to run the bouncycastle specific tests
Posted by "seanjmullan (via GitHub)" <gi...@apache.org>.
seanjmullan commented on code in PR #204:
URL: https://github.com/apache/santuario-xml-security-java/pull/204#discussion_r1318773801
##########
src/test/java/org/apache/xml/security/test/dom/signature/SignatureTest.java:
##########
@@ -93,13 +96,18 @@ void testSigningVerifyingFromExistingSignature() throws Throwable {
@Test
void testSigningVerifyingFromExistingSignatureWithProvider() throws Throwable {
- Provider provider = new org.bouncycastle.jce.provider.BouncyCastleProvider();
- Document doc = getOriginalDocument();
- XMLSignature signature = signDocument(doc, provider);
- assertEquals(provider.getName(), signature.getSignedInfo().getSignatureAlgorithm().getJCEProviderName());
-
- PublicKey pubKey = getPublicKey();
- assertTrue(signature.checkSignatureValue(pubKey));
+ try {
+ Class<?> bouncyCastleProviderClass = Class.forName("org.bouncycastle.jce.provider.BouncyCastleProvider");
+ Provider provider = (Provider)bouncyCastleProviderClass.getConstructor().newInstance();
+ Document doc = getOriginalDocument();
+ XMLSignature signature = signDocument(doc, provider);
+ assertEquals(provider.getName(), signature.getSignedInfo().getSignatureAlgorithm().getJCEProviderName());
+
+ PublicKey pubKey = getPublicKey();
+ assertTrue(signature.checkSignatureValue(pubKey));
+ } catch (ReflectiveOperationException e) {
+ // BouncyCastle not installed, ignore
+ }
}
Review Comment:
Same comment here as before.
##########
src/test/java/org/apache/xml/security/test/dom/signature/SignatureTest.java:
##########
@@ -69,17 +66,23 @@ void testSigningVerifyingFromRebuildSignature() throws Throwable {
@Test
void testSigningVerifyingFromRebuildSignatureWithProvider() throws Throwable {
- Provider provider = new org.bouncycastle.jce.provider.BouncyCastleProvider();
- Document doc = getOriginalDocument();
- XMLSignature signature = signDocument(doc, provider);
- assertEquals(provider.getName(), signature.getSignedInfo().getSignatureAlgorithm().getJCEProviderName());
-
- Element signatureElem = (Element) doc.getElementsByTagNameNS(DS_NS, "Signature").item(0);
- signature = new XMLSignature(signatureElem, "", provider);
- assertEquals(provider.getName(), signature.getSignedInfo().getSignatureAlgorithm().getJCEProviderName());
-
- PublicKey pubKey = getPublicKey();
- assertTrue(signature.checkSignatureValue(pubKey));
+ try {
+ Class<?> bouncyCastleProviderClass = Class.forName("org.bouncycastle.jce.provider.BouncyCastleProvider");
+ Provider provider = (Provider)bouncyCastleProviderClass.getConstructor().newInstance();
+
+ Document doc = getOriginalDocument();
+ XMLSignature signature = signDocument(doc, provider);
+ assertEquals(provider.getName(), signature.getSignedInfo().getSignatureAlgorithm().getJCEProviderName());
+
+ Element signatureElem = (Element) doc.getElementsByTagNameNS(DS_NS, "Signature").item(0);
+ signature = new XMLSignature(signatureElem, "", provider);
+ assertEquals(provider.getName(), signature.getSignedInfo().getSignatureAlgorithm().getJCEProviderName());
+
+ PublicKey pubKey = getPublicKey();
+ assertTrue(signature.checkSignatureValue(pubKey));
+ } catch (ReflectiveOperationException e) {
+ // BouncyCastle not installed, ignore
+ }
}
Review Comment:
Same comment here as before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@santuario.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [santuario-xml-security-java] seanjmullan commented on pull request #204: Adding a "bouncycastle" provider to run the bouncycastle specific tests
Posted by "seanjmullan (via GitHub)" <gi...@apache.org>.
seanjmullan commented on PR #204:
URL: https://github.com/apache/santuario-xml-security-java/pull/204#issuecomment-1710481037
Another enhancement related to this that I was thinking is to (optionally) run **all** of the tests with BouncyCastle installed as the first provider. This would give us more confirmation that Santuario works with different JCE providers (those installed in the JDK or BouncyCastle). This might require some Junit wizardry that I'm not an expert enough to know if it is possible or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@santuario.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org