Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BC-FIPS 2.0 cache eviction throws error when using Hashicorp Vault OCSP responder. #1789

Open
revanthalampally opened this issue Aug 26, 2024 · 7 comments

Comments

@revanthalampally
Copy link

revanthalampally commented Aug 26, 2024

  1. A TLS JAVA client(using BCFIPS 2.0) is talking to a server which is using Hashicorp Vault issued certificate.

  2. The TLS JAVA client(BCFIPS) is doing OCSP validation of the server certificate by sending OCSP request to the vault OCSP responder.

  3. The BCFIPS maintains a cache of the OCSP response it gets from the responder.

  4. Suppose the TLS java client is trying to make one more connection to the same server after the OCSP interval time (in our case its 12 hours in vault), then BCFIPS is returning error while doing OCSP check of the server cert. The error is "OCSP cache expired."

  5. When we investigated this issue and found that the BCFIPS caching logic is not able to evict the expired certificate because of a NULL element present as part of CertID(hashing algorithm) of the OCSP response.

Hashicorp has responded that setting the hash algorithm parameters field to an explicit ASN.1 NULL value is valid
There are other examples of code doing the same:

https://github.com/snowflakedb/gosnowflake/blob/master/ocsp.go#L279

https://github.com/golang/crypto/blob/master/ocsp/ocsp.go

Example here

similarly with [www.microsoft.com]: results here

This behavior your java client is experiencing appears to be due to the way the BCFIPS library is handling the responses.

There needs to be a way to ignore the NULL values when BCFIPS is storing in its responseMap in order to avoid this.

This is blocking us from our fedramp release. Can someone please look into this soon?

@peterdettman
Copy link
Collaborator

It's not completely clear what the problem is, so could you confirm my understanding please:

  1. A first OCSP request is made for a certificate not in the cache. The response goes into the cache indexed by a CertID created locally by BCFIPS.
  2. A second OCSP request for the same certificate finds the cached response, however (none of) the CertID(s) contained in the SingleResponse object(s) actually compares equal to the locally-created CertID used by the cache (response validation at step 1 did not detect this).
  3. Since the CertID never matches, this response will never be evicted from the cache based on nextUpdate expiry.
  4. The reason for the CertID inequality is that the BCFIPS one has CertID.hashAlgorithm.parameters absent, whereas the one in the response that should match actually has an explicit ASN.1 NULL value in CertID.hashAlgorithm.parameters.

FYI, ASN.1 NULL in the parameters field is only really valid for particular digest algorithms (for historical reasons), however in a situation like this we would probably choose to be tolerant. I am curious what the digest algorithm OID (CertID.hashAlgorithm.algorithm) is in this case though.

@revanthalampally
Copy link
Author

@peterdettman : 1. is correct.
In 2, A second OCSP request is made since this TLS connection is after the cached certID's expiry( i.e. the lastUpdate time of the first OCSP response that gets stored in the responseMap).
Now, when BCFIPS get the OCSP response from Vault, the cache eviction logic kicks in since certID from the second response matches the responseMap.get(certID).

Now, cache eviction logic kicks in, and it fails here since the certID from the OCSP responder (during the second call) and the certID in the responseMap(stored from the first call's response) are not equal because one has a NULL parameter, whereas the cache has DERNull.

The RFC governs the OCSP response->CertID->hashingAlgorithm says below:

--  mda-sha1 DIGEST-ALGORITHM ::= {
--      IDENTIFIER id-sha1
--      PARAMS TYPE NULL ARE preferredAbsent
--  }
   required,         -- Parameters MUST be encoded in structure
   preferredPresent, -- Parameters SHOULD be encoded in structure
   preferredAbsent,  -- Parameters SHOULD NOT be encoded in structure
   absent,           -- Parameters MUST NOT be encoded in structure
   inheritable,      -- Parameters are inherited if not present
   optional,         -- Parameters MAY be encoded in the structure
   ...
}

preferredAbsent means Parameters SHOULD NOT be encoded in structure, should not doesn't mean it can't be, otherwise it would be listed as absent.

@dghgit
Copy link
Contributor

dghgit commented Aug 29, 2024

@revanthalampally if you register this one with the support portal at Keyfactor we will be able to organise a patch release for you.

@revanthalampally
Copy link
Author

revanthalampally commented Aug 29, 2024

@dghgit : Im not sure which support portal are you talking about. Could you share the link? Are you talking about: https://support.keyfactor.com/hc/en-us
I don't seem to have login and there is not option to sign up also.

Also we need this in BCFIPS not just commercial BC.

@dghgit
Copy link
Contributor

dghgit commented Aug 29, 2024

The portal is here -> https://support.keyfactor.com/hc/en-us

Contract details if you need them available at https://www.keyfactor.com/open-source/bouncy-castle-support/

Note although we can put a patch together quickly, getting a certified update to a FIPS API is not an easy or cheap task.

hubot pushed a commit that referenced this issue Sep 1, 2024
@revanthalampally
Copy link
Author

@dghgit : Wondering why do you need to involve key factor here?

@dghgit
Copy link
Contributor

dghgit commented Sep 3, 2024

They have the license for support contracts and the BC team is currently working there. It's how we fund this work - the getting of and maintenance of FIPS runs into 100K+ USD, this is not something you can fund using a lemonade stand or a chook raffle.

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

No branches or pull requests

10 participants
@peterdettman @dghgit @revanthalampally and others