olamy commented on PR #591: URL: https://github.com/apache/mina-sshd/pull/591#issuecomment-2325318758
@tomaswolf Thanks a lot!! > @olamy: All right. The original idea _was_ sound after all. With this PR at commit [9518312](https://github.com/apache/mina-sshd/commit/9518312b76aa4552343c56ad58b54e0ebb0b4950) and the following patch applied to your test [jenkinsci/mina-sshd-api-plugin#114](https://github.com/jenkinsci/mina-sshd-api-plugin/pull/114) that test succeeds. > > Patch: > > ``` > public class FIPSBouncyCastleSecurityProviderRegistar extends AbstractSecurityProviderRegistrar { > > public FIPSBouncyCastleSecurityProviderRegistar() { > - super("BCFIPS"); > + super("BC"); > + } > + > + @Override > + public String getProviderName() { > + return "BCFIPS"; > } > > @Override > ``` > > Test run succeeds: > > ``` > [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.665 s -- in io.jenkins.plugins.mina_sshd_api.core.bouncycaslte_registar.TestBCFIPSRegistar > ``` > > Note that I did not include the customizeable RandomFactory. Not sure it's needed; it'll use `JceRandom` with BCFIPS anyway. (And JceRandom is changed to use `SecureRandom.getInstanceStrong()`.) > Right so it's not needed anymore. > Two observations: > > 1. The typo in the package name I mentioned before is io.jenkins.plugins.mina_sshd_api.core.bouncyca**slte**_registar. That should probably be "bouncyca**stl**e_registrar". Haha good catch. I was checking if String constants in System.setProperty were right but the problem was something else. Uhm I need some new glasses... > 2. I think typical applications will include either the BCFIPS bundles _or_ the normal BC bundles, but not both. Mix and match won't work anyway. In such typical applications, there shouldn't be a need for an extra registrar at all, since the built-in `BouncyCastleSecurityRegistrar` can handle either, depending on what is accessible or what security providers are installed already. I presume no application would have both the BCFIPS _and_ the BC provider installed. > > Your use case seems to be in Jenkins and apparently both libraries (BC and BCFIPS) may be present. That use case may still need its special-purpose registrar. Also you mentioned you'd not want to use the SunJCE AES, so a system property to disable the SunJCEWrapper registrar would be needed anyway if you don't override the registrars. (If you do -- as is currently the case -- that SunJCEWrapper won't be registered at all.) No we don't. If Jenkins is running in FIPS mode, the classic BC is not registered (not easy to figure when looking at the poms only but it's here https://github.com/jenkinsci/bouncycastle-api-plugin/blob/e27176eb46cbe94c1d1de6a9f318e3d538c34a4a/src/main/java/jenkins/bouncycastle/api/BouncyCastlePlugin.java#L27) -- 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-unsubscr...@mina.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
