The following looks entirely bogus to me. Whatever the problem is, it can hardly be there. What's the story?
revision 1.79 date: 2004/10/13 23:53:21; author: bill; state: Exp; lines: +3 -1 for some reason, context->random doesn't get set when using sslfile. this should be fixed now.
@@ -356,6 +356,8 @@ stream->set_close_callback (0); stream->set_id (1);
+ if(!ctx->random) + ctx->random = Crypto.Random.random_string; conn = SSL.connection (!is_client, ctx);
if(is_blocking) {
What's so bogus? There's a prototype for this member in context.pike, but it's never defined. Ideally, the problem should be fixed in the context object, but there was apparently a reason why it wasn't set there in the first place.
Bill
On Wed, 26 Jan 2005, Martin Stjernholm, Roxen IS @ Pike developers forum wrote:
The following looks entirely bogus to me. Whatever the problem is, it can hardly be there. What's the story?
revision 1.79 date: 2004/10/13 23:53:21; author: bill; state: Exp; lines: +3 -1 for some reason, context->random doesn't get set when using sslfile. this should be fixed now.
@@ -356,6 +356,8 @@ stream->set_close_callback (0); stream->set_id (1);
if(!ctx->random)
ctx->random = Crypto.Random.random_string;conn = SSL.connection (!is_client, ctx);
if(is_blocking) {
The bogusness is that sslfile really has nothing to do with the state in the context. It simply passes it on to the connection object. So the "fix" belong equally well in the caller of SSL.sslfile(), or in the create function for SSL.connection (probably slightly more motivated there).
Ideally, the problem should be fixed in the context object, but there was apparently a reason why it wasn't set there in the first place.
There are lots of odd things in the SSL code - it's very old. Without looking in to it I'd say it could just as well be a simple bug in context.pike, but maybe you have specific reasons to believe it's intentional?
If we should try to figure out a proper fix, it's good to begin with the specific case where this caused an error for you. What happened?
The bogusness is that sslfile really has nothing to do with the state in the context. It simply passes it on to the connection object. So the "fix" belong equally well in the caller of SSL.sslfile(), or in the create function for SSL.connection (probably slightly more motivated there).
Personally, I think the fix belongs in context. It would seem appropriate to set a reasonable value to that element there. I didn't do that because other code (non-sslfile) seemed to work just fine, so in the spirit of not messing with common code I didn't totally understand, it went in sslfile.
There are lots of odd things in the SSL code - it's very old. Without looking in to it I'd say it could just as well be a simple bug in context.pike, but maybe you have specific reasons to believe it's intentional?
As I've found out. I like to think there was a good reason for it being the way it is, though I don't confess to be able to read the author's mind :) The reason I believe it to be intentional is that it's not just defined, it's also set to zero. This tells me that there's a particular reason for not setting it to a function that works.
If we should try to figure out a proper fix, it's good to begin with the specific case where this caused an error for you. What happened?
I wish I could tell you. The only way for me to tell is to revert the change and see what happens, though it's possible it wasn't an all out failure. I'll check out 7.7 to see if it jogs my memory.
Bill
Personally, I think the fix belongs in context.
Yes, we agree on that.
/.../ I didn't do that because other code (non-sslfile) seemed to work just fine, so in the spirit of not messing with common code I didn't totally understand, it went in sslfile.
Aha, so you're implying you totally understand sslfile. Congrats, that's impressive! ;) Even though I wrote essentially all of it I still can't say I understand it all the time.
/.../ The reason I believe it to be intentional is that it's not just defined, it's also set to zero. This tells me that there's a particular reason for not setting it to a function that works.
If the declaration was "function(int:string) random = 0;" I'd agree with you. But now I can't find any place where it's explicitly set to zero. Where do you look?
Besides your kludge, I can't even find a place where any code tests its value. (Granted, I haven't looked very closely in Roxen or ChiliMoon or any of the other big TLS apps - searching for "random" in them trigs a bit too many false hits.)
I think both Roxen WebServer and ChiliMoon just sets the the function when SSLProtocol is initialized.
Aha, so you're implying you totally understand sslfile. Congrats, that's impressive! ;) Even though I wrote essentially all of it I still can't say I understand it all the time.
Well, I don't know that total understanding is the case, but I understand the whole module much more clearly after digging through it.
If the declaration was "function(int:string) random = 0;" I'd agree with you. But now I can't find any place where it's explicitly set to zero. Where do you look?
I don't remember offhand; though I'm fairly certain that was the case at some point. The reality is that the current context doesn't make that declaration, so perhaps it was unintentional. Either way, I tend to think that it belongs in context.
Besides your kludge, I can't even find a place where any code tests its value. (Granted, I haven't looked very closely in Roxen or ChiliMoon or any of the other big TLS apps - searching for "random" in them trigs a bit too many false hits.)
That's the problem. The code just assumes it's been set to a function. I'm pretty sure the problem was that it was throwing errors because 0!=function. I can't think of a real reason to make that function configurable, unless you're worried about performance of randomness vs. quality of randomness.
Bill
/.../ I can't think of a real reason to make that function configurable, unless you're worried about performance of randomness vs. quality of randomness.
As Nilsson mentioned, the TLS apps have set that function themselves. I see several examples of that now, both in Roxen and in Protocol.HTTP.Query when it sets up a client side TLS connection. So the de-facto interface seems to be that you should do it in your own app before the SSL connection is established.
I can't see the point with forcing the apps to do that explicitly, so a decent default value for SSL.context.random would be nice. It wouldn't produce any problem with the existing apps either.
pike-devel@lists.lysator.liu.se
-
Bill Welliver -
Martin Nilsson (DivX Networks) ＠ Pike (-) developers forum -
Martin Stjernholm, Roxen IS ＠ Pike developers forum