-
-
Notifications
You must be signed in to change notification settings - Fork 187
Tiny ps improvement #594
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
base: master
Are you sure you want to change the base?
Tiny ps improvement #594
Conversation
TuxSH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also please rephrase your commit's title? Thanks
| u8 exponent[0x100]; | ||
| u32 rsa_bitsize;//The signature byte size is rsa_bitsize>>3. | ||
| u32 unk;//Normally zero? | ||
| u8 is_full_exponent; // 1 if big endian full exponent, 0 if small 4 byte little endian exponent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it likely is_private_key? The "small" exponent, is it 0x10001 perchance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name comes from 3ds_ps; 0x10001 happens to be the only public exponent used officially, and it is passed as a u32, but IMO it would be wrong to enforce a public-small/private-big grouping for exponents, as there's no actual constraint for what concerns the RSA engine, and it's purely a conventional distinction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x10001 is the most used RSA public exponent ever. No one is going to use small exponents for private exponent as it would be trivially factored.
Does the field has any impact on p9?
And "Tiny ps improvement" isn't an acceptable commit name, Nikki.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realistically speaking no one is going to use this API anyway, as it's limited in use case with very specific purposes; my argument for keeping things as generic as possible, and the reason why I made this PR, is documentation; as a reverse engineer myself I think libctru is a valuable resource even outside programming, and enforcing such convention on the structure would pass off the (incorrect) message that the engine makes distinctions between exponent types, when the distinction is merely about encoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realistically speaking no one is going to use this API anyway
This is not an excuse to commit technical debt
The problem with is_full_exponent is that it is much less clear than is_private_key for a user wanting to use this API. It also encourages misuse of the API, as 4-byte private keys would be very trivially bruteforced.
as a reverse engineer myself I think libctru is a valuable resource even outside programming, and enforcing such convention on the structure would pass off the (incorrect) message that the engine makes distinctions between exponent types
So am I. You could rename the field as advised then explain what it means in the doxygen comment (like you already did).
Matches the structure described in 3ds_ps.