Skip to content

Mechanism to configure AES-GCM in TLS 1.2 with custom nonce generator - #1950

Merged
hubot merged 1 commit into
bcgit:mainfrom
crlorentzen:main
Dec 24, 2024
Merged

Mechanism to configure AES-GCM in TLS 1.2 with custom nonce generator#1950
hubot merged 1 commit into
bcgit:mainfrom
crlorentzen:main

Conversation

@crlorentzen

Copy link
Copy Markdown
Contributor

This code enables a builder to override the default nonce generator class for BouncyCastleJsseProvider. For developers who wish to use an alternative provider which exposes its nonce generator.

@dghgit

dghgit commented Dec 22, 2024

Copy link
Copy Markdown
Contributor

I think this is mostly okay, on further review though, one small question:

import static org.bouncycastle.jsse.provider.GcmTls12NonceGeneratorUtil.createGcmFipsNonceGenerator;
import static org.bouncycastle.jsse.provider.GcmTls12NonceGeneratorUtil.isGcmFipsNonceGeneratorFactorySet;

Is there a particular reason GcmTls12NonceGeneratorUtil in the provider package? There shouldn't really be any dependencies between org.bouncycastle.tls and org.bouncycastle.jsse - not everyone has or uses the JSSE (believe it or not...). We're also trying to avoid public classes in the provider package (other than the provider). It looks like it would make sense to move the classes to org.bouncycastle.tls.crypto though. Happy to make the change here, just thought I should check I wasn't missing something. Thanks.

@dghgit dghgit self-assigned this Dec 22, 2024
@crlorentzen

Copy link
Copy Markdown
Contributor Author

Thanks for the review;
I agree, we don't see a need to build the put it in the provider package. We have no concern with it being migrated to org.bouncycastle.tls. I'm happy to let you do the conversion, or if you prefer I can with your guidance; to ensure I put the classes where it make the most sense.

Guidance request: what is the preferred location for the GcmTls12NonceGeneratorUtil and TlsNonceGeneratorFactory.

  • package org.bouncycastle.tls.crypto.impl
  • package org.bouncycastle.tls.crypto
  • package org.bouncycastle.tls

hubot pushed a commit that referenced this pull request Dec 24, 2024
@hubot
hubot merged commit c35119b into bcgit:main Dec 24, 2024
@dghgit

dghgit commented Dec 26, 2024

Copy link
Copy Markdown
Contributor

I've done it here, they ended up in crypto.impl. I also rolled in some code from our TLS fips library so in addition to the class move TlsNonceGeneratorFactory in now AEADNonceGeneratorFactory, the difference there is that AEADNonceGeneratorFactory now returns an AEADNonceGenerator (formerly just in FIPS), I've trialed this implementation with how we do it in FIPS as well and it appears to be working okay. Let me know what you think.

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.

3 participants