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

Fix ops flag mask master #4649

Merged
merged 4 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions .github/workflows/ca-hsm-operation-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
name: CA with HSM and custom operation key flags

on: workflow_call

env:
DB_IMAGE: ${{ vars.DB_IMAGE || 'quay.io/389ds/dirsrv' }}

jobs:
# docs/installation/ca/Installing_CA_with_HSM.md
test:
name: Test
runs-on: ubuntu-latest
env:
SHARED: /tmp/workdir/pki
steps:
- name: Clone repository
uses: actions/checkout@v3

- name: Retrieve PKI images
uses: actions/cache@v3
with:
key: pki-images-${{ github.sha }}
path: pki-images.tar

- name: Load PKI images
run: docker load --input pki-images.tar

- name: Create network
run: docker network create example

- name: Set up DS container
run: |
tests/bin/ds-container-create.sh ds
env:
IMAGE: ${{ env.DB_IMAGE }}
HOSTNAME: ds.example.com
PASSWORD: Secret.123

- name: Connect DS container to network
run: docker network connect example ds --alias ds.example.com

- name: Set up PKI container
run: |
tests/bin/runner-init.sh pki
env:
HOSTNAME: pki.example.com

- name: Connect PKI container to network
run: docker network connect example pki --alias pki.example.com

- name: Install dependencies
run: |
docker exec pki dnf install -y softhsm

- name: Create SoftHSM token
run: |
# allow PKI user to access SoftHSM files
docker exec pki usermod pkiuser -a -G ods

# create SoftHSM token for PKI server
docker exec pki runuser -u pkiuser -- \
softhsm2-util \
--init-token \
--label HSM \
--so-pin Secret.HSM \
--pin Secret.HSM \
--free

docker exec pki ls -laR /var/lib/softhsm/tokens

- name: Install CA with HSM and no sign flag
run: |
docker exec pki pkispawn \
-f /usr/share/pki/server/examples/installation/ca.cfg \
-s CA \
-D pki_instance_name=pki-failing-tomcat \
-D pki_ds_url=ldap://ds.example.com:3389 \
-D pki_hsm_enable=True \
-D pki_token_name=HSM \
-D pki_token_password=Secret.HSM \
-D pki_server_database_password=Secret.123 \
-D pki_ca_signing_token=HSM \
-D pki_ocsp_signing_token=HSM \
-D pki_audit_signing_token=HSM \
-D pki_subsystem_token=HSM \
-D pki_sslserver_token=internal \
-D pki_ca_signing_opsFlagMask=sign \
-v
continue-on-error: true
id: hsm_no_sign

- name: Check the install with no sign ops failed
if: job.steps.hsm_no_sign.status != failure()
run: exit 1

- name: Install CA with HSM reintroducing sign flag
run: |
docker exec pki pkispawn \
-f /usr/share/pki/server/examples/installation/ca.cfg \
-s CA \
-D pki_ds_url=ldap://ds.example.com:3389 \
-D pki_hsm_enable=True \
-D pki_token_name=HSM \
-D pki_token_password=Secret.HSM \
-D pki_server_database_password=Secret.123 \
-D pki_ca_signing_token=HSM \
-D pki_ocsp_signing_token=HSM \
-D pki_audit_signing_token=HSM \
-D pki_subsystem_token=HSM \
-D pki_sslserver_token=internal \
-D pki_ca_signing_opsFlag=sign \
-D pki_ca_signing_opsFlagMask=sign \
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too familiar with how the flag & mask work in NSS, but does this mean the mask should ultimately disable the sign operation in NSS so the installation should fail too? Or does opsFlagMask simply remove the values from opsFlag (in that case the mask might not be that useful since the user can just specify the correct opsFlag without the masked value in the first place)?

Copy link
Member Author

Choose a reason for hiding this comment

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

opsFlagMask is applied to the flags identified by NSS, then the opsFlag are added to the remaining operation. This test show that the opsFlag are added to the resulting list. (I will change all opsFlag* is opFlags*)

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, does it mean the opsFlag param overrides opsFlagMask? Does it work that way too with --keyOpFlagsOn and --keyOpFlagsOff in certutil? We probably should document this behavior.

-v

- name: Gather artifacts
if: always()
run: |
tests/bin/ds-artifacts-save.sh --output=/tmp/artifacts/pki ds
tests/bin/pki-artifacts-save.sh pki
continue-on-error: true

- name: Remove CA
run: docker exec pki pkidestroy -i pki-tomcat -s CA -v

- name: Remove SoftHSM token
run: |
docker exec pki ls -laR /var/lib/softhsm/tokens
docker exec pki runuser -u pkiuser -- softhsm2-util --delete-token --token HSM

- name: Upload artifacts
if: always()
uses: actions/upload-artifact@v3
with:
name: ca-hsm
path: |
/tmp/artifacts/pki
5 changes: 5 additions & 0 deletions .github/workflows/ca-tests2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,8 @@ jobs:
name: SCEP responder
needs: build
uses: ./.github/workflows/scep-test.yml

hsm-operation-test:
name: CA with HSM and custom operation key flags
needs: build
uses: ./.github/workflows/ca-hsm-operation-test.yml
10 changes: 9 additions & 1 deletion base/common/python/pki/nssdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,9 @@ def create_key(
key_size=None,
key_wrap=False,
curve=None,
ssl_ecdh=False):
ssl_ecdh=False,
ops_flag=None,
ops_flag_mask=None):

cmd = [
'pki',
Expand Down Expand Up @@ -686,6 +688,12 @@ def create_key(
if ssl_ecdh:
cmd.append('--ssl-ecdh')

if ops_flag:
cmd.extend(['--ops-flag', ops_flag])

if ops_flag_mask:
cmd.extend(['--ops-flag-mask', ops_flag_mask])

if logger.isEnabledFor(logging.DEBUG):
cmd.append('--debug')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import java.util.Map;
import java.util.StringTokenizer;
import java.util.Vector;
import java.util.stream.Stream;

import javax.crypto.BadPaddingException;
import javax.crypto.SecretKeyFactory;
Expand Down Expand Up @@ -461,6 +462,10 @@ public static KeyPair generateRSAKeyPair(
keygen.extractablePairs(extractable);
}

String usageList = usages != null ? String.join(",", Stream.of(usages).map(org.mozilla.jss.crypto.KeyPairGeneratorSpi.Usage::name).toArray(String[]::new)) : "";
logger.debug("CryptoUtil: generateRSAKeyPair with key usage {}", usageList);
String usageMaskList = usagesMask != null ? String.join(",", Stream.of(usagesMask).map(org.mozilla.jss.crypto.KeyPairGeneratorSpi.Usage::name).toArray(String[]::new)) : "";
logger.debug("CryptoUtil: generateRSAKeyPair with key usage mask {}", usageMaskList);
keygen.setKeyPairUsages(usages, usagesMask);

logger.debug("CryptoUtil: - key size: " + keySize);
Expand Down Expand Up @@ -561,6 +566,10 @@ public static KeyPair generateECCKeyPair(
keygen.extractablePairs(extractable);
}

String usageList = usages != null ? String.join(",", Stream.of(usages).map(org.mozilla.jss.crypto.KeyPairGeneratorSpi.Usage::name).toArray(String[]::new)) : "";
logger.debug("CryptoUtil: generateRSAKeyPair with key usage {}", usageList);
String usageMaskList = usagesMask != null ? String.join(",", Stream.of(usagesMask).map(org.mozilla.jss.crypto.KeyPairGeneratorSpi.Usage::name).toArray(String[]::new)) : "";
logger.debug("CryptoUtil: generateRSAKeyPair with key usage mask {}", usageMaskList);
keygen.setKeyPairUsages(usages, usagesMask);

keygen.initialize(curveCode);
Expand Down Expand Up @@ -3047,6 +3056,12 @@ public static byte[] getDesParity(byte[] key) throws Exception {

return desKey;
}

public static KeyPairGeneratorSpi.Usage[] generateUsage(String usage) {
return Arrays.stream(usage.toUpperCase().split(",")).map(String::trim)
.map(KeyPairGeneratorSpi.Usage::valueOf).toArray(KeyPairGeneratorSpi.Usage[]::new);

}
}

// START ENABLE_ECC
Expand Down
16 changes: 16 additions & 0 deletions base/server/etc/default.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ pki_audit_signing_key_size=2048
pki_audit_signing_key_type=rsa
pki_audit_signing_signing_algorithm=SHA256withRSA
pki_audit_signing_token=
pki_audit_signing_opsFlag=
pki_audit_signing_opsFlagMask=

pki_backup_keys=False
pki_backup_file=
Expand Down Expand Up @@ -123,6 +125,8 @@ pki_sslserver_key_type=%(pki_ssl_server_key_type)s
pki_sslserver_nickname=%(pki_ssl_server_nickname)s
pki_sslserver_subject_dn=%(pki_ssl_server_subject_dn)s
pki_sslserver_token=%(pki_ssl_server_token)s
pki_sslserver_opsFlag=
pki_sslserver_opsFlagMask=

pki_self_signed_nickname=temp %(pki_sslserver_nickname)s
pki_self_signed_token=
Expand All @@ -134,6 +138,8 @@ pki_subsystem_key_type=rsa
pki_subsystem_nickname=subsystemCert cert-%(pki_instance_name)s
pki_subsystem_subject_dn=cn=Subsystem Certificate,ou=%(pki_instance_name)s,o=%(pki_security_domain_name)s
pki_subsystem_token=
pki_subsystem_opsFlag=
pki_subsystem_opsFlagMask=

#Set this if we want to use PSS signing when RSA is specified
pki_use_pss_rsa_signing_algorithm=False
Expand Down Expand Up @@ -270,6 +276,8 @@ pki_ca_signing_serial_number=1
pki_ca_signing_signing_algorithm=SHA256withRSA
pki_ca_signing_subject_dn=cn=CA Signing Certificate,ou=%(pki_instance_name)s,o=%(pki_security_domain_name)s
pki_ca_signing_token=
pki_ca_signing_opsFlag=
pki_ca_signing_opsFlagMask=

# DEPRECATED: Use 'pki_ca_signing_csr_path' instead.
pki_external_csr_path=
Expand Down Expand Up @@ -305,6 +313,8 @@ pki_ocsp_signing_nickname=ocspSigningCert cert-%(pki_instance_name)s CA
pki_ocsp_signing_signing_algorithm=SHA256withRSA
pki_ocsp_signing_subject_dn=cn=CA OCSP Signing Certificate,ou=%(pki_instance_name)s,o=%(pki_security_domain_name)s
pki_ocsp_signing_token=
pki_ocsp_signing_opsFlag=
pki_ocsp_signing_opsFlagMask=

pki_profiles_in_ldap=False
pki_random_serial_numbers_enable=False
Expand Down Expand Up @@ -411,6 +421,8 @@ pki_storage_nickname=storageCert cert-%(pki_instance_name)s KRA
pki_storage_signing_algorithm=SHA256withRSA
pki_storage_subject_dn=cn=DRM Storage Certificate,ou=%(pki_instance_name)s,o=%(pki_security_domain_name)s
pki_storage_token=
pki_storage_opsFlag=
pki_storage_opsFlagMask=

pki_transport_key_algorithm=SHA256withRSA
pki_transport_key_size=2048
Expand All @@ -419,6 +431,8 @@ pki_transport_nickname=transportCert cert-%(pki_instance_name)s KRA
pki_transport_signing_algorithm=SHA256withRSA
pki_transport_subject_dn=cn=DRM Transport Certificate,ou=%(pki_instance_name)s,o=%(pki_security_domain_name)s
pki_transport_token=
pki_transport_opsFlag=
pki_transport_opsFlagMask=

pki_admin_email=%(pki_admin_name)s@%(pki_dns_domainname)s
pki_admin_name=%(pki_admin_uid)s
Expand Down Expand Up @@ -504,6 +518,8 @@ pki_ocsp_signing_nickname=ocspSigningCert cert-%(pki_instance_name)s OCSP
pki_ocsp_signing_signing_algorithm=SHA256withRSA
pki_ocsp_signing_subject_dn=cn=OCSP Signing Certificate,ou=%(pki_instance_name)s,o=%(pki_security_domain_name)s
pki_ocsp_signing_token=
pki_ocsp_signing_opsFlag=
pki_ocsp_signing_opsFlagMask=

pki_admin_email=%(pki_admin_name)s@%(pki_dns_domainname)s
pki_admin_name=%(pki_admin_uid)s
Expand Down
9 changes: 7 additions & 2 deletions base/server/python/pki/server/deployment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2839,6 +2839,8 @@ def create_system_cert_info(self, subsystem, tag):
system_cert.nickname = self.mdict['pki_%s_nickname' % cert_id]
system_cert.subjectDN = self.mdict['pki_%s_subject_dn' % cert_id]
system_cert.token = self.mdict['pki_%s_token' % cert_id]
system_cert.ops_flag = self.mdict['pki_%s_opsFlag' % cert_id]
system_cert.ops_flag_mask = self.mdict['pki_%s_opsFlagMask' % cert_id]

if not system_cert.token:
if config.str2bool(self.mdict['pki_hsm_enable']):
Expand Down Expand Up @@ -2971,7 +2973,8 @@ def create_cert_key(self, tag, request):

token = request.systemCert.token
key_type = request.systemCert.keyType

ops_flag = request.systemCert.ops_flag
ops_flag_mask = request.systemCert.ops_flag_mask
key_size = None
key_wrap = False
curve = None
Expand All @@ -2996,7 +2999,9 @@ def create_cert_key(self, tag, request):
key_size=key_size,
key_wrap=key_wrap,
curve=curve,
ssl_ecdh=ssl_ecdh)
ssl_ecdh=ssl_ecdh,
ops_flag=ops_flag,
ops_flag_mask=ops_flag_mask)
finally:
nssdb.close()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ public void createOptions() {
option.setArgName("boolean");
options.addOption(option);

option = new Option(null, "ops-flag", true, "Custom flags for key usage (empty for HSM default)");
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thinking, would --op-flags / op-flags-mask be more appropriate since these options contains a list of flags?

It's probably not necessary to mention (empty for HSM default) since the list will be empty by default for all types of tokens, not just HSM.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about the wording but your clarification seems reasonable so I'll update.

option.setArgName("usage list");
options.addOption(option);

option = new Option(null, "ops-flag-mask", true, "Custom flags mask for key usage (empty for HSM default)");
option.setArgName("usage list");
options.addOption(option);

option = new Option(null, "output-format", true, "Output format: text (default), json.");
option.setArgName("format");
options.addOption(option);
Expand Down Expand Up @@ -129,6 +137,9 @@ public void execute(CommandLine cmd) throws Exception {
extractable = Boolean.valueOf(extractableStr);
}

String opsFlag = cmd.getOptionValue("ops-flag");
String opsFlagMask = cmd.getOptionValue("ops-flag-mask");

MainCLI mainCLI = (MainCLI) getRoot();
mainCLI.init();

Expand All @@ -141,11 +152,21 @@ public void execute(CommandLine cmd) throws Exception {

logger.info("Creating " + keyType + " in token " + tokenName);

if ("RSA".equalsIgnoreCase(keyType)) {
Usage[] usages = null;
Usage[] usagesMask = null;

if ("RSA".equalsIgnoreCase(keyType)) {
if (keySize == null) keySize = "2048";
Usage[] usages = keyWrap ? CryptoUtil.RSA_KEYPAIR_USAGES : null;
Usage[] usagesMask = keyWrap ? CryptoUtil.RSA_KEYPAIR_USAGES_MASK : null;
if (opsFlag != null && !opsFlag.isEmpty()) {
usages = CryptoUtil.generateUsage(opsFlag);
} else {
usages = keyWrap ? CryptoUtil.RSA_KEYPAIR_USAGES : null;
}
if (opsFlagMask != null && !opsFlagMask.isEmpty()) {
usagesMask = CryptoUtil.generateUsage(opsFlagMask);
} else {
usagesMask = keyWrap ? CryptoUtil.RSA_KEYPAIR_USAGES_MASK : null;
}

KeyPair keyPair = nssdb.createRSAKeyPair(
token,
Expand All @@ -164,9 +185,14 @@ public void execute(CommandLine cmd) throws Exception {
keyInfo.setAlgorithm(privateKey.getAlgorithm());

} else if ("EC".equalsIgnoreCase(keyType)) {

Usage[] usages = null;
Usage[] usagesMask = sslECDH ? CryptoUtil.ECDH_USAGES_MASK : CryptoUtil.ECDHE_USAGES_MASK;
if (opsFlag != null && !opsFlag.isEmpty()) {
usages = CryptoUtil.generateUsage(opsFlagMask);
}
if (opsFlagMask != null && !opsFlagMask.isEmpty()) {
usagesMask = CryptoUtil.generateUsage(opsFlagMask);
} else {
usagesMask = sslECDH ? CryptoUtil.ECDH_USAGES_MASK : CryptoUtil.ECDHE_USAGES_MASK;
}

KeyPair keyPair = nssdb.createECKeyPair(
token,
Expand Down
10 changes: 10 additions & 0 deletions docs/changes/v11.5.0/Server-Changes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,13 @@ To install a new KRA with the legacy sequential serial numbers specify the follo

* `pki_key_id_generator=legacy`
* `pki_request_id_generator=legacy`


== Add pki_<cert_id>_opsFlag and pki_<cert_id>_opsFlagMask parameters ==

Two new parameters are added to pkispawn configuration for setting the key flags in HSM.
The new parameters are available for all certificates created during the subsystem installation
and their value is a comma separated list of the following flags: encrypt, decrypt, sign,
sign_recover, verify, verify_recover, wrap, unwrap and derive. The first parameter add flags to
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest adding back quotes for the actual values that can be specified in these params, e.g. encrypt, decrypt.

the list identified by underneath module while the second remove them.
Default values are empty lists to get the HSM default key flags.
Loading