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

Add privateKeyPassword option for private key decryption #465

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ signature algorithms enabled at same time.
When signing a xml document you can pass the following options to the `SignedXml` constructor to customize the signature process:

- `privateKey` - **[required]** a `Buffer` or pem encoded `String` containing your private key
- `privateKeyPass` - **[optional]** the password to decrypt the private key
shunkica marked this conversation as resolved.
Show resolved Hide resolved
- `publicCert` - **[optional]** a `Buffer` or pem encoded `String` containing your public key
- `signatureAlgorithm` - **[required]** one of the supported [signature algorithms](#signature-algorithms). Ex: `sign.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"`
- `canonicalizationAlgorithm` - **[required]** one of the supported [canonicalization algorithms](#canonicalization-and-transformation-algorithms). Ex: `sign.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#WithComments"`
Expand Down Expand Up @@ -132,6 +133,7 @@ When verifying a xml document you can pass the following options to the `SignedX

- `publicCert` - **[optional]** your certificate as a string, a string of multiple certs in PEM format, or a Buffer
- `privateKey` - **[optional]** your private key as a string or a Buffer - used for verifying symmetrical signatures (HMAC)
- `privateKeyPass` - **[optional]** the password to decrypt the private key

The certificate that will be used to check the signature will first be determined by calling `this.getCertFromKeyInfo()`, which function you can customize as you see fit. If that returns `null`, then `publicCert` is used. If that is `null`, then `privateKey` is used (for symmetrical signing applications).

Expand Down Expand Up @@ -256,8 +258,9 @@ The `SignedXml` constructor provides an abstraction for sign and verify xml docu

- `idMode` - default `null` - if the value of `wssecurity` is passed it will create/validate id's with the ws-security namespace.
- `idAttribute` - string - default `Id` or `ID` or `id` - the name of the attribute that contains the id of the element
- `privateKey` - string or Buffer - default `null` - the private key to use for signing
- `publicCert` - string or Buffer - default `null` - the public certificate to use for verifying
- `privateKey` - string or Buffer or crypto.KeyObject - the private key to use for signing
- `privateKeyPass` - string - the password to decrypt the private key
- `publicCert` - string or Buffer or crypto.KeyObject - the public certificate to use for verifying
- `signatureAlgorithm` - string - the signature algorithm to use
- `canonicalizationAlgorithm` - string - default `undefined` - the canonicalization algorithm to use
- `inclusiveNamespacesPrefixList` - string - default `null` - a list of namespace prefixes to include during canonicalization
Expand Down
23 changes: 19 additions & 4 deletions src/signed-xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export class SignedXml {
* A {@link Buffer} or pem encoded {@link String} containing your private key
*/
privateKey?: crypto.KeyLike;
privateKeyPass?: string;
publicCert?: crypto.KeyLike;
/**
* One of the supported signature algorithms.
Expand Down Expand Up @@ -264,7 +265,7 @@ export class SignedXml {

const signedInfoCanon = this.getCanonSignedInfoXml(doc);
const signer = this.findSignatureAlgorithm(this.signatureAlgorithm);
const key = this.getCertFromKeyInfo(this.keyInfo) || this.publicCert || this.privateKey;
const key = this.getCertFromKeyInfo(this.keyInfo) || this.publicCert || this.getPrivateKey();
if (key == null) {
throw new Error("KeyInfo or publicCert or privateKey is required to validate signature");
}
Expand Down Expand Up @@ -339,13 +340,14 @@ export class SignedXml {
private calculateSignatureValue(doc: Document, callback?: ErrorFirstCallback<string>) {
const signedInfoCanon = this.getCanonSignedInfoXml(doc);
const signer = this.findSignatureAlgorithm(this.signatureAlgorithm);
if (this.privateKey == null) {
const privateKey = this.getPrivateKey();
if (privateKey === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is null now permissible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getPrivateKey() will never return null.
It could never have been null to begin with. It could only be undefined.

Copy link
Contributor

Choose a reason for hiding this comment

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

That doesn't appear to be true. While the typing system would prevent anything other than those values, not everyone uses Typescript. I see no code that would prevent any value from being used for privateKey.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dont think thats a rabbit hole worth going down.
By that logic this.privateKey == null also does nothing, because i could set privateKey=1, privateKey='' etc.., and that check would not catch it.
You cant reasonably expect the library to work if you set privateKey to null.
In any case an error will be thrown by the crypto library.

throw new Error("Private key is required to compute signature");
}
if (typeof callback === "function") {
signer.getSignature(signedInfoCanon, this.privateKey, callback);
signer.getSignature(signedInfoCanon, privateKey, callback);
} else {
this.signatureValue = signer.getSignature(signedInfoCanon, this.privateKey);
this.signatureValue = signer.getSignature(signedInfoCanon, privateKey);
}
}

Expand Down Expand Up @@ -1124,4 +1126,17 @@ export class SignedXml {
getSignedXml(): string {
return this.signedXml;
}

getPrivateKey(): crypto.KeyLike | undefined {
if (
this.privateKeyPass &&
(this.privateKey instanceof Buffer || typeof this.privateKey === "string")
) {
return crypto.createPrivateKey({
key: this.privateKey,
passphrase: this.privateKeyPass,
});
}
return this.privateKey;
}
}
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export interface SignedXmlOptions {
idMode?: "wssecurity";
idAttribute?: string;
privateKey?: crypto.KeyLike;
privateKeyPass?: string;
publicCert?: crypto.KeyLike;
signatureAlgorithm?: SignatureAlgorithmType;
canonicalizationAlgorithm?: CanonicalizationAlgorithmType;
Expand Down
10 changes: 10 additions & 0 deletions test/signature-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1265,4 +1265,14 @@ describe("Signature unit tests", function () {
"MIIDZ",
);
});

it("can decrypt the private key when privateKeyPass is set", function () {
shunkica marked this conversation as resolved.
Show resolved Hide resolved
const xml = "<root><x /></root>";
const sig = new SignedXml();
sig.privateKey = fs.readFileSync("./test/static/client_encrypted.pem");
sig.privateKeyPass = "password";
sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#";
sig.signatureAlgorithm = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
expect(() => sig.computeSignature(xml)).to.not.throw();
});
});
18 changes: 18 additions & 0 deletions test/static/client_encrypted.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-----BEGIN ENCRYPTED PRIVATE KEY-----
MIIC3TBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQIVAXXug6NnZkCAggA
MAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAEqBBBomZwMw4b2v6QQbrOXOqhEBIIC
gHaI/eE3LZ7QkEkXUj0pQqIeI5e7CqbXeqWjoICZmkDinzDVb5eLsFbA7iNjZ/On
tSon2uB5s33vgzC5L07lLUjl4Qvz4SN19xfurNpwKbSGYWqm0FfCjg/TK2L7Y7rV
SsA89f/SlqyTIfolsS9p2VxpAuboCpsHUH+c2YAKBUSK9bXjD8ncDEpVgZZXsUzK
o8Re2qwqwb1CyS0jS5pwlGjFXyLIf3km6apTOIifNwsvsqnZEgJjWSZMlQ8ESQMI
0J1wSPt9iU9B12hdfhr5sX9cIYqelP+Pa++tgWEOFWWOnvkD1vKhx4CoSMsgQs3u
O630F2iGaraAb254SSubVQSk0jjOXsjA85tjU2g8PjE6mjdzQDGJBPwxe0nLIaO6
lJs4Qf08aclspYklbZ0Tu82xtAJ0MxRch/yXQ6SPVZL6wCAnwpQuzXV/CdB4dZ2M
xnjs3kWL/c/gTaIhjaOuTyeG32B9/j+wo1fc++wzXnMgBl1LIpgXQpOQBkZ3PjgE
/sjILPB9VHzigw0LbrT71E3jIoOHbQXh/F20wBDCLJzNNYBZARcK8wI2vQI7/glQ
+hjoC1PqOrKqejBCGcG3NOe+YDeyKfnXYJNA11psSuKBQ4/X/WaL29MfP8UloMrD
WIZkZVc6D6HdYCuFkeS3e7X5XyChIkBfDWFIbWsq9gVt9IhSJ9W8uDYaqsGQ/uBM
aG1brwk8mIJ0JLzz0FQTUpKVSMCUKyz3NCFxRU7wN7tD4/+pqXctnV8IugmNn/DE
aEgG7O+WW+d0Yqm0SIF++/Za6q/9N0GNvcywRu9tgP/5N2pTIygzJQwxuYmr61nU
YD0SBzeR8PoVot1DHQqiKI8=
-----END ENCRYPTED PRIVATE KEY-----
Loading