Codecov Report

Merging #142 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
+ Coverage   96.55%   96.57%   +0.01%     
==========================================
  Files          14       14              
  Lines        1075     1079       +4     
==========================================
+ Hits         1038     1042       +4     
  Misses         37       37

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update deea760...40920a5. Read the comment docs.

If multiple backends allow people to verify with private keys, doesn't seem to imply that somebody somewhere has a valid usecase for doing so?

As mpdavis said:

This happens because the private key contains both the public and private key info. The verify method extracts the public key info and happily uses it.

I think it might be appropriate to simply throw a suppressible warning but continue the verification as normal.

I agree that there may be a use case for this: It can certainly be a bit easier to just put in the private key and do verification with it. But I think there may be cases that is not correct, eg. somebody just using (same) private key on both ends of communication channel and thinking they're secure.

That's why I presonally would rather see this chosen explicitly.

I didn't want to just merge it since I think it merits a discussion.

But I think there may be cases that is not correct, eg. somebody just using (same) private key on both ends of communication channel and thinking they're secure.

Well, the recipient is (AIUI) just calculating the public key from the private key, so while it isn't the way RSA intended RSA to be used, it's also not guaranteed to be insecure (aside from securely distributing the private key itself, but this library doesn't deal with that). It's definitely super weird, IMO, but it's not necessarily "wrong", and I don't think it's something we should concern ourselves with preventing outright.

I would be more comfortable with just emitting a warning. @zejn @dumptyd Is that acceptable to you for now? If somebody can demonstrate that this is guaranteed to be insecure, we can always implement this PR at a later date.

Warning seems sensible and OK.

If somebody can demonstrate that this is guaranteed to be insecure, we can always implement this PR at a later date.

I couldn't think of such a scenario.

Is that acceptable to you for now?

Yup, I have no issues with that. Thanks!