Skip to content

Fix broken tests and only put certificates in the X509Certificate element#1

Merged
cjbarth merged 10 commits intocjbarth:key-infofrom
shunkica:key-info
Jun 15, 2023
Merged

Fix broken tests and only put certificates in the X509Certificate element#1
cjbarth merged 10 commits intocjbarth:key-infofrom
shunkica:key-info

Conversation

@shunkica
Copy link

This will fix the tests which are broken, but I still think that the algorithms should be an instance member, as opposed to static.

@shunkica
Copy link
Author

shunkica commented Jun 15, 2023

It still seems there is some kind of race condition or scoping issue regarding the static SignatureAlgorithm member when running the tests. Sometimes they pass sometimes they fail, it is arbitrary.

When they fail, the value of SignedXml.SignatureAlgorithms is {}, even though it is never set to an empty object.

@shunkica
Copy link
Author

Making SignatureAlgorithms, HashAlgorithms and CanonicalizationAlgorithms instance members of SignedXml, made sense, and it also had the side effect of fixing the issues with the tests.

@shunkica
Copy link
Author

Since the X509Certificate elements should only contain X509 certificates, there is no point in putting anything other than the keys starting with BEGIN CERTIFICATE (and it is also unsafe).
Public keys like RSA, DSA etc should not go into the X509Certificate element. They should go in their respective elements inside the KeyValue element. If you want we can add support for that too.

With this commit all the tests are passing.

@shunkica shunkica changed the title Fix broken tests Fix broken tests and only put certificates in the X509Certificate element Jun 15, 2023
@shunkica
Copy link
Author

Here is a theoretical untested function for extracting public keys and their respective types:

var crypto = require("crypto");

const EXTRACT_PUBLIC_KEYS = new RegExp(
 "-----BEGIN [A-Z\x20]*PUBLIC[A-Z\x20]*-----([^-]*)-----END [A-Z\x20]*-----",
    "g"
);

function getPublicKeysFromPem(pem) {
  const publicKeys = [];
  let match;

  while ((match = EXTRACT_PUBLIC_KEYS.exec(pem)) !== null) {
    const key = crypto.createPublicKey(match[0]);
    if (!key.asymmetricKeyType) {continue;}
    publicKeys.push([key,key.asymmetricKeyType]);
  }

  return publicKeys;
}

And then in getKeyInfoContent:

  let rsaKeyValue = '';
  let dsaKeyValue = '';
  let ecKeyValue = '';
  let publicKeys = utils.getPublicKeysFromPem(publicCert);
  publicKeys.forEach(([key,type]) => {
    switch (type) {
      case "rsa": {
        const {n: modulus, e: exponent} = key.export({format: 'jwk'});
        rsaKeyValue += `<${prefix}RSAKeyValue><${prefix}Modulus>${modulus}</${prefix}Modulus><${prefix}Exponent>${exponent}</${prefix}Exponent></${prefix}RSAKeyValue>`;
        break;
      }
      case "dsa": {
        const {p: p, q: q, g: g, y: y} = key.export({format: 'jwk'});
        dsaKeyValue += `<${prefix}DSAKeyValue><${prefix}P>${p}</${prefix}P><${prefix}Q>${q}</${prefix}Q><${prefix}G>${g}</${prefix}G><${prefix}Y>${y}</${prefix}Y></${prefix}DSAKeyValue>`;
        break;
      }
      case "ec": {
        const {crv: crv, x: x, y: y} = key.export({format: 'jwk'});
        ecKeyValue += `<${prefix}ECKeyValue><${prefix}NamedCurve URI="urn:oid:${crv}"/><${prefix}PublicKey><${prefix}X>${x}</${prefix}X><${prefix}Y>${y}</${prefix}Y></${prefix}PublicKey></${prefix}ECKeyValue>`;
        break;
      }
    }});

However, this does not fall into the scope of this PR so I do not want to work on this any further right now.

@cjbarth cjbarth merged commit 2340320 into cjbarth:key-info Jun 15, 2023
@shunkica shunkica deleted the key-info branch June 20, 2023 08:44
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