-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Exposing KeyObject fields vs. native JWK support #26854
Comments
I think that Constructing a PEM from a passed JWK is not hard, it's just annoying and it's something that is either error prone when done by each implementer or requires additional dependencies in the form of asn1.js (that in turn require ~7 more in the form of browser-ready version of assert, BN (doesn't use the built-in when available), and even that doesn't come with the actual asn.1 type definition, so again, prone to errors. In addition to JWK support I could use .fields because i'd otherwise have to run |
For the record here are the bare bone JWK key types and their attributes. Props with
|
Just realized the use case i have for implementing both. In addition to JWK<>PEM two-way support we have the need to use x509 certificates in OpenID Connect/OAuth2.0 for binding tokens to mutual-TLS client certificates. We have a client established property on the server to check for in the certificate, either DN or SAN values. We also want to do x5c (chain) validation. In those cases i'd pass the certificate(s) to So I'd like both. |
/cc @omsmith |
While JWK support could be cool, I don't think it precludes implementing fields. JWK's kty field is defined by this registry: https://www.iana.org/assignments/jose/jose.xhtml#web-key-types So node would be stuck making a decision about key types that aren't currently specified by JOSE, while the fields may still be valuable (including for an individual looking to make a non-standard JWK) |
cc @nodejs/security-wg |
The
I completely agree with you. I don't think we should support exporting those keys. |
Understood, thank you for the clarification. |
Another thing to consider are the names of the fields we are talking about. There are not really standardized names for all of the fields. JWK uses short keys that are close to the mathematical description of the properties. OpenSSL follows this scheme roughly, but some fields still have different names. RFCs for encoding these keys usually use different names that are easier to read: We had a similar discussion around the scrypt API which originally used parameters |
I agree with a verbose naming scheme. |
This makes sense, existing create APIs only create from ASN.1, but once
Its not clear to me that "native jwk support" is necessarily an alternative to JWK (EDIT: I meant to ".fields"), did you intend to imply that? If the fields structure was identical to JWK, and Just to clarify whether I understand this correctly, JOSE is https://jose.readthedocs.io/en/latest/, and has spun off sub-projects, JWK for key representation, JWS for signing, JWE for encryption. And the relation of JOSE to the WebCrypto API is... what? It looks like WebCrypto implements lower-level cryptographic primitives than JWS and JWE, but that it supports JWK key import/export. Do I understand correctly? Many node APIs predate the emerging the "standard" APIs, but there is a general drift in node towards exposing the standard APIs as appropriate (an individuals have the energy and time to implement). URL and Performance Timeline for example. I'd bet heavily that in 5 years we have a fetch API, some support for async streams, etc. The evolution is a bit jarring, but leaving aside questions of small-core, and whther we should do this, I have a question: Would it be possible to expose the WebCrypto API as the primary crypto API? Does it define sufficient primitives, of a wide enough scope, or is its scope of application too limited? For example, I think it has only one-shot symetric encrypt/decrypt APIs, which makes it insufficiently general for non-browser use. Regardless of future node API directions, I think we all agree on the basic principle: it's our intention that the crypto API evolve to provide sufficient functionality that web-standard (and other) crypto APIs can be implemented using WRT. naming: ideally, we would have long form "readable" names, where the names come from a recognizable standard (so we don't have to invent them). JWK is a recognizable standard, and its tempting to use its naming, but it looks problematic:
The other options are:
We'll live with this decision for a long time, so what I'd like to see if its possible (and I know this is a lot of work), is a table of equivalent names for the .field of all key types: JWK, ASN.1, WebCrypto (if this is a thing, it might not be), and OpenSSL API name. The goal would be to 1) pick the best name, and 2) make sure that .fields contains sufficient raw information that JWK, WebCrypto, and ASN.1 (though we already do this internally with OpenSSL) can be implemented on top of it. The second goal is critical, I assume if that's not achieved then |
Oh, and I've yet to be convinced that any of the raw crypto information should be exposed as numbers. The ASN.1 does not tend to do that. While we all know that some crypto operations are mathematical in nature (but not all, some is just bit operations like shift and XOR, and those aren't best done to numbers), that doesn't mean that users of the broken-out data are going to do math with them. They are mostly, effectively, opaque data that is shoved in and out of various structures, or passed to low-level crypto libs for use (and they may do math with them, but they didn't need or want BigInts in order to do that). For this reason, and that most languages don't have arbitrary precision integers, key parts are usually treated as opaque data. I think JWK did the right thing here. |
Yes. JOSE (JWA) restricts / profiles the primitives to form With the exception of node/openssl unsupported RSA-OAEP-256 you can see all the algorithm implementations using node's crypto here. For clarity here are the final JOSE-family specs
I'm not including JWT because it's just JWS profile using compact serialization and specific claim definition (type and processing), i'm also omitting JWS format extensions Options 1 and 3 sound good to me,
But frankly, since @tniessen clarified const jwk = {
kty: 'EC',
crv: 'P-256',
x: 'gskvlmd8hChm_hxtH4oyDC2rizV1jwK4exTn3icBxu8',
y: '0El8NqTyybjkEJ2grVUDK1BdPo49OF1pmCJCD92_h-Y'
};
const keyObject = createPublicKey(jwk);
keyObject.export({ format: 'jwk' });
// jwk members equal to exported members And we could forget about naming because JWK cut out the work for us and we're just supporting another key representation, not extending the API with new methods and properties per se. |
It is certainly possible to implement WebCrypto as part of node core, but I still believe that it is not a sufficient standard for non-browser applications. It has no streaming interface and there is a lot of overhead in each operation (e.g. by using
Locally, I have been using RFC names when possible. The fields are sufficient to be used to implement JWK on top of them, but if that is their primary use, we might be better off supporting JWK directly.
So far, I sticked to the data types OpenSSL is using in its public APIs, and those are mostly BIGNUMs. If we decide to use strings instead, then |
@panva I think that maybe eventually
I'm puzzled by this. For two reasons. 1, one of your primary use-cases of JWK import/export is met by I do think a X.509 decode (and, ultimately, support for the range of OpenSSL APIs for import/export/create of cert requests, certs, PKCS7 and CMS, PFX, etc.) would be desierable. X.509 decode is relatively easy, the code is there, I might do it soon, but TLS1.3 is hight priority ATM.
Can you describe what use you think users would make of BigInts coming out of The only uses I can think of is if they want to experiment with implementing, for example, cryptographic primitives (for example, ECMQV) that are not supported by Node.js. Or perhaps they are cryptography/math students, and want to directly implement some of the existing algorithms to explore how they are done. I've done this before, but with ruby arbitrary precision numbers, when I was trying to get a deeper understanding of RSA. Both those use-cases are supported by outputting strings, though with a loss of efficiency since conversion to a JS numeric type is required. That conversion cost would be dwarfed by the actual overhead of most crypto algorithms, I think. The most common use of the We could have our cake and eat it too by having |
I don't think its the primary use-case, its just the primary use-case of @panva who is the most helpful and active potential user ATM. ;-) Also, we won't be able to support fields of anything not covered by JWK (such as DSA or DH keys). I think the .fields is the first useful layer. Later, maybe, adding direct JWK support might be useful (or maybe everyone wanting it will want to do JWE and JWS, so will be using a JOSE module and won't care if the import/export is directly implemented by |
Not import, no, having .fields is fine for exporting, but doesn't solve import and the whole roundtrip on its own. See @tniessen's first message:
I can tell you there is use for getting contents of a cert. But let's table that for now, i agree with you that I am totally behind building in layers, so if the idea is
💯% fine
Primary use-case aside (haven't seen a cryptography/math student make a node feature request building on top of KeyObject, i think they're all using python ), non-JWK defined key types are probably the best argument for starting with the fields layer. |
Just re-read. JWK uses strings for keys, but the strings are b64url encoded? That is pretty specific... I'm about +0 on JWK in/out support ATM, but still +lots on a .fields, though I'm starting to wonder, maybe it should be An equivalent |
Much needed, yes. Today we're mostly working with Buffer instances that we get from asn.1 parser implementations.
Defining |
What are possible use cases for
Or an option to keep it extensible:
base64url is one of the many oddities of WebCrypto / JWK. The conversion between base64 and base64url is relatively easy to do outside of node core, so unless we decide to implement JWK, we should probably not use it (or promote its use). |
Related problem: nodejs/webcrypto#17 |
Like JWK, any crypto scheme, algorithm, protocol, or format that is not currently supported needs access to the raw key data. I don't know the current list of "what node or openssl doesn't support", but it has to be a non-empty set!
👍
Right, forget I mentioned it, it sounds too fringe, and easily implemented in JWK. Also, the point that JWK has a number of features that node doesn't support doesn't, in my mind, exclude us just doing the parts we can, but I do think just exposing the fields and letting WebCrypto (and any other crypto extensions there might be out there) have access is a way for Node.js to just focus on mechanism, and let npm packages use those mechanisms as they need. JWK could always be added into node later if it was useful. |
Fixes: nodejs#678 Refs: nodejs#26854 Signed-off-by: James M Snell <[email protected]>
Fixes: #678 Refs: #26854 Signed-off-by: James M Snell <[email protected]> PR-URL: #35093 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Fixes: nodejs#678 Refs: nodejs#26854 Signed-off-by: James M Snell <[email protected]> PR-URL: nodejs#35093 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Rich Trott <[email protected]>
With JWK export/import now being part of the |
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
1 similar comment
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document. |
There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment. For more information on how the project manages feature requests, please consult the feature request management document. |
Now that node has a decent API to deal with cryptographic keys, users have requested features to access parts of keys, e.g. the modulus of an RSA key or the curve name of an EC key. @sam-github and I came up with the idea to add a
.fields
property toKeyObject
which exposes those parts. Other users have encouraged this approach.I am currently working on a PR for that and the basics are working nicely:
One of the reasons users have requested this feature is to be able to implement JWK on top of the native crypto module without having to tap into OpenSSL. However, this would still only make that work in one direction (
KeyObject
→ JWK), to create aKeyObject
from JWK, a different API would be necessary to construct keys from their fields.Another solution would be to natively support JWK. I am not an expert when it comes to JWK, but it shouldn't be difficult to implement as long as we don't need to include algorithm information in the key as WebCrypto does. This approach would extend
create***Key
andKeyObject.export
with support for JWK.If JWK support is not the only reason to access parts of the key, it might still make sense to implement
.fields
since JWK was designed for storing keys, not for interacting with its components. For example,.fields
could make use of ES BigInts whereas JWK encodes everything as strings.So as I see it, there are four options: Implement both, implement one and not the other, or implement none of it. What do you think?
cc @nodejs/crypto @panva @mscdex
The text was updated successfully, but these errors were encountered: