Skip to content

Commit

Permalink
Remove bouncy castle shaded module to avoid bring error of verifySing…
Browse files Browse the repository at this point in the history
…leJar (apache#7453)

### Motivation

shade bouncy castle will cause some signature errors, this PR tries to remove the bouncy castle shaded module.

Here is the related error stack:
```
10:01:34.257 [pulsar-client-io-33-1] ERROR org.apache.pulsar.client.impl.ConsumerImpl - MessageCryptoBc may not included in the jar. e:
java.lang.SecurityException: JCE cannot authenticate the provider BC
	at javax.crypto.Cipher.getInstance(Cipher.java:657) ~[?:1.8.0_121]
	at javax.crypto.Cipher.getInstance(Cipher.java:596) ~[?:1.8.0_121]
	at org.apache.pulsar.client.impl.crypto.MessageCryptoBc.<init>(MessageCryptoBc.java:147) ~[classes/:?]
	at org.apache.pulsar.client.impl.ConsumerImpl.<init>(ConsumerImpl.java:270) ~[classes/:?]
	at org.apache.pulsar.client.impl.ConsumerImpl.newConsumerImpl(ConsumerImpl.java:209) ~[classes/:?]
	at org.apache.pulsar.client.impl.PulsarClientImpl.lambda$doSingleTopicSubscribeAsync$5(PulsarClientImpl.java:364) ~[classes/:?]
	at java.util.concurrent.CompletableFuture.uniAccept(CompletableFuture.java:656) ~[?:1.8.0_131]
	at java.util.concurrent.CompletableFuture$UniAccept.tryFire(CompletableFuture.java:632) ~[?:1.8.0_131]
...

Caused by: java.util.jar.JarException: file:/Users/jia/.m2/repository/org/apache/pulsar/bouncy-castle-bc-shaded/2.7.0-SNAPSHOT/bouncy-castle-bc-shaded-2.7.0-SNAPSHOT.jar has unsigned entries - org/bouncycastle/cert/AttributeCertificateHolder.class
	at javax.crypto.JarVerifier.verifySingleJar(JarVerifier.java:500) ~[?:1.8.0_121]
	at javax.crypto.JarVerifier.verifyJars(JarVerifier.java:361) ~[?:1.8.0_121]
	at javax.crypto.JarVerifier.verify(JarVerifier.java:289) ~[?:1.8.0_121]
	at javax.crypto.JceSecurity.verifyProviderJar(JceSecurity.java:159) ~[?:1.8.0_121]
	at javax.crypto.JceSecurity.getVerificationResult(JceSecurity.java:185) ~[?:1.8.0_121]
	at javax.crypto.Cipher.getInstance(Cipher.java:653) ~[?:1.8.0_121]
```

### Modifications

- Remove bouncy castle shaded module, avoid package bouncy castle into a dependency jar.
- enhance test case to identify this error.

### Verifying this change

ut passed.


* remove dep of bc-shaded from other module

* remove bc-shaded module

* enhance testECDSAEncryption and testRSAEncryption to cover error case

* fix license check

* remove bc-shaded module

* build a jar in jar to avoid break bc signature

* use new bc dependency by classifier in maven

* build pulsar-all docker image instead of pull from dockerhub in integration tests

* remove nar

* fix licence, fix error brings in apache#7640

* add bc when broker/client is referenced in pom

* add missing bc reference in pom

* change ci back to not build docker image
  • Loading branch information
jiazhai authored Oct 23, 2020
1 parent 7f9b7cf commit ef614ad
Show file tree
Hide file tree
Showing 48 changed files with 236 additions and 937 deletions.
84 changes: 0 additions & 84 deletions bouncy-castle/bc-shaded/pom.xml

This file was deleted.

18 changes: 16 additions & 2 deletions bouncy-castle/bc/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,23 @@

<build>
<plugins>

<!-- build a `jar in jar` to avoid break bc signature-->
<plugin>
<groupId>org.apache.nifi</groupId>
<artifactId>nifi-nar-maven-plugin</artifactId>
<groupId>de.ntcomputer</groupId>
<artifactId>executable-packer-maven-plugin</artifactId>
<version>1.0.1</version>
<configuration>
<finalName>${project.artifactId}-${project.version}</finalName>
<mainClass>org.apache.pulsar.bcloader.BouncyCastleLoader</mainClass>
</configuration>
<executions>
<execution>
<goals>
<goal>pack-executable-jar</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
Expand Down
4 changes: 2 additions & 2 deletions bouncy-castle/bcfips-include-test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
<exclusions>
<exclusion>
<groupId>${project.groupId}</groupId>
<artifactId>bouncy-castle-bc-shaded</artifactId>
<artifactId>bouncy-castle-bc</artifactId>
</exclusion>
</exclusions>
<type>test-jar</type>
Expand All @@ -61,7 +61,7 @@
<exclusions>
<exclusion>
<groupId>${project.groupId}</groupId>
<artifactId>bouncy-castle-bc-shaded</artifactId>
<artifactId>bouncy-castle-bc</artifactId>
</exclusion>
</exclusions>
<scope>test</scope>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,15 @@ public void testTlsClientAuthOverBinaryProtocol() throws Exception {
Assert.fail("Server should have failed the TLS handshake since client didn't .");
} catch (Exception ex) {
// OK
log.info("first test success: without certs set, meet exception {}", ex);
}

// Test 2 - Using TLS on binary protocol - sending certs
internalSetUpForClient(true, pulsar.getBrokerServiceUrlTls());
try {
pulsarClient.newConsumer().topic("persistent://my-property/use/my-ns/my-topic1")
.subscriptionName("my-subscriber-name").subscriptionType(SubscriptionType.Exclusive).subscribe();
log.info("second test success: with certs set, consumer sub success");
} catch (Exception ex) {
Assert.fail("Should not fail since certs are sent.");
}
Expand All @@ -116,13 +118,15 @@ public void testTlsClientAuthOverHTTPProtocol() throws Exception {
Assert.fail("Server should have failed the TLS handshake since client didn't .");
} catch (Exception ex) {
// OK
log.info("first test success: without certs set, meet exception {}", ex);
}

// Test 2 - Using TLS on https - sending certs
internalSetUpForClient(true, pulsar.getWebServiceAddressTls());
try {
pulsarClient.newConsumer().topic("persistent://my-property/use/my-ns/my-topic1")
.subscriptionName("my-subscriber-name").subscriptionType(SubscriptionType.Exclusive).subscribe();
log.info("second test success: with certs set, consumer sub success");
} catch (Exception ex) {
Assert.fail("Should not fail since certs are sent.");
}
Expand Down
102 changes: 0 additions & 102 deletions bouncy-castle/bcfips-nar-test/pom.xml

This file was deleted.

This file was deleted.

Loading

0 comments on commit ef614ad

Please sign in to comment.