From 735762747d818563555e13932b8e49f3801a8c7f Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sat, 22 Nov 2025 12:59:42 +0100 Subject: [PATCH 1/5] [ruby/json] Fix the parser to not accept invalid escapes Only `"\/bfnrtu` are valid after a backslash. https://github.com/ruby/json/commit/f7f8f552ed --- ext/json/parser/parser.c | 53 ++++++++++--------- .../fixtures/{pass15.json => fail15.json} | 0 .../fixtures/{pass16.json => fail16.json} | 0 .../fixtures/{pass17.json => fail17.json} | 0 .../fixtures/{pass26.json => fail26.json} | 0 test/json/json_fixtures_test.rb | 2 + test/json/json_parser_test.rb | 4 +- 7 files changed, 31 insertions(+), 28 deletions(-) rename test/json/fixtures/{pass15.json => fail15.json} (100%) rename test/json/fixtures/{pass16.json => fail16.json} (100%) rename test/json/fixtures/{pass17.json => fail17.json} (100%) rename test/json/fixtures/{pass26.json => fail26.json} (100%) diff --git a/ext/json/parser/parser.c b/ext/json/parser/parser.c index cb22648dbc1dfc..2f584a51a932f4 100644 --- a/ext/json/parser/parser.c +++ b/ext/json/parser/parser.c @@ -639,44 +639,43 @@ static inline VALUE json_string_fastpath(JSON_ParserState *state, const char *st static VALUE json_string_unescape(JSON_ParserState *state, const char *string, const char *stringEnd, bool is_name, bool intern, bool symbolize) { size_t bufferSize = stringEnd - string; - const char *p = string, *pe = string, *unescape, *bufferStart; + const char *p = string, *pe = string, *bufferStart; char *buffer; - int unescape_len; - char buf[4]; VALUE result = rb_str_buf_new(bufferSize); rb_enc_associate_index(result, utf8_encindex); buffer = RSTRING_PTR(result); bufferStart = buffer; +#define APPEND_CHAR(chr) *buffer++ = chr; p = ++pe; + while (pe < stringEnd && (pe = memchr(pe, '\\', stringEnd - pe))) { - unescape = (char *) "?"; - unescape_len = 1; if (pe > p) { MEMCPY(buffer, p, char, pe - p); buffer += pe - p; } switch (*++pe) { + case '"': + case '/': + p = pe; // nothing to unescape just need to skip the backslash + break; + case '\\': + APPEND_CHAR('\\'); + break; case 'n': - unescape = (char *) "\n"; + APPEND_CHAR('\n'); break; case 'r': - unescape = (char *) "\r"; + APPEND_CHAR('\r'); break; case 't': - unescape = (char *) "\t"; - break; - case '"': - unescape = (char *) "\""; - break; - case '\\': - unescape = (char *) "\\"; + APPEND_CHAR('\t'); break; case 'b': - unescape = (char *) "\b"; + APPEND_CHAR('\b'); break; case 'f': - unescape = (char *) "\f"; + APPEND_CHAR('\f'); break; case 'u': if (pe > stringEnd - 5) { @@ -714,18 +713,23 @@ static VALUE json_string_unescape(JSON_ParserState *state, const char *string, c break; } } - unescape_len = convert_UTF32_to_UTF8(buf, ch); - unescape = buf; + + char buf[4]; + int unescape_len = convert_UTF32_to_UTF8(buf, ch); + MEMCPY(buffer, buf, char, unescape_len); + buffer += unescape_len; + p = ++pe; } break; default: - p = pe; - continue; + if ((unsigned char)*pe < 0x20) { + raise_parse_error_at("invalid ASCII control character in string: %s", state, pe - 1); + } + raise_parse_error_at("invalid escape character in string: %s", state, pe - 1); + break; } - MEMCPY(buffer, unescape, char, unescape_len); - buffer += unescape_len; - p = ++pe; } +#undef APPEND_CHAR if (stringEnd > p) { MEMCPY(buffer, p, char, stringEnd - p); @@ -976,9 +980,6 @@ static inline VALUE json_parse_string(JSON_ParserState *state, JSON_ParserConfig case '\\': { state->cursor++; escaped = true; - if ((unsigned char)*state->cursor < 0x20) { - raise_parse_error("invalid ASCII control character in string: %s", state); - } break; } default: diff --git a/test/json/fixtures/pass15.json b/test/json/fixtures/fail15.json similarity index 100% rename from test/json/fixtures/pass15.json rename to test/json/fixtures/fail15.json diff --git a/test/json/fixtures/pass16.json b/test/json/fixtures/fail16.json similarity index 100% rename from test/json/fixtures/pass16.json rename to test/json/fixtures/fail16.json diff --git a/test/json/fixtures/pass17.json b/test/json/fixtures/fail17.json similarity index 100% rename from test/json/fixtures/pass17.json rename to test/json/fixtures/fail17.json diff --git a/test/json/fixtures/pass26.json b/test/json/fixtures/fail26.json similarity index 100% rename from test/json/fixtures/pass26.json rename to test/json/fixtures/fail26.json diff --git a/test/json/json_fixtures_test.rb b/test/json/json_fixtures_test.rb index c153ebef7cbed1..c0d10379393f7b 100644 --- a/test/json/json_fixtures_test.rb +++ b/test/json/json_fixtures_test.rb @@ -10,6 +10,8 @@ class JSONFixturesTest < Test::Unit::TestCase source = File.read(f) define_method("test_#{name}") do assert JSON.parse(source), "Did not pass for fixture '#{File.basename(f)}': #{source.inspect}" + rescue JSON::ParserError + raise "#{File.basename(f)} parsing failure" end end diff --git a/test/json/json_parser_test.rb b/test/json/json_parser_test.rb index a5b4763618c192..3e662bda324ba5 100644 --- a/test/json/json_parser_test.rb +++ b/test/json/json_parser_test.rb @@ -510,8 +510,8 @@ def test_backslash data = ['"'] assert_equal data, parse(json) # - json = '["\\\'"]' - data = ["'"] + json = '["\\/"]' + data = ["/"] assert_equal data, parse(json) json = '["\/"]' From c7a84ae0bb8c6dab3463076d7e5ca9b6f89880f4 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sat, 22 Nov 2025 14:13:30 +0100 Subject: [PATCH 2/5] [ruby/json] parser.c: Record escape positions while parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can then pass them to the decoder to save having to parse the string again. ``` == Parsing activitypub.json (58160 bytes) ruby 3.4.6 (2025-09-16 revision https://github.com/ruby/json/commit/dbd83256b1) +YJIT +PRISM [arm64-darwin24] Warming up -------------------------------------- after 1.275k i/100ms Calculating ------------------------------------- after 12.774k (± 0.8%) i/s (78.29 μs/i) - 65.025k in 5.090834s Comparison: before: 12314.3 i/s after: 12773.8 i/s - 1.04x faster == Parsing twitter.json (567916 bytes) ruby 3.4.6 (2025-09-16 revision https://github.com/ruby/json/commit/dbd83256b1) +YJIT +PRISM [arm64-darwin24] Warming up -------------------------------------- after 143.000 i/100ms Calculating ------------------------------------- after 1.441k (± 0.2%) i/s (693.86 μs/i) - 7.293k in 5.060345s Comparison: before: 1430.1 i/s after: 1441.2 i/s - 1.01x faster == Parsing citm_catalog.json (1727030 bytes) ruby 3.4.6 (2025-09-16 revision https://github.com/ruby/json/commit/dbd83256b1) +YJIT +PRISM [arm64-darwin24] Warming up -------------------------------------- after 69.000 i/100ms Calculating ------------------------------------- after 695.919 (± 0.4%) i/s (1.44 ms/i) - 3.519k in 5.056691s Comparison: before: 687.8 i/s after: 695.9 i/s - 1.01x faster ``` https://github.com/ruby/json/commit/4f4551f993 --- ext/json/parser/parser.c | 88 ++++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 25 deletions(-) diff --git a/ext/json/parser/parser.c b/ext/json/parser/parser.c index 2f584a51a932f4..0216eef987e3f3 100644 --- a/ext/json/parser/parser.c +++ b/ext/json/parser/parser.c @@ -616,8 +616,10 @@ static inline bool json_string_cacheable_p(const char *string, size_t length) return length <= JSON_RVALUE_CACHE_MAX_ENTRY_LENGTH && rb_isalpha(string[0]); } -static inline VALUE json_string_fastpath(JSON_ParserState *state, const char *string, const char *stringEnd, bool is_name, bool intern, bool symbolize) +static inline VALUE json_string_fastpath(JSON_ParserState *state, JSON_ParserConfig *config, const char *string, const char *stringEnd, bool is_name) { + bool intern = is_name || config->freeze; + bool symbolize = is_name && config->symbolize_names; size_t bufferSize = stringEnd - string; if (is_name && state->in_array && RB_LIKELY(json_string_cacheable_p(string, bufferSize))) { @@ -636,8 +638,33 @@ static inline VALUE json_string_fastpath(JSON_ParserState *state, const char *st return build_string(string, stringEnd, intern, symbolize); } -static VALUE json_string_unescape(JSON_ParserState *state, const char *string, const char *stringEnd, bool is_name, bool intern, bool symbolize) +#define JSON_MAX_UNESCAPE_POSITIONS 16 +typedef struct _json_unescape_positions { + long size; + const char **positions; + bool has_more; +} JSON_UnescapePositions; + +static inline const char *json_next_backslash(const char *pe, const char *stringEnd, JSON_UnescapePositions *positions) +{ + while (positions->size) { + positions->size--; + const char *next_position = positions->positions[0]; + positions->positions++; + return next_position; + } + + if (positions->has_more) { + return memchr(pe, '\\', stringEnd - pe); + } + + return NULL; +} + +static NOINLINE() VALUE json_string_unescape(JSON_ParserState *state, JSON_ParserConfig *config, const char *string, const char *stringEnd, bool is_name, JSON_UnescapePositions *positions) { + bool intern = is_name || config->freeze; + bool symbolize = is_name && config->symbolize_names; size_t bufferSize = stringEnd - string; const char *p = string, *pe = string, *bufferStart; char *buffer; @@ -649,7 +676,7 @@ static VALUE json_string_unescape(JSON_ParserState *state, const char *string, c #define APPEND_CHAR(chr) *buffer++ = chr; p = ++pe; - while (pe < stringEnd && (pe = memchr(pe, '\\', stringEnd - pe))) { + while (pe < stringEnd && (pe = json_next_backslash(pe, stringEnd, positions))) { if (pe > p) { MEMCPY(buffer, p, char, pe - p); buffer += pe - p; @@ -893,20 +920,6 @@ static inline VALUE json_decode_object(JSON_ParserState *state, JSON_ParserConfi return object; } -static inline VALUE json_decode_string(JSON_ParserState *state, JSON_ParserConfig *config, const char *start, const char *end, bool escaped, bool is_name) -{ - VALUE string; - bool intern = is_name || config->freeze; - bool symbolize = is_name && config->symbolize_names; - if (escaped) { - string = json_string_unescape(state, start, end, is_name, intern, symbolize); - } else { - string = json_string_fastpath(state, start, end, is_name, intern, symbolize); - } - - return string; -} - static inline VALUE json_push_value(JSON_ParserState *state, JSON_ParserConfig *config, VALUE value) { if (RB_UNLIKELY(config->on_load_proc)) { @@ -964,22 +977,30 @@ static ALWAYS_INLINE() bool string_scan(JSON_ParserState *state) return false; } -static inline VALUE json_parse_string(JSON_ParserState *state, JSON_ParserConfig *config, bool is_name) +static VALUE json_parse_escaped_string(JSON_ParserState *state, JSON_ParserConfig *config, bool is_name, const char *start) { - state->cursor++; - const char *start = state->cursor; - bool escaped = false; + const char *backslashes[JSON_MAX_UNESCAPE_POSITIONS]; + JSON_UnescapePositions positions = { + .size = 0, + .positions = backslashes, + .has_more = false, + }; - while (RB_UNLIKELY(string_scan(state))) { + do { switch (*state->cursor) { case '"': { - VALUE string = json_decode_string(state, config, start, state->cursor, escaped, is_name); + VALUE string = json_string_unescape(state, config, start, state->cursor, is_name, &positions); state->cursor++; return json_push_value(state, config, string); } case '\\': { + if (RB_LIKELY(positions.size < JSON_MAX_UNESCAPE_POSITIONS)) { + backslashes[positions.size] = state->cursor; + positions.size++; + } else { + positions.has_more = true; + } state->cursor++; - escaped = true; break; } default: @@ -988,12 +1009,29 @@ static inline VALUE json_parse_string(JSON_ParserState *state, JSON_ParserConfig } state->cursor++; - } + } while (string_scan(state)); raise_parse_error("unexpected end of input, expected closing \"", state); return Qfalse; } +static ALWAYS_INLINE() VALUE json_parse_string(JSON_ParserState *state, JSON_ParserConfig *config, bool is_name) +{ + state->cursor++; + const char *start = state->cursor; + + if (RB_UNLIKELY(!string_scan(state))) { + raise_parse_error("unexpected end of input, expected closing \"", state); + } + + if (RB_LIKELY(*state->cursor == '"')) { + VALUE string = json_string_fastpath(state, config, start, state->cursor, is_name); + state->cursor++; + return json_push_value(state, config, string); + } + return json_parse_escaped_string(state, config, is_name, start); +} + #if JSON_CPU_LITTLE_ENDIAN_64BITS // From: https://lemire.me/blog/2022/01/21/swar-explained-parsing-eight-digits/ // Additional References: From f9efa0cc0468692739770e754c12edf46cdf7c8e Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sat, 22 Nov 2025 22:11:31 +0900 Subject: [PATCH 3/5] [ruby/openssl] pkey/ec: fix OpenSSL::PKey::EC::Group#curve_name for unknown curves EC_GROUP_get_curve_name() returns NID_undef when OpenSSL does not recognize the curve and there is no associated OID. Handle this case explicitly and return nil instead of the string "UNDEF", which should not be exposed outside the extension. https://github.com/ruby/openssl/commit/2c16821c07 --- ext/openssl/ossl_pkey_ec.c | 16 +++++++--------- test/openssl/test_pkey_ec.rb | 12 ++++++++++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c index c063450a4f2ef3..a9133062081c05 100644 --- a/ext/openssl/ossl_pkey_ec.c +++ b/ext/openssl/ossl_pkey_ec.c @@ -849,25 +849,23 @@ static VALUE ossl_ec_group_get_cofactor(VALUE self) /* * call-seq: - * group.curve_name => String + * group.curve_name -> string or nil * - * Returns the curve name (sn). + * Returns the curve name (short name) corresponding to this group, or +nil+ + * if \OpenSSL does not have an OID associated with the group. * * See the OpenSSL documentation for EC_GROUP_get_curve_name() */ static VALUE ossl_ec_group_get_curve_name(VALUE self) { - EC_GROUP *group = NULL; + EC_GROUP *group; int nid; GetECGroup(self, group); - if (group == NULL) - return Qnil; - nid = EC_GROUP_get_curve_name(group); - -/* BUG: an nid or asn1 object should be returned, maybe. */ - return rb_str_new2(OBJ_nid2sn(nid)); + if (nid == NID_undef) + return Qnil; + return rb_str_new_cstr(OBJ_nid2sn(nid)); } /* diff --git a/test/openssl/test_pkey_ec.rb b/test/openssl/test_pkey_ec.rb index df91a1be255f07..88085bc68c7ef5 100644 --- a/test/openssl/test_pkey_ec.rb +++ b/test/openssl/test_pkey_ec.rb @@ -369,18 +369,26 @@ def test_ec_point point2.to_octet_string(:uncompressed) assert_equal point2.to_octet_string(:uncompressed), point3.to_octet_string(:uncompressed) + end + def test_small_curve begin group = OpenSSL::PKey::EC::Group.new(:GFp, 17, 2, 2) group.point_conversion_form = :uncompressed generator = OpenSSL::PKey::EC::Point.new(group, B(%w{ 04 05 01 })) group.set_generator(generator, 19, 1) - point = OpenSSL::PKey::EC::Point.new(group, B(%w{ 04 06 03 })) rescue OpenSSL::PKey::EC::Group::Error pend "Patched OpenSSL rejected curve" if /unsupported field/ =~ $!.message raise end - + assert_equal 17.to_bn.num_bits, group.degree + assert_equal B(%w{ 04 05 01 }), + group.generator.to_octet_string(:uncompressed) + assert_equal 19.to_bn, group.order + assert_equal 1.to_bn, group.cofactor + assert_nil group.curve_name + + point = OpenSSL::PKey::EC::Point.new(group, B(%w{ 04 06 03 })) assert_equal 0x040603.to_bn, point.to_bn assert_equal 0x040603.to_bn, point.to_bn(:uncompressed) assert_equal 0x0306.to_bn, point.to_bn(:compressed) From 424499dd10aa01b3d84957761b19dde63b28113c Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Sat, 17 May 2025 18:42:28 +0900 Subject: [PATCH 4/5] [ruby/openssl] ts: refactor converting string to ASN1_OBJECT obj_to_asn1obj() in ossl_ts.c and ossl_asn1.c are identical. Let's remove one in ossl_ts.c. eASN1Error can now be made static to ossl_asn1.c. https://github.com/ruby/openssl/commit/dcb05c40c2 --- ext/openssl/ossl_asn1.c | 10 +++++----- ext/openssl/ossl_asn1.h | 6 +++++- ext/openssl/ossl_ts.c | 19 +++---------------- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c index 186679da4c6b55..58efe96f2a8ec2 100644 --- a/ext/openssl/ossl_asn1.c +++ b/ext/openssl/ossl_asn1.c @@ -160,7 +160,7 @@ asn1integer_to_num_i(VALUE arg) #define ossl_asn1_set_indefinite_length(o,v) rb_ivar_set((o),sivINDEFINITE_LENGTH,(v)) VALUE mASN1; -VALUE eASN1Error; +static VALUE eASN1Error; VALUE cASN1Data; static VALUE cASN1Primitive; @@ -247,8 +247,8 @@ obj_to_asn1null(VALUE obj) return null; } -static ASN1_OBJECT* -obj_to_asn1obj(VALUE obj) +ASN1_OBJECT * +ossl_to_asn1obj(VALUE obj) { ASN1_OBJECT *a1obj; @@ -544,7 +544,7 @@ ossl_asn1_get_asn1type(VALUE obj) free_func = (free_func_type *)ASN1_STRING_free; break; case V_ASN1_OBJECT: - ptr = obj_to_asn1obj(value); + ptr = ossl_to_asn1obj(value); free_func = (free_func_type *)ASN1_OBJECT_free; break; case V_ASN1_UTCTIME: @@ -1205,7 +1205,7 @@ ossl_asn1obj_get_oid(VALUE self) ASN1_OBJECT *a1obj; int state; - a1obj = obj_to_asn1obj(ossl_asn1_get_value(self)); + a1obj = ossl_to_asn1obj(ossl_asn1_get_value(self)); str = rb_protect(asn1obj_get_oid_i, (VALUE)a1obj, &state); ASN1_OBJECT_free(a1obj); if (state) diff --git a/ext/openssl/ossl_asn1.h b/ext/openssl/ossl_asn1.h index 3b689c0ee7ded5..a723b06f602624 100644 --- a/ext/openssl/ossl_asn1.h +++ b/ext/openssl/ossl_asn1.h @@ -31,11 +31,15 @@ VALUE asn1str_to_str(const ASN1_STRING *); VALUE asn1integer_to_num(const ASN1_INTEGER *); ASN1_INTEGER *num_to_asn1integer(VALUE, ASN1_INTEGER *); +/* + * ASN1_OBJECT conversions + */ +ASN1_OBJECT *ossl_to_asn1obj(VALUE obj); + /* * ASN1 module */ extern VALUE mASN1; -extern VALUE eASN1Error; extern VALUE cASN1Data; diff --git a/ext/openssl/ossl_ts.c b/ext/openssl/ossl_ts.c index 3c505b64a9f6a5..57d4b2a70e4ae6 100644 --- a/ext/openssl/ossl_ts.c +++ b/ext/openssl/ossl_ts.c @@ -132,23 +132,10 @@ asn1_to_der(void *template, int (*i2d)(void *template, unsigned char **pp)) return str; } -static ASN1_OBJECT* -obj_to_asn1obj(VALUE obj) -{ - ASN1_OBJECT *a1obj; - - StringValue(obj); - a1obj = OBJ_txt2obj(RSTRING_PTR(obj), 0); - if(!a1obj) a1obj = OBJ_txt2obj(RSTRING_PTR(obj), 1); - if(!a1obj) ossl_raise(eASN1Error, "invalid OBJECT ID"); - - return a1obj; -} - static VALUE obj_to_asn1obj_i(VALUE obj) { - return (VALUE)obj_to_asn1obj(obj); + return (VALUE)ossl_to_asn1obj(obj); } static VALUE @@ -264,7 +251,7 @@ ossl_ts_req_set_algorithm(VALUE self, VALUE algo) X509_ALGOR *algor; GetTSRequest(self, req); - obj = obj_to_asn1obj(algo); + obj = ossl_to_asn1obj(algo); mi = TS_REQ_get_msg_imprint(req); algor = TS_MSG_IMPRINT_get_algo(mi); if (!X509_ALGOR_set0(algor, obj, V_ASN1_NULL, NULL)) { @@ -394,7 +381,7 @@ ossl_ts_req_set_policy_id(VALUE self, VALUE oid) int ok; GetTSRequest(self, req); - obj = obj_to_asn1obj(oid); + obj = ossl_to_asn1obj(oid); ok = TS_REQ_set_policy_id(req, obj); ASN1_OBJECT_free(obj); if (!ok) From dd489ee9c48fc8c2b499b80f3ebcd053de33bb0a Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 19 Nov 2025 01:41:35 +0900 Subject: [PATCH 5/5] [ruby/openssl] asn1: refactor converting ASN1_OBJECT to string ruby/openssl exposes OIDs to Ruby as strings in many places, but the conversion logic has been duplicated and the behavior is inconsistent. There are mainly two patterns: - Returns the short name associated with the OID/NID, or the dotted decimal notation if it is unknown to OpenSSL. - Returns the long name, or the dotted decimal notation. These patterns are implemented using different OpenSSL APIs and that caused subtle differences. Add helper functions ossl_asn1obj_to_string() and ossl_asn1obj_to_string_long_name() to unify the logic. Also, document the current behaviors where it is not yet done. The inconsistency was likely unintentional, but since it dates back to the original implementations, standardizing it now would cause more issues than it resolves. https://github.com/ruby/openssl/commit/2ea36c21a4 --- ext/openssl/ossl_asn1.c | 91 +++++++++++++++++++++-------------- ext/openssl/ossl_asn1.h | 10 ++++ ext/openssl/ossl_ocsp.c | 11 +---- ext/openssl/ossl_ts.c | 29 ++--------- ext/openssl/ossl_x509attr.c | 21 ++------ ext/openssl/ossl_x509cert.c | 20 +++----- ext/openssl/ossl_x509crl.c | 20 ++++---- ext/openssl/ossl_x509ext.c | 23 +++------ ext/openssl/ossl_x509name.c | 36 +++++--------- ext/openssl/ossl_x509req.c | 21 ++++---- test/openssl/test_asn1.rb | 7 ++- test/openssl/test_ts.rb | 3 +- test/openssl/test_x509cert.rb | 1 + test/openssl/test_x509crl.rb | 1 + test/openssl/test_x509req.rb | 5 ++ 15 files changed, 139 insertions(+), 160 deletions(-) diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c index 58efe96f2a8ec2..1309811c742e31 100644 --- a/ext/openssl/ossl_asn1.c +++ b/ext/openssl/ossl_asn1.c @@ -147,6 +147,48 @@ asn1integer_to_num_i(VALUE arg) return asn1integer_to_num((ASN1_INTEGER *)arg); } +/* + * ASN1_OBJECT conversions + */ +VALUE +ossl_asn1obj_to_string_oid(const ASN1_OBJECT *a1obj) +{ + VALUE str; + int len; + + str = rb_usascii_str_new(NULL, 127); + len = OBJ_obj2txt(RSTRING_PTR(str), RSTRING_LENINT(str), a1obj, 1); + if (len <= 0 || len == INT_MAX) + ossl_raise(eOSSLError, "OBJ_obj2txt"); + if (len > RSTRING_LEN(str)) { + /* +1 is for the \0 terminator added by OBJ_obj2txt() */ + rb_str_resize(str, len + 1); + len = OBJ_obj2txt(RSTRING_PTR(str), len + 1, a1obj, 1); + if (len <= 0) + ossl_raise(eOSSLError, "OBJ_obj2txt"); + } + rb_str_set_len(str, len); + return str; +} + +VALUE +ossl_asn1obj_to_string(const ASN1_OBJECT *obj) +{ + int nid = OBJ_obj2nid(obj); + if (nid != NID_undef) + return rb_str_new_cstr(OBJ_nid2sn(nid)); + return ossl_asn1obj_to_string_oid(obj); +} + +VALUE +ossl_asn1obj_to_string_long_name(const ASN1_OBJECT *obj) +{ + int nid = OBJ_obj2nid(obj); + if (nid != NID_undef) + return rb_str_new_cstr(OBJ_nid2ln(nid)); + return ossl_asn1obj_to_string_oid(obj); +} + /********/ /* * ASN1 module @@ -393,32 +435,27 @@ decode_null(unsigned char* der, long length) return Qnil; } +VALUE +asn1obj_to_string_i(VALUE arg) +{ + return ossl_asn1obj_to_string((const ASN1_OBJECT *)arg); +} + static VALUE decode_obj(unsigned char* der, long length) { ASN1_OBJECT *obj; const unsigned char *p; VALUE ret; - int nid; - BIO *bio; + int state; p = der; - if(!(obj = d2i_ASN1_OBJECT(NULL, &p, length))) - ossl_raise(eASN1Error, NULL); - if((nid = OBJ_obj2nid(obj)) != NID_undef){ - ASN1_OBJECT_free(obj); - ret = rb_str_new2(OBJ_nid2sn(nid)); - } - else{ - if(!(bio = BIO_new(BIO_s_mem()))){ - ASN1_OBJECT_free(obj); - ossl_raise(eASN1Error, NULL); - } - i2a_ASN1_OBJECT(bio, obj); - ASN1_OBJECT_free(obj); - ret = ossl_membio2str(bio); - } - + if (!(obj = d2i_ASN1_OBJECT(NULL, &p, length))) + ossl_raise(eASN1Error, "d2i_ASN1_OBJECT"); + ret = rb_protect(asn1obj_to_string_i, (VALUE)obj, &state); + ASN1_OBJECT_free(obj); + if (state) + rb_jump_tag(state); return ret; } @@ -1172,23 +1209,7 @@ ossl_asn1obj_get_ln(VALUE self) static VALUE asn1obj_get_oid_i(VALUE vobj) { - ASN1_OBJECT *a1obj = (void *)vobj; - VALUE str; - int len; - - str = rb_usascii_str_new(NULL, 127); - len = OBJ_obj2txt(RSTRING_PTR(str), RSTRING_LENINT(str), a1obj, 1); - if (len <= 0 || len == INT_MAX) - ossl_raise(eASN1Error, "OBJ_obj2txt"); - if (len > RSTRING_LEN(str)) { - /* +1 is for the \0 terminator added by OBJ_obj2txt() */ - rb_str_resize(str, len + 1); - len = OBJ_obj2txt(RSTRING_PTR(str), len + 1, a1obj, 1); - if (len <= 0) - ossl_raise(eASN1Error, "OBJ_obj2txt"); - } - rb_str_set_len(str, len); - return str; + return ossl_asn1obj_to_string_oid((const ASN1_OBJECT *)vobj); } /* diff --git a/ext/openssl/ossl_asn1.h b/ext/openssl/ossl_asn1.h index a723b06f602624..b605df8f3f8ce2 100644 --- a/ext/openssl/ossl_asn1.h +++ b/ext/openssl/ossl_asn1.h @@ -35,6 +35,16 @@ ASN1_INTEGER *num_to_asn1integer(VALUE, ASN1_INTEGER *); * ASN1_OBJECT conversions */ ASN1_OBJECT *ossl_to_asn1obj(VALUE obj); +/* + * Returns the short name if available, the dotted decimal notation otherwise. + * This is the most common way to return ASN1_OBJECT to Ruby. + */ +VALUE ossl_asn1obj_to_string(const ASN1_OBJECT *a1obj); +/* + * However, some places use long names instead. This is likely unintentional, + * but we keep the current behavior in existing methods. + */ +VALUE ossl_asn1obj_to_string_long_name(const ASN1_OBJECT *a1obj); /* * ASN1 module diff --git a/ext/openssl/ossl_ocsp.c b/ext/openssl/ossl_ocsp.c index 84d38760e5c0de..390215a8e3d67c 100644 --- a/ext/openssl/ossl_ocsp.c +++ b/ext/openssl/ossl_ocsp.c @@ -1591,19 +1591,10 @@ ossl_ocspcid_get_hash_algorithm(VALUE self) { OCSP_CERTID *id; ASN1_OBJECT *oid; - BIO *out; GetOCSPCertId(self, id); OCSP_id_get0_info(NULL, &oid, NULL, NULL, id); - - if (!(out = BIO_new(BIO_s_mem()))) - ossl_raise(eOCSPError, "BIO_new"); - - if (!i2a_ASN1_OBJECT(out, oid)) { - BIO_free(out); - ossl_raise(eOCSPError, "i2a_ASN1_OBJECT"); - } - return ossl_membio2str(out); + return ossl_asn1obj_to_string_long_name(oid); } /* diff --git a/ext/openssl/ossl_ts.c b/ext/openssl/ossl_ts.c index 57d4b2a70e4ae6..575418ccc6ce46 100644 --- a/ext/openssl/ossl_ts.c +++ b/ext/openssl/ossl_ts.c @@ -138,27 +138,6 @@ obj_to_asn1obj_i(VALUE obj) return (VALUE)ossl_to_asn1obj(obj); } -static VALUE -get_asn1obj(const ASN1_OBJECT *obj) -{ - BIO *out; - VALUE ret; - int nid; - if ((nid = OBJ_obj2nid(obj)) != NID_undef) - ret = rb_str_new2(OBJ_nid2sn(nid)); - else{ - if (!(out = BIO_new(BIO_s_mem()))) - ossl_raise(eTimestampError, "BIO_new(BIO_s_mem())"); - if (i2a_ASN1_OBJECT(out, obj) <= 0) { - BIO_free(out); - ossl_raise(eTimestampError, "i2a_ASN1_OBJECT"); - } - ret = ossl_membio2str(out); - } - - return ret; -} - static VALUE ossl_ts_req_alloc(VALUE klass) { @@ -229,7 +208,7 @@ ossl_ts_req_get_algorithm(VALUE self) mi = TS_REQ_get_msg_imprint(req); algor = TS_MSG_IMPRINT_get_algo(mi); X509_ALGOR_get0(&obj, NULL, NULL, algor); - return get_asn1obj(obj); + return ossl_asn1obj_to_string(obj); } /* @@ -358,7 +337,7 @@ ossl_ts_req_get_policy_id(VALUE self) GetTSRequest(self, req); if (!TS_REQ_get_policy_id(req)) return Qnil; - return get_asn1obj(TS_REQ_get_policy_id(req)); + return ossl_asn1obj_to_string(TS_REQ_get_policy_id(req)); } /* @@ -948,7 +927,7 @@ ossl_ts_token_info_get_policy_id(VALUE self) TS_TST_INFO *info; GetTSTokenInfo(self, info); - return get_asn1obj(TS_TST_INFO_get_policy_id(info)); + return ossl_asn1obj_to_string(TS_TST_INFO_get_policy_id(info)); } /* @@ -976,7 +955,7 @@ ossl_ts_token_info_get_algorithm(VALUE self) mi = TS_TST_INFO_get_msg_imprint(info); algo = TS_MSG_IMPRINT_get_algo(mi); X509_ALGOR_get0(&obj, NULL, NULL, algo); - return get_asn1obj(obj); + return ossl_asn1obj_to_string(obj); } /* diff --git a/ext/openssl/ossl_x509attr.c b/ext/openssl/ossl_x509attr.c index d983af59686946..9beb15f4a0ca71 100644 --- a/ext/openssl/ossl_x509attr.c +++ b/ext/openssl/ossl_x509attr.c @@ -164,29 +164,18 @@ ossl_x509attr_set_oid(VALUE self, VALUE oid) /* * call-seq: - * attr.oid => string + * attr.oid -> string + * + * Returns the OID of the attribute. Returns the short name or the dotted + * decimal notation. */ static VALUE ossl_x509attr_get_oid(VALUE self) { X509_ATTRIBUTE *attr; - ASN1_OBJECT *oid; - BIO *out; - VALUE ret; - int nid; GetX509Attr(self, attr); - oid = X509_ATTRIBUTE_get0_object(attr); - if ((nid = OBJ_obj2nid(oid)) != NID_undef) - ret = rb_str_new2(OBJ_nid2sn(nid)); - else{ - if (!(out = BIO_new(BIO_s_mem()))) - ossl_raise(eX509AttrError, NULL); - i2a_ASN1_OBJECT(out, oid); - ret = ossl_membio2str(out); - } - - return ret; + return ossl_asn1obj_to_string(X509_ATTRIBUTE_get0_object(attr)); } /* diff --git a/ext/openssl/ossl_x509cert.c b/ext/openssl/ossl_x509cert.c index c7653031b4bc1f..cf00eb78bf9c03 100644 --- a/ext/openssl/ossl_x509cert.c +++ b/ext/openssl/ossl_x509cert.c @@ -318,27 +318,23 @@ ossl_x509_set_serial(VALUE self, VALUE num) /* * call-seq: * cert.signature_algorithm => string + * + * Returns the signature algorithm used to sign this certificate. This returns + * the algorithm name found in the TBSCertificate structure, not the outer + * \Certificate structure. + * + * Returns the long name of the signature algorithm, or the dotted decimal + * notation if \OpenSSL does not define a long name for it. */ static VALUE ossl_x509_get_signature_algorithm(VALUE self) { X509 *x509; - BIO *out; const ASN1_OBJECT *obj; - VALUE str; GetX509(self, x509); - out = BIO_new(BIO_s_mem()); - if (!out) ossl_raise(eX509CertError, NULL); - X509_ALGOR_get0(&obj, NULL, NULL, X509_get0_tbs_sigalg(x509)); - if (!i2a_ASN1_OBJECT(out, obj)) { - BIO_free(out); - ossl_raise(eX509CertError, NULL); - } - str = ossl_membio2str(out); - - return str; + return ossl_asn1obj_to_string_long_name(obj); } /* diff --git a/ext/openssl/ossl_x509crl.c b/ext/openssl/ossl_x509crl.c index b9ee5f05692b52..af4803f47d3cc9 100644 --- a/ext/openssl/ossl_x509crl.c +++ b/ext/openssl/ossl_x509crl.c @@ -166,26 +166,26 @@ ossl_x509crl_set_version(VALUE self, VALUE version) return version; } +/* + * call-seq: + * crl.signature_algorithm -> string + * + * Returns the signature algorithm used to sign this CRL. + * + * Returns the long name of the signature algorithm, or the dotted decimal + * notation if \OpenSSL does not define a long name for it. + */ static VALUE ossl_x509crl_get_signature_algorithm(VALUE self) { X509_CRL *crl; const X509_ALGOR *alg; const ASN1_OBJECT *obj; - BIO *out; GetX509CRL(self, crl); - if (!(out = BIO_new(BIO_s_mem()))) { - ossl_raise(eX509CRLError, NULL); - } X509_CRL_get0_signature(crl, NULL, &alg); X509_ALGOR_get0(&obj, NULL, NULL, alg); - if (!i2a_ASN1_OBJECT(out, obj)) { - BIO_free(out); - ossl_raise(eX509CRLError, NULL); - } - - return ossl_membio2str(out); + return ossl_asn1obj_to_string_long_name(obj); } static VALUE diff --git a/ext/openssl/ossl_x509ext.c b/ext/openssl/ossl_x509ext.c index 01aa3a8f51cd17..01b342b5be7a32 100644 --- a/ext/openssl/ossl_x509ext.c +++ b/ext/openssl/ossl_x509ext.c @@ -359,27 +359,20 @@ ossl_x509ext_set_critical(VALUE self, VALUE flag) return flag; } +/* + * call-seq: + * ext.oid -> string + * + * Returns the OID of the extension. Returns the short name or the dotted + * decimal notation. + */ static VALUE ossl_x509ext_get_oid(VALUE obj) { X509_EXTENSION *ext; - ASN1_OBJECT *extobj; - BIO *out; - VALUE ret; - int nid; GetX509Ext(obj, ext); - extobj = X509_EXTENSION_get_object(ext); - if ((nid = OBJ_obj2nid(extobj)) != NID_undef) - ret = rb_str_new2(OBJ_nid2sn(nid)); - else{ - if (!(out = BIO_new(BIO_s_mem()))) - ossl_raise(eX509ExtError, NULL); - i2a_ASN1_OBJECT(out, extobj); - ret = ossl_membio2str(out); - } - - return ret; + return ossl_asn1obj_to_string(X509_EXTENSION_get_object(ext)); } static VALUE diff --git a/ext/openssl/ossl_x509name.c b/ext/openssl/ossl_x509name.c index 7d0fd35247f5cb..5483450aa7606b 100644 --- a/ext/openssl/ossl_x509name.c +++ b/ext/openssl/ossl_x509name.c @@ -341,34 +341,22 @@ static VALUE ossl_x509name_to_a(VALUE self) { X509_NAME *name; - X509_NAME_ENTRY *entry; - int i,entries,nid; - char long_name[512]; - const char *short_name; - VALUE ary, vname, ret; - ASN1_STRING *value; + int entries; + VALUE ret; GetX509Name(self, name); entries = X509_NAME_entry_count(name); ret = rb_ary_new_capa(entries); - for (i=0; itype)); - rb_ary_push(ret, ary); + for (int i = 0; i < entries; i++) { + const X509_NAME_ENTRY *entry = X509_NAME_get_entry(name, i); + if (!entry) + ossl_raise(eX509NameError, "X509_NAME_get_entry"); + const ASN1_OBJECT *obj = X509_NAME_ENTRY_get_object(entry); + VALUE vname = ossl_asn1obj_to_string(obj); + const ASN1_STRING *data = X509_NAME_ENTRY_get_data(entry); + VALUE vdata = asn1str_to_str(data); + VALUE type = INT2NUM(ASN1_STRING_type(data)); + rb_ary_push(ret, rb_ary_new_from_args(3, vname, vdata, type)); } return ret; } diff --git a/ext/openssl/ossl_x509req.c b/ext/openssl/ossl_x509req.c index eae57969241954..1ae0fd56a6e612 100644 --- a/ext/openssl/ossl_x509req.c +++ b/ext/openssl/ossl_x509req.c @@ -255,27 +255,26 @@ ossl_x509req_set_subject(VALUE self, VALUE subject) return subject; } +/* + * call-seq: + * req.signature_algorithm -> string + * + * Returns the signature algorithm used to sign this request. + * + * Returns the long name of the signature algorithm, or the dotted decimal + * notation if \OpenSSL does not define a long name for it. + */ static VALUE ossl_x509req_get_signature_algorithm(VALUE self) { X509_REQ *req; const X509_ALGOR *alg; const ASN1_OBJECT *obj; - BIO *out; GetX509Req(self, req); - - if (!(out = BIO_new(BIO_s_mem()))) { - ossl_raise(eX509ReqError, NULL); - } X509_REQ_get0_signature(req, NULL, &alg); X509_ALGOR_get0(&obj, NULL, NULL, alg); - if (!i2a_ASN1_OBJECT(out, obj)) { - BIO_free(out); - ossl_raise(eX509ReqError, NULL); - } - - return ossl_membio2str(out); + return ossl_asn1obj_to_string_long_name(obj); } static VALUE diff --git a/test/openssl/test_asn1.rb b/test/openssl/test_asn1.rb index 501e35151fb5e4..df8b0accb36c84 100644 --- a/test/openssl/test_asn1.rb +++ b/test/openssl/test_asn1.rb @@ -306,7 +306,11 @@ def test_null end def test_object_identifier - encode_decode_test B(%w{ 06 01 00 }), OpenSSL::ASN1::ObjectId.new("0.0".b) + obj = encode_decode_test B(%w{ 06 01 00 }), OpenSSL::ASN1::ObjectId.new("0.0".b) + assert_equal "0.0", obj.oid + assert_nil obj.sn + assert_nil obj.ln + assert_equal obj.oid, obj.value encode_decode_test B(%w{ 06 01 28 }), OpenSSL::ASN1::ObjectId.new("1.0".b) encode_decode_test B(%w{ 06 03 88 37 03 }), OpenSSL::ASN1::ObjectId.new("2.999.3".b) encode_decode_test B(%w{ 06 05 2A 22 83 BB 55 }), OpenSSL::ASN1::ObjectId.new("1.2.34.56789".b) @@ -314,6 +318,7 @@ def test_object_identifier assert_equal "2.16.840.1.101.3.4.2.1", obj.oid assert_equal "SHA256", obj.sn assert_equal "sha256", obj.ln + assert_equal obj.sn, obj.value assert_raise(OpenSSL::ASN1::ASN1Error) { OpenSSL::ASN1.decode(B(%w{ 06 00 })) } diff --git a/test/openssl/test_ts.rb b/test/openssl/test_ts.rb index 7b154d1c37994e..cca7898bc13d86 100644 --- a/test/openssl/test_ts.rb +++ b/test/openssl/test_ts.rb @@ -88,8 +88,9 @@ def test_request_assignment assert_raise(TypeError) { req.version = nil } assert_raise(TypeError) { req.version = "foo" } - req.algorithm = "SHA1" + req.algorithm = "sha1" assert_equal("SHA1", req.algorithm) + assert_equal("SHA1", OpenSSL::ASN1.ObjectId("SHA1").sn) assert_raise(TypeError) { req.algorithm = nil } assert_raise(OpenSSL::ASN1::ASN1Error) { req.algorithm = "xxx" } diff --git a/test/openssl/test_x509cert.rb b/test/openssl/test_x509cert.rb index 55481690e9a49a..877eac69ce5e34 100644 --- a/test/openssl/test_x509cert.rb +++ b/test/openssl/test_x509cert.rb @@ -236,6 +236,7 @@ def test_invalid_extension def test_sign_and_verify cert = issue_cert(@ca, @rsa1, 1, [], nil, nil, digest: "SHA256") + assert_equal("sha256WithRSAEncryption", cert.signature_algorithm) # ln assert_equal(true, cert.verify(@rsa1)) assert_equal(false, cert.verify(@rsa2)) assert_equal(false, certificate_error_returns_false { cert.verify(@ec1) }) diff --git a/test/openssl/test_x509crl.rb b/test/openssl/test_x509crl.rb index 3c364f57d535e1..81c9247df2c9d6 100644 --- a/test/openssl/test_x509crl.rb +++ b/test/openssl/test_x509crl.rb @@ -20,6 +20,7 @@ def test_basic assert_equal(cert.issuer.to_der, crl.issuer.to_der) assert_equal(now, crl.last_update) assert_equal(now+1600, crl.next_update) + assert_equal("sha256WithRSAEncryption", crl.signature_algorithm) # ln crl = OpenSSL::X509::CRL.new(crl.to_der) assert_equal(1, crl.version) diff --git a/test/openssl/test_x509req.rb b/test/openssl/test_x509req.rb index 0a2df47bcac7eb..b198a1185aba59 100644 --- a/test/openssl/test_x509req.rb +++ b/test/openssl/test_x509req.rb @@ -42,6 +42,11 @@ def test_subject assert_equal(@dn.to_der, req.subject.to_der) end + def test_signature_algorithm + req = issue_csr(0, @dn, @rsa1, "SHA256") + assert_equal("sha256WithRSAEncryption", req.signature_algorithm) # ln + end + def create_ext_req(exts) ef = OpenSSL::X509::ExtensionFactory.new exts = exts.collect{|e| ef.create_extension(*e) }