Hi, 2011/3/17 Samuel Mimram <[email protected]>: > 2011/3/16 Jehan Pagès <[email protected]> >> >> I'll make an undevelopped response, embedded in your email below, >> before going to bed, because in my timezone, that's 5h AM passed. And >> I'll send more details in a few days because tomorrow afternoon, I >> must travel for a few days. So I'll get back a computer and Internet >> only on Sunday afternoon. > > Hehe, you should really think of getting some sleep at some point ;)
Yes I should! >> > 1. I thing that you are using the "get_verify_result" function. A better >> > alternative is to use the "verify" function which returns unit but raise >> > an >> > exception of type "verify_error" which is pretty informative. Is it what >> > you >> > were looking for? >> >> I think I must have missed this "verify" function (I don't know how >> with the place taken by the verify_error type!). :-) >> That's better indeed but this is still missing information. If I were >> the maintainer ;-), I would have adequate contextual parameters on the >> error type. Like this: >> type verify_error = >> | Error_v_cert_has_expired of string (* the date of the expiration. It >> could also be a int, which could indicate the number of day it has >> expired since. That's maybe better for internationalization. Anyway >> anything usable... :-) *) >> | Error_v_subject_issuer_mismatch of string * string (* the subject >> and issuer which mismatch >> [...] > > I'm not so sure of this. The idea of this library is to make a binding of > ssl, not to improve it. Moreover, which parameters are useful really depends > on your application (for example there is no canonical choice for > representing dates as you mention). So I think it would be much better to > leave the exceptions this way, and to provide helper functions to get the > necessary complementary information on the error. Yes I tend to think usually that libraries should make the developer's life easier, by being more "intelligent". But I understand your logics. That's up to you. Though I am also often annoyed that binding from C into higher level languages (like Ocaml) don't try more to use the language possibilities instead of following the limitations of C. C is not able to be contextual. functions just return a single type's values and this is up to developers to give meaning to it. Ocaml on the other side can return exception on errors, and can even have contextual arguments of different type and size for these exceptions, as well as on any type in fact. That give a lot more meaning and readability to a code written in Ocaml than one you read in C, which is very opaque. Your binding though is rather nice. You transformed returned error number into exceptions and this kind of thing. So that's not a critic directed to you but a general thought on many binding I saw where the idea is to follow the exact same interface as the original C API, which makes very ugly code. And also to say that we can always go further into code intelligence, like giving contextual arguments to verify_errors for instance. Because obviously when I am told a cert is expired, I will want to know since then. If a software does not care, for instance because it is very strict and will refuse the cert anyway, it simply won't check the argument. No big deal. For any other software which will want to present a user the possibility to validate manually a cert, it will want more details about the date. So we could avoid them the additional lines of code for this. >> Stuffs like this. My examples may be badly chosen (I did not think >> them through) but that's the idea. By the way, you might want to >> review the "unused" description on some of these. That does not seem >> up to date with http://www.openssl.org/docs/apps/verify.html >> On my distribution too, the man is not up to date... But I guess >> upstream has better information. :-) > > This is now fixed. Nice. >> Also I think I found a bug, but I'll need to make further test to >> point you where it comes from (from looking at your C code, I have an >> idea, but I need to confirm by testing). > > It is really the best if you can reproduce the bug with a C snippet. It will > be easy for me to debug this afterwards... Yes. I am not sure the bug is in the Ocaml binding or the original C library. Or simply if there is no bug and that's the logics of the C API that I did not get. I would have to make a test fully in C. Probably later. Basically the issue is that: 1/ If I make a context with an empty verify_mode list [] (= SSL_VERIFY_NONE), it does not stop on Ssl.connect and I can check the cert and cipher afterwards with Ssl.get_certificate/cipher. 2/ Now if I use a [Ssl.Verify_peer] mode, it will raise Ssl.connection_error during Ssl.connect. Until here all is fine, this is the logics. Then the SSL negotiation is ended, still fine. Now I catch the exception of course and I would want after to know what went wrong in the cert. So I try as previously to get the cert/cipher with Ssl.get_certificate/cipher. This time, they will both raise Ssl.certificate_error! So the question is: is it normal that when we set the verify mode to SSL_VERIFY_PEER, we cannot check the cert afterwards? I would think it is not. But further tests directly on the C API should be tried to see the original API behaviour. >> > 2. This should be easy. Please provide me with the exact list of things >> > that >> > you would like to see, and ideally with how you would do this in C (I >> > only >> > added issuer and and subject because these are the only provided >> > examples in >> > man SSL_CTX_set_verify...). >> >> That's indeed pretty strange. There is not even a man for the >> functions you use X509_get_subject_name and X509_get_issuer_name. They >> seem to be used only in examples of other mans. >> And searching the web, it seems only these 2 functions exist as >> "simple" field interface. For the rest (all other fields of a cert), I >> think generic functions must be used. For instance, this maybe: >> « >> X509_NAME_ENTRY_get_data() retrieves the field value of ne in and >> ASN1_STRING structure. >> » >> Or maybe this: >> « >> X509_NAME *d2i_X509_NAME(X509_NAME **a, unsigned char **pp, long length); >> These functions decode and encode an X509_NAME structure which is the >> the same as the Name type defined in RFC2459 (and elsewhere) and used >> for example in >> certificate subject and issuer names. >> » >> I guess that needs to be tested. >> I may make C test later (next week?) unless you do them before. :-) > > Again, C code printing the information you need would be perfect since it is > quite easy for me to convert this to bindings afterwards. I don't have much > free time right now, so I think that you will have done this before me ;) Yes. I won't do it right now because once again it is late here. But I'll try to find out directly with C code which C function needs to be used to get all the fields in a cert. >> > 3. The verify_callback is really a hack, where you can only use the >> > provided >> > client_verify_callback: this it why it should change in the future, in >> > order >> > to allow caml functions. Actually, the problem is that >> > SSL_CTX_set_verify is >> > waiting for a C function and there is no custom data which is passed in >> > which I could pass a pointer to the caml function to call (ideally I >> > should >> > be able to put the pointer to the caml function insinde X509_STORE_CTX >> > which >> > is passed to the callback but this does not seem to be possible). Maybe >> > do >> > you have an idea of how I could design the bindings? >> >> I don't think that's difficult thanks to OCaml closure facilities. You >> just need to wrap the user-provided closure into a single function. >> Then you make a callback of this single function in C, and you always >> call only this same function, which is actually an "empty" >> implementation calling the user-provided one. >> >> Example: >> >> let user_callback = ref (fun _ -> ());; >> >> let unique_callback n = >> !user_callback n >> ;; >> >> Callback.register "verify callback" unique_callback;; >> >> Then in C: >> void verify_callback(int arg) >> { >> caml_callback(*caml_named_value("verify callback"), Val_int(arg)); >> } > > Yes, this is a possible way to implement this. However, this is not nice > because the callback is then global (ie you cannot have different callbacks > for different ssl contexts). This is why I did not use this trick, waiting > for a better design. Anyway, a global callback would be better than no > custom callback, so I might implement this soon, but if you think of > something better this would be great... I agree. Right now your context type seems to map only on C code. One idea could be to have on the Ocaml side the type context defined as a Ocaml record with 2 values, whose one value embed the C context and the other the Ocaml callback for this context because callbacks are actually context-dependent. So you would not call a global callback but the one recorded in the corresponding context. >> Ah also, a last feature I forgot in my previous email: >> you have a function to get a cert from a file, but none to write a >> cert into a file. That would be good to be able to save certs received >> through a connection (for various reasons, the "typical" one is that >> you want to save a cert the user has accepted not to ask him each >> time; maybe also for security, to compare certs between connections, >> and any other reason a developer might think of!). > > This is easy to implement, I will add this. Nice! > So, I'm waiting for your input, and will add things I promised in the > meantime... > > ++ > > Sam. > I'll try and make the C code when I get time myself. Hopefully this week. :-) Thanks for all. Jehan ------------------------------------------------------------------------------ Colocation vs. Managed Hosting A question and answer guide to determining the best fit for your organization - today and in the future. http://p.sf.net/sfu/internap-sfd2d _______________________________________________ Savonet-devl mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/savonet-devl
