Skip to content
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

OpenSSL::BN is used both as a signed and unsigned value, binary encoding should support both #431

Closed
rickmark opened this issue Mar 30, 2021 · 4 comments

Comments

@rickmark
Copy link
Contributor

A high bit when binary encoding a BN can signal the number is negative. There should be a way when encoding / decoding OpenSSL::BN to indicate if the encoded value is signed or unsigned.

Parsing a unsigned : OpenSSL::BN.new('FF', 16, signed: false) -> .to_i => 255
Parsing a signed : OpenSSL::BN.new('FF', 16, signed: true) -> .negative? => true
Omitting the value for signed: maintains compatibility

Similar parameters may be needed for #to_s as if it is a unsigned with the most significant bit set occurring on the byte boundary it needs an additional "\x00" prefix to disambiguate its sign

@rhenium
Copy link
Member

rhenium commented Mar 31, 2021

What is the use case?

For hexadecimal number, BN_hex2bn() takes a string prefixed by '-': OpenSSL::BN.new("-ff", 16) will be -255.

@rickmark
Copy link
Contributor Author

rickmark commented Mar 31, 2021 via email

@rhenium
Copy link
Member

rhenium commented Apr 2, 2021

OpenSSL::BN.new(octet_string, 2) will always parse octet_string as a positive number encoded in big-endian. So, if the positive number is the only concern, it should be good already.

I don't see the format (two's complement, variable length) is widely used outside ASN.1 BER's context. For decoding such encoding I'd suggest prepending a proper header and using the ASN.1 routine:

bin = "\xff\x00"
der = [2, bin.bytesize, bin].pack("Cwa*")
OpenSSL::ASN1.decode(der).value #=> -256

@rickmark
Copy link
Contributor Author

rickmark commented Apr 2, 2021

Yeah - the unfortunate bit is bad conformance has lead some implementations to place a 32 octet big int with a high most significant bit directly into the integer type - causing it to be mis-interpreted as a negative value. For ECDSA signatures you know that both values are in fact positive so being able to hint to decode them as unsigned would help....

I know its awful to build features around non-conformance though :-/

@rickmark rickmark closed this as completed Apr 2, 2021
rhenium added a commit that referenced this issue Apr 2, 2021
Clarify that BN.new(str, 2) and bn.to_s(2) handles binary string in
big-endian, and the sign of the bignum is ignored.

Reference: #431
rhenium added a commit to rhenium/ruby that referenced this issue Jul 18, 2021
… #to_s

Clarify that BN.new(str, 2) and bn.to_s(2) handles binary string in
big-endian, and the sign of the bignum is ignored.

Reference: ruby/openssl#431

ruby/openssl@6fae2bd612
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants