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

Address- Investigate and fix CA installation is failing in exporting … #1018

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

jmagne
Copy link
Contributor

@jmagne jmagne commented Aug 8, 2024

…the admin certificate at pk12util command in FIPS mode.

https://issues.redhat.com/browse/RHCS-5222

The fix to follow addresses the part of the above issue with respect to how PKI through JSS creates p12 files. This patch modifies the procedure to include higher rated algs for things such as the MAC of the entire PFX and the HMAC and possible algs allowed when creating the encrypted private key info blob to place in the private key safe bag.

Currently we support our own version of PK11_ExportEncryptedPrivKeyInfoV2 that , to this point has served two purposes:

  1. Allow us to use the new AES key wrap KWP algs.
  2. In the case of fips mode, we have added a routine that moves a key between slots when needed, which doesn't currently work in the current nss routine.

The fix implements changes that alows the routine to support the various AES_CBC enc algs as well as KWP. KWP is called by the pki kra when creating p12 files, if so configured to do so. Alternatively we have a pkcs12 related comand utility that specifies AES_256_CBC.

The fix to JSS simply upgrades some defaults at this point. If we want to get more involved, we could also modify the cmd line tools to be able to specify the algs in question through params.

…the admin certificate at pk12util command in FIPS mode.

https://issues.redhat.com/browse/RHCS-5222

The fix to follow addresses the part of the above issue with respect to how PKI through JSS creates p12 files.
This patch modifies the procedure to include higher rated algs for things such as the MAC of the entire PFX and the HMAC and possible
algs allowed when creating the encrypted private key info blob to place in the private key safe bag.

Currently we support our own version of PK11_ExportEncryptedPrivKeyInfoV2 that , to this point has served two purposes:

1. Allow us to use the new AES key wrap KWP algs.
2. In the case of fips mode, we have added a routine that moves a key between slots when needed, which doesn't currently
work in the current nss routine.

The fix implements changes that alows the routine to support the various AES_CBC enc algs as well as KWP. KWP is called by the pki
kra when creating p12 files, if so configured to do so. Alternatively we have a pkcs12 related comand utility that specifies AES_256_CBC.

The fix to JSS simply upgrades some defaults at this point. If we want to get more involved, we could also modify the cmd line tools to be able
to specify the algs in question through params.
Copy link

sonarqubecloud bot commented Aug 8, 2024

@jmagne jmagne requested a review from ladycfu August 12, 2024 16:46
Copy link
Contributor

@ladycfu ladycfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to my other comments, one question I have is regarding backward compatibility. If one has a p12 file generated from an earlier platform, could the pkcs12 command utility handle import?

TokenException, CharConversionException
{

//Make this alg the default mac alg.
AlgorithmIdentifier algID = new AlgorithmIdentifier(DigestAlgorithm.SHA256.toOID());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that you are changing the default mac alg, but is there not a need to allow caller of computeMacData() to specify algID for better future support?

JSS_throwMsgPrErr(
algTag, /* PBE algorithm to encrypt the key with */
SEC_OID_UNKNOWN, /* Encryption algorithm to Encrypt the key with */
SEC_OID_HMAC_SHA256, /* Hash algorithm for PRF, make this upgraded to SHA256, we can make configurable in future. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like you intentionally hard-code this. I somehow got the impression from your git commit comment that the fix would allow for more flexible specifications of the algorithms.

Copy link
Contributor

@ladycfu ladycfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, according to @jmagne , this PR only addresses "export".

@jmagne jmagne merged commit 5eac839 into dogtagpki:master Aug 13, 2024
31 of 35 checks passed
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.

2 participants