[ruby-core:33906] [Ruby 1.9-Feature#4183] [ext/openssl] Timestamp support

Issue #4183 has been updated by Martin Bosslet.

File ts2.tar.gz added

Thanks for the input! I updated the code with regard to Aaron's and Nobuyoshi's comments. Concerning Nobuyoshi's questions:

>Just curious for my eyes,
>In ossl_ts_verify():
>1068     if (!(ctx->store = X509_STORE_new())) {
>1069         ossl_raise(eTimestampError, NULL);
>1070         goto end;
>1071     }

>Jump to clean-up after raise?

>1109 end:
>1110     TS_VERIFY_CTX_free(ctx);
>1111     return ret;

I corrected this, cleanup is now performed before the raise.


>In ossl_tsfac_create_ts():
>1231         if (!(inter_certs = sk_X509_new_null())) goto end;

>When inter_certs can't get created, all the rest are just skipped?

I forgot to set the error message there. Corrected this, now an error is raised when allocation fails.


>1232         if (tsa_cert)
>1233         if (rb_obj_is_kind_of(additional_certs, rb_cArray)) {
>1234             for (i = 0; i < RARRAY_LEN(additional_certs); i++) {
>1235                 cert = rb_ary_entry(additional_certs, i);
>1236                 sk_X509_push(inter_certs, GetX509CertPtr(cert));
>1237             }
>1238         }
>1239         else {
>1240             sk_X509_push(inter_certs, GetX509CertPtr(additional_certs));
>1241         }

>Just indentation of 1233..1241 is wrong, or 1232 is misplaced?

No, you're right, it was simply misplaced, I removed it.

Regarding your patch, I already applied it in the update.


There is still some functionality that is not supported and some features I dislike about the way OpenSSL handles timestamp verification. Concerning the missing functionality - I'm planning to add this in case the feature will be accepted, I first wanted to get your reactions before I indulge too deeply in something nobody would want anyway :) Missing features right now are:

- Extensions are neither supported for Request nor for the Response
- Accuracy and TSAName are not supported for the Response.
- I'd like to add some factory methods, that create Responses indicating errors easily. E.g. 

error_ts = OpenSSL::Timestamp::Response.create_fault_response(OpenSSL::Timestamp::Response::REJECTION,
                                                            :UNACCEPTED_POLICY,
                                                            "The policy you provided is not supported by this server")

This would simplify error creation for errors detected on the server side when manually analyzing the Request.


Things I would like different:
- I don't like the fact that for verification, you need to pass OpenSSL a BIO, even if you're still in possession of the TS_REQ*, as in our case (and I guess in most use cases). This forces encoding (and later decoding in OpenSSL) the TS_REQ*, which is unnecessary. I'll try to submit a patch to OpenSSL regarding this.
- The second thing I'm not really happy with is the fact that you cannot validate a timestamp solely based on the timestamp end entity certificate, verification always includes certificate validation of the entire chain. This is too tightly coupled in my opinion, certificate validation should only be an option (so that there is the possibility to perform it in a separate step). In addition, decoupling would also remove the necessity to provide intermediate certificates or root certificates. If the timestamp already contains the timestamp authority certificate(which it must if Request#cert_requested? is true), validation would be self-contained, no other external resources needed. I'll also try to submit a patch for this to OpenSSL.

@Hiroshi: thanks for the confidence, I'm happy to help :)

Regards,
Martin

----------------------------------------
http://redmine.ruby-lang.org/issues/show/4183

----------------------------------------
http://redmine.ruby-lang.org