LibCrypto: Make SECPxxxr1Signature carry the scalar size

Our `UnsignedBigInteger` implementation cannot handle numbers whose
size is not a multiple of 4. For this reason we need to carry the real
size around for P-521 support.
This commit is contained in:
devgianlu 2025-01-26 20:59:06 +01:00 committed by Ali Mohammad Pur
parent fd2014f5c2
commit f2e530ec14
Notes: github-actions[bot] 2025-01-27 11:26:14 +00:00
4 changed files with 39 additions and 10 deletions

View file

@ -15,6 +15,7 @@
#include <AK/StringView.h>
#include <AK/UFixedBigInt.h>
#include <AK/UFixedBigIntDivision.h>
#include <LibCrypto/ASN1/Constants.h>
#include <LibCrypto/ASN1/DER.h>
#include <LibCrypto/Curves/EllipticCurve.h>
#include <LibCrypto/OpenSSL.h>
@ -98,14 +99,40 @@ struct SECPxxxr1Point {
struct SECPxxxr1Signature {
UnsignedBigInteger r;
UnsignedBigInteger s;
size_t size;
static ErrorOr<SECPxxxr1Signature> from_asn(ReadonlyBytes signature, Vector<StringView> current_scope)
static ErrorOr<SECPxxxr1Signature> from_asn(Span<int const> curve_oid, ReadonlyBytes signature, Vector<StringView> current_scope)
{
ASN1::Decoder decoder(signature);
ENTER_TYPED_SCOPE(Sequence, "SECPxxxr1Signature");
READ_OBJECT(Integer, UnsignedBigInteger, r_big_int);
READ_OBJECT(Integer, UnsignedBigInteger, s_big_int);
return SECPxxxr1Signature { r_big_int, s_big_int };
size_t scalar_size;
if (curve_oid == ASN1::secp256r1_oid) {
scalar_size = ceil_div(256, 8);
} else if (curve_oid == ASN1::secp384r1_oid) {
scalar_size = ceil_div(384, 8);
} else if (curve_oid == ASN1::secp521r1_oid) {
scalar_size = ceil_div(521, 8);
} else {
return Error::from_string_literal("Unknown SECPxxxr1 curve");
}
if (r_big_int.byte_length() < scalar_size || s_big_int.byte_length() < scalar_size)
return Error::from_string_literal("Invalid SECPxxxr1 signature");
return SECPxxxr1Signature { r_big_int, s_big_int, scalar_size };
}
ErrorOr<ByteBuffer> r_bytes() const
{
return SECPxxxr1Point::scalar_to_bytes(r, size);
}
ErrorOr<ByteBuffer> s_bytes() const
{
return SECPxxxr1Point::scalar_to_bytes(s, size);
}
ErrorOr<ByteBuffer> to_asn()
@ -458,7 +485,7 @@ public:
return sign_scalar(hash, private_key);
}
return SECPxxxr1Signature { storage_type_to_unsigned_big_integer(r), storage_type_to_unsigned_big_integer(s) };
return SECPxxxr1Signature { storage_type_to_unsigned_big_integer(r), storage_type_to_unsigned_big_integer(s), KEY_BYTE_SIZE };
}
ErrorOr<SECPxxxr1Signature> sign(ReadonlyBytes hash, ReadonlyBytes private_key_bytes)

View file

@ -482,7 +482,7 @@ ssize_t TLSv12::verify_ecdsa_server_key_exchange(ReadonlyBytes server_key_info_b
return (i8)Error::NotUnderstood;
}
auto maybe_signature = Crypto::Curves::SECPxxxr1Signature::from_asn(signature_bytes, {});
auto maybe_signature = Crypto::Curves::SECPxxxr1Signature::from_asn(*public_key.algorithm.ec_parameters, signature_bytes, {});
if (maybe_signature.is_error()) {
dbgln("verify_ecdsa_server_key_exchange failed: Signature is not ASN.1 DER encoded");
return (i8)Error::NotUnderstood;

View file

@ -354,7 +354,7 @@ bool Context::verify_certificate_pair(Certificate const& subject, Certificate co
auto public_point = issuer.public_key.ec.to_secpxxxr1_point();
auto maybe_signature = Crypto::Curves::SECPxxxr1Signature::from_asn(subject.signature_value, {});
auto maybe_signature = Crypto::Curves::SECPxxxr1Signature::from_asn(*issuer.public_key.algorithm.ec_parameters, subject.signature_value, {});
if (maybe_signature.is_error()) {
dbgln("verify_certificate_pair: Signature is not ASN.1 DER encoded");
return false;

View file

@ -3921,12 +3921,14 @@ WebIDL::ExceptionOr<GC::Ref<JS::ArrayBuffer>> ECDSA::sign(AlgorithmParams const&
// 4. Let n be the smallest integer such that n * 8 is greater than the logarithm to base 2 of the order of the base point of the elliptic curve identified by params.
// 5. Convert r to an octet string of length n and append this sequence of bytes to result.
VERIFY(signature.r.byte_length() <= coord_size);
(void)signature.r.export_data(result.span());
auto r_bytes = TRY_OR_THROW_OOM(vm, signature.r_bytes());
VERIFY(r_bytes.size() <= coord_size);
result.overwrite(0, r_bytes.data(), r_bytes.size());
// 6. Convert s to an octet string of length n and append this sequence of bytes to result.
VERIFY(signature.s.byte_length() <= coord_size);
(void)signature.s.export_data(result.span().slice(coord_size));
auto s_bytes = TRY_OR_THROW_OOM(vm, signature.s_bytes());
VERIFY(s_bytes.size() <= coord_size);
result.overwrite(coord_size, s_bytes.data(), s_bytes.size());
} else {
// FIXME: Otherwise, the namedCurve attribute of the [[algorithm]] internal slot of key is a value specified in an applicable specification:
// FIXME: Perform the ECDSA signature steps specified in that specification, passing in M, params and d and resulting in result.
@ -4001,7 +4003,7 @@ WebIDL::ExceptionOr<JS::Value> ECDSA::verify(AlgorithmParams const& params, GC::
auto maybe_result = curve.visit(
[](Empty const&) -> ErrorOr<bool> { return Error::from_string_literal("Failed to create valid crypto instance"); },
[&](auto instance) { return instance.verify_point(M, Q.to_secpxxxr1_point(), ::Crypto::Curves::SECPxxxr1Signature { r, s }); });
[&](auto instance) { return instance.verify_point(M, Q.to_secpxxxr1_point(), ::Crypto::Curves::SECPxxxr1Signature { r, s, half_size }); });
if (maybe_result.is_error()) {
auto error_message = MUST(String::from_utf8(maybe_result.error().string_literal()));