openssl icon indicating copy to clipboard operation
openssl copied to clipboard

instead of looking of NIDs and then using X509V3_EXT_nconf_nid,

Open mcr opened this issue 8 years ago • 9 comments

instead just pass strings to X509V3_EXT_nconf, which has all the logic for processing dealing with generic extensions. This is not yet right; it may insert two additional bytes to the value, and does not deal with the critical correctly. This pull request is for discussion.

mcr avatar Aug 28 '17 16:08 mcr

The ruby-core mailing list or this GitHub issue tracker is the right place for questions about ruby-openssl.

From your email on openssl-users:

Like Bob Moskowitz who has been posting about IDevID, I have also been
creating certificates with custom/private extensions in aid of creating
IDevIDs.

I'm one of the authors of both:
    https://datatracker.ietf.org/doc/draft-ietf-anima-bootstrapping-keyinfra/
and https://datatracker.ietf.org/doc/draft-ietf-anima-voucher/

I want to create certificates with custom/private extensions programatically,
and I found it very difficult to do using the current ruby-openssl modules.

I was sure that it must be possible from the C-API, and found that this
following change to ruby's interface worked well for me:
    https://github.com/ruby/openssl/pull/141


    } I haven't found a seperate openssl-ruby list as yet, so please  {
    } excuse the BCC, and as this is not a security issue, I haven't  {
    } used that address.  Please redirect me.                         {


The critical change being:
-    ext = X509V3_EXT_nconf_nid(conf, ctx, nid, RSTRING_PTR(valstr));
+    ext = X509V3_EXT_nconf(conf, ctx, RSTRING_PTR(oid), RSTRING_PTR(value));

Because EXT_nconf does all the nid lookups, and processes the generics
itself, no point in the ruby-ssl code limiting itself to OIDs predefined
in objects.h by it's use of nid lookups directly.


    ** I'm asking the Openssl team if I'm using the API reasonably **
    ** Clearly, I have some possible garbage that has leaked in    **

My code looks like:

    @idevid.add_extension(ef.create_extension(
                          "subjectAltName",
                          sprintf("otherName:1.3.6.1.4.1.46930.1;UTF8:%s",
                                   self.sanitized_eui64),
                          false))

which let me put a private extension having my private hardware number into
the SAN.  That works just great, I think.  I have in fact extracted the
result in metdtls code in my constrained device a few months ago.

After my changes above, I could now also do: (46930 being my PEN)

@idevid.add_extension(ef.create_extension(
                      "1.3.6.1.4.1.46930.2",
                      "ASN1:UTF8String:http://www.sandelman.ca",
                      false))

which would insert a MASAURL (using a PEN until we get an id-pe OID
allocated) as described in: https://tools.ietf.org/html/draft-ietf-anima-bootstrapping-keyinfra-07#section-2.2

Of concern is that when I look at the resulting certificate:

dooku-[fountain/spec/certs](2.3.0) mcr 10006 %openssl x509 -noout -text -in
12-00-00-66-4D-02.crt
Certificate:
...
            X509v3 Subject Alternative Name:
                othername:
            1.3.6.1.4.1.46930.2:
                ..http://www.sandelman.ca

Looking at a hexdump I see "0x0c" and "0x17" prior to the http, but maybe
it's a length or something.... I wondered if there was garbage or a UTF-8 BOM
or something inserted..  so, I pointed asn1parse at the result, and I see:

400:d=5  hl=2 l=   9 prim: OBJECT            :1.3.6.1.4.1.46930.2
411:d=5  hl=2 l=  25 prim: OCTET STRING      [HEX DUMP]:0C17687474703A2F2F7777772E73616E64656C6D616E2E6361

So the 0xc and 0x17 are really there.

Clearly, it's not being take as an UTF8String (because I would expect to see
UTF8STRING as the type if it were), yet the ASN1:UTF8String is in fact being
processed somehow.

If you want to see the whole certificate result, as sample/test data to my
Registrar:
  https://github.com/mcr/fountain/blob/master/spec/certs/12-00-00-66-4D-02.crt

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        | network architect  [
]     [email protected]  http://www.sandelman.ca/        |   ruby on rails    [

rhenium avatar Aug 29 '17 03:08 rhenium

The critical change being:
-    ext = X509V3_EXT_nconf_nid(conf, ctx, nid, RSTRING_PTR(valstr));
+    ext = X509V3_EXT_nconf(conf, ctx, RSTRING_PTR(oid), RSTRING_PTR(value));

Because EXT_nconf does all the nid lookups, and processes the generics
itself, no point in the ruby-ssl code limiting itself to OIDs predefined
in objects.h by it's use of nid lookups directly.

NIDs can be added at run time with OpenSSL::ASN1::ObjectId.register (which calls OBJ_create()), but yes, this should be fixed.

For whatever reason, OpenSSL::X509::ExtensionFactory#create_ext has accepted long names which aren't handled by the non-generic extensions path of X509V3_EXT_nconf(). For compatibility I guess it will be like this.

const char *oid_cstr = StringValueCStr(oid);
int nid = OBJ_ln2nid(oid_cstr);
if (nid != NID_undef)
    oid_cstr = OBJ_nid2sn(nid);
ext = X509V3_EXT_nconf(conf, ctx, oid_cstr, RSTRING_PTR(valstr));
@idevid.add_extension(ef.create_extension(
                      "1.3.6.1.4.1.46930.2",
                      "ASN1:UTF8String:http://www.sandelman.ca",
                      false))
[...]

400:d=5  hl=2 l=   9 prim: OBJECT            :1.3.6.1.4.1.46930.2
411:d=5  hl=2 l=  25 prim: OCTET STRING      [HEX DUMP]:0C17687474703A2F2F7777772E73616E64656C6D616E2E6361

So the 0xc and 0x17 are really there.

Clearly, it's not being take as an UTF8String (because I would expect to see
UTF8STRING as the type if it were), yet the ASN1:UTF8String is in fact being
processed somehow.

It's working as expected. The ASN.1 type definition of Extension is:

Extension  ::=  SEQUENCE  {
     extnID      OBJECT IDENTIFIER,
     critical    BOOLEAN DEFAULT FALSE,
     extnValue   OCTET STRING
                 -- contains the DER encoding of an ASN.1 value
                 -- corresponding to the extension type identified
                 -- by extnID
     }

The leading "\x0c\x17" is the BER tag and the length of the UTF8String encapsulated in the 'extnValue'.

require "openssl"
require "pp"

ef = OpenSSL::X509::ExtensionFactory.new
# Workaround -- allocate a NID
OpenSSL::ASN1::ObjectId.register("1.3.6.1.4.1.46930.2", "x-oid", "x-oid")
e = ef.create_extension("x-oid",
                        "ASN1:UTF8String:http://www.sandelman.ca",
                        false)

pp x = OpenSSL::ASN1.decode(e.to_der)
# =>
# #<OpenSSL::ASN1::Sequence:0x0000000000ba6f58
#  @infinite_length=false,
#  @tag=16,
#  @tag_class=:UNIVERSAL,
#  @tagging=nil,
#  @value=
#   [#<OpenSSL::ASN1::ObjectId:0x0000000000ba73b8
#     @infinite_length=false,
#     @tag=6,
#     @tag_class=:UNIVERSAL,
#     @tagging=nil,
#     @value="x-oid">,
#    #<OpenSSL::ASN1::OctetString:0x0000000000ba7200
#     @infinite_length=false,
#     @tag=4,
#     @tag_class=:UNIVERSAL,
#     @tagging=nil,
#     @value="\f\x17http://www.sandelman.ca">]>

pp OpenSSL::ASN1.decode(x.value[1].value)
# =>
# #<OpenSSL::ASN1::UTF8String:0x0000000000b70908
#  @infinite_length=false,
#  @tag=12,
#  @tag_class=:UNIVERSAL,
#  @tagging=nil,
#  @value="http://www.sandelman.ca">

rhenium avatar Aug 29 '17 03:08 rhenium

Thank you so much for the reply.

I will comment in the issue as requested, but I'll do so in email so that I can CC the openssl-users list.

Kazuki Yamaguchi [email protected] wrote: > The ruby-core mailing list or this GitHub issue tracker is the right > place for questions about ruby-openssl.

mcr> Of concern is that when I look at the resulting certificate:

mcr> dooku-[fountain/spec/certs](2.3.0) mcr 10006 %openssl x509 -noout -text
mcr> -in 12-00-00-66-4D-02.crt Certificate: ...  X509v3 Subject Alternative
mcr> Name: othername: 1.3.6.1.4.1.46930.2: ..http://www.sandelman.ca

mcr> Looking at a hexdump I see "0x0c" and "0x17" prior to the http, but
mcr> maybe it's a length or something.... I wondered if there was garbage or
mcr> a UTF-8 BOM or something inserted..  so, I pointed asn1parse at the
mcr> result, and I see:

ky> NIDs can be added at run time with OpenSSL::ASN1::ObjectId.register ky> (which calls OBJ_create()), but yes, this should be fixed.

I did not find a way to call OBJ_create() from ruby. Is there one? Many OpenSSL FAQs suggest you need to hack objects.h and recompile, which is clearly a PITA if you are trying to live above distribute ruby binaries, so I was looking for another way.

ky> For whatever reason, OpenSSL::X509::ExtensionFactory#create_ext has ky> accepted long names which aren't handled by the non-generic extensions ky> path of X509V3_EXT_nconf(). For compatibility I guess it will be like ky> this...

Ah, that's why it uses that way. I'll add that code to my tree, and update the pull request.

Are there regression tests which cover that? I was hoping travis would tell me about such failures that I didn't know about :-)

ky> It's working as expected. The ASN.1 type definition of Extension is:

ky> -- contains the DER encoding of an ASN.1 value

ky> The leading "\x0c\x17" is the BER tag and the length of the UTF8String ky> encapsulated in the 'extnValue'.

okay, so "openssl x509 -text" is failing to decode that then.

@value="http://www.sandelman.ca">

Awesome!

-- ] Never tell me the odds! | ipv6 mesh networks [ ] Michael Richardson, Sandelman Software Works | network architect [ ] [email protected] http://www.sandelman.ca/ | ruby on rails [

mcr avatar Aug 29 '17 20:08 mcr

I did not find a way to call OBJ_create() from ruby. Is there one? Many OpenSSL FAQs suggest you need to hack objects.h and recompile, which is clearly a PITA if you are trying to live above distribute ruby binaries, so I was looking for another way.

OpenSSL::ASN1::ObjectId.register (in ext/openssl/ossl_asn1.c) calls.

Are there regression tests which cover that? I was hoping travis would tell me about such failures that I didn't know about :-)

No, sadly. It would be helpful if you could add some to test/test_x509ext.rb.

rhenium avatar Aug 30 '17 00:08 rhenium

I have revised the code:

  • retains the critical check
  • includes the nid lookup you suggested
  • moves all declations to the top.

mcr avatar Sep 05 '17 21:09 mcr

I will investigate adding a regress test case for this, can you give me an example of the nid lookup that would not have worked without the ln2nid() call?

mcr avatar Sep 05 '17 21:09 mcr

The changes in ext/openssl/ossl_x509ext.c look good. Thanks.

I will investigate adding a regress test case for this, can you give me an example of the nid lookup that would not have worked without the ln2nid() call?

For example...

$ ruby -ropenssl -e'p OpenSSL::ASN1::ObjectId.new("keyUsage").ln'              
"X509v3 Key Usage"

rhenium avatar Sep 15 '17 18:09 rhenium

@mcr do you mind rebasing on master and explaining where we are at with this PR?

ioquatix avatar Oct 13 '19 04:10 ioquatix

Just say that I haven't lost track of this. I just had less time/energy while travelling than I thought I would.

mcr avatar Dec 05 '19 18:12 mcr

I'm sorry it took so long to reply. This patch is still relevant and the changes in ext/openssl looks good to me.

Let me add some test cases.

rhenium avatar Aug 31 '23 05:08 rhenium

I had to add a cast from const char * to char * for OpenSSL 1.0.2's X509V3_EXT_nconf().

rhenium avatar Aug 31 '23 06:08 rhenium

I had to add a cast from const char * to char * for OpenSSL 1.0.2's X509V3_EXT_nconf().

Given how insecure OpenSSL 1.0.x is, do we really care? It would be nice to bring us up to 1.1.x as the minimum, as we try to get to 3.x :-)

mcr avatar Aug 31 '23 13:08 mcr

I had to add a cast from const char * to char * for OpenSSL 1.0.2's X509V3_EXT_nconf().

Given how insecure OpenSSL 1.0.x is, do we really care? It would be nice to bring us up to 1.1.x as the minimum, as we try to get to 3.x :-)

The openssl/ruby currently maintains the old OpenSSL versions even when these are the end of life (EOL).

https://github.com/ruby/openssl/blob/fcda6cf9d5f69f6dc0c297ca76feff71f9021f00/.github/workflows/test.yml#L78-L82

As a real use case, Red Hat Enterprise Linux (RHEL) 7's latest version RHEL 7.9 has OpenSSL version 1.0.2k in my investigation. As RHEL 7's end of life is in June 2024, according to this announcement. I assume that some people may use the openssl gem in the environment until the EOL, June 2024.

junaruga avatar Aug 31 '23 14:08 junaruga

I personally don't believe OpenSSL 1.0.2 support is a strict requirement for us, but we should keep master branch compatible with 1.0.2 until we officially drop it.

rhenium avatar Aug 31 '23 14:08 rhenium

CI is happy with -Werror now. Let's merge this.

rhenium avatar Aug 31 '23 14:08 rhenium

Thank you so much!

rhenium avatar Aug 31 '23 14:08 rhenium

Nice work team!

ioquatix avatar Aug 31 '23 21:08 ioquatix