-
Notifications
You must be signed in to change notification settings - Fork 156
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
Towards a more secure API design (for JWT at least) #566
Comments
@yanosz thanks for starting this discussion. The reason why you see Similar rules also apply to most of vert.x public APIs, so you will not see I do agree we need to improve the examples and documentation. We could also update the APIs where base64 are expected to expect a |
Thanks for your reply. Regarding :
I do not yet see how this kind of architecture is needed for keystore-options :-) (however, that does not mean much, because I'm only starting with vert.x). Intuitively, I would expect keystore-configuration to be independent of actual data that is processed - i.e. I can hardly think of a situation in which a passphrase or even a private-key for a key-store option is transmitted with a JSON-based wire-protocol in a vert.x setting |
It's not a requirement for keystore options, it's a requirement because vert.x started as a "polyglot" runtime, so in order to support other languages, say for example, With vert.x 5 we may start removing these restrictions as, actually, we dropped nashorn and have a graalvm alternative that supports pretty much any JVM type. Let me illustrate how this used to work. This is the same code for both Java and JavaScript: JWT jwt = new JWT()
.addJWK(
new JWK(new PubSecKeyOptions()
.setAlgorithm("HS256")
// we're providing a secret as a base64 string
.setBuffer("qnscAdgRlkIhAUPY44oiexBKtQbGY0orf7OV1I50"))); In JavaScript this used to be like this: JWT jwt = new JWT()
.addJWK(
new JWK({
algorithm: "HS256",
// we're providing a secret as a base64 string
buffer: "qnscAdgRlkIhAUPY44oiexBKtQbGY0orf7OV1I50"
}); Options would "magically" be converted to/from JSON which would impair the choice of types in the API. With graalvm, we don't have this "magic" conversion anymore, which means we can have richer types. I think we should start documenting a good refactoring for vert.x 5.0.0 where:
This would fix the ambiguity in some APIs where by the type This change would also require that we relax the codegen rules, which is used to perform API validation and used by all modules and downstream projects. |
I've recently started working on vert.x - however, I was surprised about the JWT-API and its documentation.
I'd suggest re-considering its design in a more usable and secure way, hence addressing usable security from an API perspective.
I'd like to suggest these improvements. These ideas resulted from working with the JWT-API for a few hours - not a review or audit.
Use arrays (byte arrays, char arrays) instead of passwords. Many Java APIs do so (for instance https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/security/KeyStore.html#getInstance(java.io.File,char%5B%5D) - whereas Vert.x does not (for instance: https://vertx.io/docs/apidocs/io/vertx/ext/auth/KeyStoreOptions.html#setPassword-java.lang.String-) . This is a security thing: Arrays are mutable and can be wiped out of memory, whereas Strings cannot. Hence, it's widespread practice to do so.
Document best-practices for using the JWT integration. This may be obvious for an experienced programmer, but dangerous for a not-so crypto savvy one.
Looking at https://vertx.io/docs/vertx-auth-jwt/java/ - secret key material is encoded in java source code. This is broken and insecure. Please provide a best-practice example for using this API in a secure manner - please avoid insecure examples without outlining their danger.
https://vertx.io/docs/vertx-web/java/#_jwt_authentication - encodes the password in plain-text. At least, it says that this is insecure. However, it would be better to provide a secure example that can be used as best practice - i.e., how would one use an authentication provider
Be more explicit about the algorithmic recommendations in https://vertx.io/docs/vertx-auth-jwt/java/. For instance, I do not yet see why one should generate a 2048-Bit key for a HS256 using keytool. 2048-Bit is typically used in asymmetric schemes, whereas HS256 is a symmetric one.
Be explicit on encodings - for instance, by try and error I noticed that
HashingStrategy
requires the salt to be base64-encoded, whereas Java (PBEKeySpec) requires a non-encoded array. This is dangerous. For the compiler, strings are strings. But from a crypto-perspective, the byte-entropy of a base64-encoded string (i.e. printable ascii characters) is 6-bit. In result, confusing encoded and non-encoded data could result in weak passwords, salts and keys.The text was updated successfully, but these errors were encountered: