search

adamcin / httpsig-java

Implementation of HTTP Signature Authentication in Java
http://adamcin.net/httpsig-java
The Unlicense
16 stars 10 forks source link

Two questions about ssh-jce #8

Open vonagam opened 7 years ago

vonagam commented 7 years ago

1) KeyFormat.SSH_RSA has Algorithm.SSH_RSA as the first candidate for algorithm, but it is not supported by node-http-signature:

https://github.com/adamcin/httpsig-java/blob/2b1fc5601bbc17ee4b953c0063423fffe2ccd0bb/ssh-jce/src/main/java/net/adamcin/httpsig/ssh/jce/KeyFormat.java#L53

https://github.com/joyent/node-http-signature/blob/529441d9d04a8ecb296a2a152929332526344673/lib/utils.js#L13

For this to work without algorithms rotations, i do the following after a signer creation:

Collection<Algorithm> algorithms = Collections.singletonList(Algorithm.RSA_SHA256);
Challenge challenge = new Challenge("<preemptive>", Constants.DEFAULT_HEADERS, algorithms);
signer.rotateKeys(challenge);

Is this how it is supposed to be used? Can't there be more "out of the box experience" for this?

2) Why SSHKey here must have public key?

https://github.com/adamcin/httpsig-java/blob/2b1fc5601bbc17ee4b953c0063423fffe2ccd0bb/ssh-jce/src/main/java/net/adamcin/httpsig/ssh/jce/SSHKey.java#L64

It seems that presence of methods like canVerify and checks in verify method itself should prevent invalid use. Just curious. Use case is that a key used only for signing, not verifying.

adamcin commented 7 years ago
  1. The SSHKey piece is what I was working on independently before I refactored to support the Joyent spec, so the default primacy of the ssh-rsa algorithm is an artifact of that. I haven't had much feedback from users outside of my own tooling ecosystem, so the ergonomics of this library have not really followed the node-http-signature implementation very closely. I'm happy to keep incorporating your feedback as long as you are actively using this library. I can probably safely change the default signature algorithm for ssh keys to be rsa-256 or rsa-512 to match whatever is most commonly supported to reduce the need for rotation requests.
  2. I think the reason for requiring the public key in the constructor is that it is necessary to compute the key fingerprint. Looking at it now it doesn't make much sense to have that error message there.
vonagam commented 7 years ago
  1. You can also add another two values to enum. Like DSA_SHA and RSA_SHA which would have only corresponding algorithms.