Re: [Freeipa-devel] [PATCH] Fix File parameter validation when prompting.
John Dennis wrote: I've been thinking about this a bit more. I wonder if part of the inelegance is due to the fact we're trying to shoehorn two distinct concepts into one item when a proper relationship does not exist. It does not seem logical that a file is a subclass of a string which is how this is set up now. Files simply do not derive from strings, they are two entirely distinct concepts. I think our parameter classes follow a different logic. They don't provide any functionality (except for calling validation/normalization routines) and are pretty much separated from the rest of the ipalib code. They *describe* the type of value a command expects to receive on the command line (or in a field in the webUI). In other words the parameter classes define syntax of what a command expects as parameters, but they don't say anything about semantics. This is why the File parameter extends the Str class - the command expects a string. Maybe we should rename it to Filename. Consider any typical command line program you're familiar with. You might pass string data as an argument or it might be read from stdin. But if you want that command to read from a file you have to pass it a different argument specifying the filename to open. Take the case of sed for example. If you pass a script on the command line you use the -e arg, however, if you want to pass the script as a file you use the -f arg. They aren't the same at the point of invocation, internally the program maps one to the other. Maybe that's why the proposed mechanism is awkward, we're trying to conflate one concept with another. Perhaps there should be distinct arguments available for the user to the use, one arg passes a string, a different arg passes a filename. The filename parameter opens the file, reads the content and then assigns the value to the appropriate keyword parameter. For the file arg you would have: kw[param.alias] = value instead of: kw[param.name] = value where param.alias is the name of the destination parameter associated with an alternative arg for supplying the same value. I think there are two distinct cases for typical command line programs I know: 1) File is specified as an argument/option or contents are loaded from stdin. 2) String data are entered as an argument/option or loaded from a file specified with another argument/option. Both of them are used by sed and are doable with what we have. We don't have mutual exclusion for parameters, but it's easy to make one parameter override another one. In this scheme the you don't need to keep state, you don't need to special case any code, you can use the existing normalize and validate mechanisms. The problem here is that validate/normalize can only be used on final values passed to commands. Both validate and normalize are called on the client and then again on the server. Files need to be loaded on the client, so you can't do it in a method that is shared. Thoughts? Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 362 remove group pwd policy on group deletion
Rob Crittenden wrote: Try to remove a group password policy when a group is deleted. No need to leave that hanging around. rob The self parameter is missing in the post_callback, it's not a static method. auto-ack when this is fixed. :) Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix File parameter validation when prompting.
On 01/29/2010 07:53 AM, Pavel Zuna wrote: John Dennis wrote: In this scheme the you don't need to keep state, you don't need to special case any code, you can use the existing normalize and validate mechanisms. The problem here is that validate/normalize can only be used on final values passed to commands. Both validate and normalize are called on the client and then again on the server. Files need to be loaded on the client, so you can't do it in a method that is shared. A File Command class validates by assuring the file is readable, it's normalize reads the contents. Then it calls the matching Str command class with the data it read. The Str class validate and normalizes the string data unaware it originated from a file. On the server side it never sees the File Command parameter, only Str parameter. Think of the File Command as a temporary command parameter which generates the resulting Str parameter. -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix File parameter validation when prompting.
John Dennis wrote: On 01/29/2010 07:53 AM, Pavel Zuna wrote: John Dennis wrote: In this scheme the you don't need to keep state, you don't need to special case any code, you can use the existing normalize and validate mechanisms. The problem here is that validate/normalize can only be used on final values passed to commands. Both validate and normalize are called on the client and then again on the server. Files need to be loaded on the client, so you can't do it in a method that is shared. A File Command class validates by assuring the file is readable, it's normalize reads the contents. Then it calls the matching Str command class with the data it read. The Str class validate and normalizes the string data unaware it originated from a file. On the server side it never sees the File Command parameter, only Str parameter. Think of the File Command as a temporary command parameter which generates the resulting Str parameter. I think we've been through this before. It would definitely be nice, but it would require a redesign and rewrite of the parameter framework and that's probably not worth it at this point. Pavel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 355 allow named to use ldapi
https://bugzilla.redhat.com/show_bug.cgi?id=558984 :-) Jason Gerard DeRose wrote: On Wed, 2010-01-27 at 14:53 -0500, Rob Crittenden wrote: Add SELinux rules so named can communicate to the DS over ldapi. This should fix the installation error when --setup-dns is set and SELinux is enforcing. rob I'm trying to test this out, but I'm not sure what I need to enter for the DNS forwarder: Enter IP address for a DNS forwarder (empty to stop): Any advice? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Jenny Galipeau jgali...@redhat.com Principal Software QA Engineer Red Hat, Inc. Security Engineering Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 360 be smarter about decoding certs
John Dennis wrote: On 01/28/2010 10:30 PM, Rob Crittenden wrote: John Dennis wrote: On 01/28/2010 04:15 PM, Rob Crittenden wrote: Gah, got the description mixed up with the last patch :-( Be a bit smarter about decoding certificates that might be base64 encoded. First see if it only contains those characters allowed before trying to decode it. This reduces the number of false positives. I'm not sure the test is doing what you want or even if it's the right test. The test is saying If there is one or more characters in the bas64 alphabet then try and decode. That means just about anything will match, which doesn't seem like a very strong test. Why not just try and decode it and let the decoder decide if it's really base64, the decoder has much strong rules about the input, including assuring the padding is correct. The reason is I had a binary cert that was correctly decoded by the base64 encoder. I don't know the why's and wherefores but there it is. Then testing to see if each byte is in the base64 alphabet would not have prevented this error. And yet it did in practice. I think you're assuming too much about the input testing in base64.b64decode(). It gladly takes binary data, as long as it fits the expected padding. For a while now I've been feeling like we need to associate a format attribute to the certificate (e.g. DER, PEM, BASE64, etc.). There is simply no good way to carry that extra data when all you have is a blob of data. We'd still need some mechanism to look at it and ask what are you? That or we simply reject some types of input. Or we need to adopt a convention that certs are always in one canonical format and the interface is responsible for assuring what it accepts as input is converted to the canonical form. Again, something would need to do that and base64.b64decode() is not sufficient. I know this seems rather hacky, I thought as much when I coded it, just trying to make it robust. rob I see what you mean about my regex being a bit weak though, it really should require that the entire string conform. I'll see what I can do. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 362 remove group pwd policy on group deletion
Pavel Zuna wrote: Rob Crittenden wrote: Try to remove a group password policy when a group is deleted. No need to leave that hanging around. rob The self parameter is missing in the post_callback, it's not a static method. auto-ack when this is fixed. :) Pavel Wow, great catch. Amazing that this worked at all with a bad function signature, guess I was just lucky. Fixed and pushed to master rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 355 allow named to use ldapi
Jenny Galipeau wrote: https://bugzilla.redhat.com/show_bug.cgi?id=558984 :-) Jason Gerard DeRose wrote: On Wed, 2010-01-27 at 14:53 -0500, Rob Crittenden wrote: Add SELinux rules so named can communicate to the DS over ldapi. This should fix the installation error when --setup-dns is set and SELinux is enforcing. rob I'm trying to test this out, but I'm not sure what I need to enter for the DNS forwarder: Enter IP address for a DNS forwarder (empty to stop): Any advice? Yeah, you probably don't need to enter anything here. David, basically with a forwarder it skips the local DNS server and instead forwards the request to the specified server(s) to do the DNS resolution work for it. You can also do per-domain forwarding but we don't supply a configuration option for that, at least during installation. I assume we could set that up post-installation. This is handy in a VPN situation. You run a local caching nameserver with DNS forwards across the VPN for your company domain(s). Everything else gets resolved using the standard public roots. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 360 be smarter about decoding certs
On 01/29/2010 09:28 AM, Rob Crittenden wrote: John Dennis wrote: On 01/28/2010 10:30 PM, Rob Crittenden wrote: John Dennis wrote: On 01/28/2010 04:15 PM, Rob Crittenden wrote: Gah, got the description mixed up with the last patch :-( Be a bit smarter about decoding certificates that might be base64 encoded. First see if it only contains those characters allowed before trying to decode it. This reduces the number of false positives. I'm not sure the test is doing what you want or even if it's the right test. The test is saying If there is one or more characters in the bas64 alphabet then try and decode. That means just about anything will match, which doesn't seem like a very strong test. Why not just try and decode it and let the decoder decide if it's really base64, the decoder has much strong rules about the input, including assuring the padding is correct. The reason is I had a binary cert that was correctly decoded by the base64 encoder. I don't know the why's and wherefores but there it is. Then testing to see if each byte is in the base64 alphabet would not have prevented this error. And yet it did in practice. I think you're assuming too much about the input testing in base64.b64decode(). It gladly takes binary data, as long as it fits the expected padding. You're right, I just went and checked the code, it skips any char not in the base64 alphabet :-( For a while now I've been feeling like we need to associate a format attribute to the certificate (e.g. DER, PEM, BASE64, etc.). There is simply no good way to carry that extra data when all you have is a blob of data. We'd still need some mechanism to look at it and ask what are you? That or we simply reject some types of input. My concern is that correctly deducing what an object is just by scanning it's contents is not robust. As you've seen it's easy to draw the wrong conclusion. Rather if the convention is it must be an object in this format (e.g. canonical) then there is no reason to even ask the question, it's simpler and more robust for most of our (internal) code, we only have to worry about it at the interface boundaries. So who enforces the canonical format? The only place we have to be concerned is when it's user provided, any item we produce will be guaranteed to be in the canonical format (hopefully :-). That just means at our interface boundaries we *must* specify the canonical format. If we're taking input from the user on the command line we offer them the option of input as pem, input as der, input as base64, try to validate as best we can trusting the user has told us the correct format and then convert to the canonical format. Think about the openssl x509 utilities, with those you must specify the input format. If we're taking input through an exposed API we do essentially the same thing. Require the format be passed along with the data, validate as best we can, and convert to the canonical format as it enters our system. BTW, by having the user/caller indicate the format they're providing will make the validation more robust, for example if it's stated the data is in DER format then there is no reason to even try to see if it can be base64 decoded which might lead to a false positive. Likewise if it's stated it's in pem format it must have the header and footer. Bottom line, I'm leery of trying to guess at random points what the format is, it's too easy for the guessing logic to draw the wrong conclusion, I'd much rather see it be explicit. Or we need to adopt a convention that certs are always in one canonical format and the interface is responsible for assuring what it accepts as input is converted to the canonical form. Again, something would need to do that and base64.b64decode() is not sufficient. I know this seems rather hacky, I thought as much when I coded it, just trying to make it robust. rob I see what you mean about my regex being a bit weak though, it really should require that the entire string conform. I'll see what I can do. rob -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 360 be smarter about decoding certs
John Dennis wrote: On 01/29/2010 09:28 AM, Rob Crittenden wrote: John Dennis wrote: On 01/28/2010 10:30 PM, Rob Crittenden wrote: John Dennis wrote: On 01/28/2010 04:15 PM, Rob Crittenden wrote: Gah, got the description mixed up with the last patch :-( Be a bit smarter about decoding certificates that might be base64 encoded. First see if it only contains those characters allowed before trying to decode it. This reduces the number of false positives. I'm not sure the test is doing what you want or even if it's the right test. The test is saying If there is one or more characters in the bas64 alphabet then try and decode. That means just about anything will match, which doesn't seem like a very strong test. Why not just try and decode it and let the decoder decide if it's really base64, the decoder has much strong rules about the input, including assuring the padding is correct. The reason is I had a binary cert that was correctly decoded by the base64 encoder. I don't know the why's and wherefores but there it is. Then testing to see if each byte is in the base64 alphabet would not have prevented this error. And yet it did in practice. I think you're assuming too much about the input testing in base64.b64decode(). It gladly takes binary data, as long as it fits the expected padding. You're right, I just went and checked the code, it skips any char not in the base64 alphabet :-( For a while now I've been feeling like we need to associate a format attribute to the certificate (e.g. DER, PEM, BASE64, etc.). There is simply no good way to carry that extra data when all you have is a blob of data. We'd still need some mechanism to look at it and ask what are you? That or we simply reject some types of input. My concern is that correctly deducing what an object is just by scanning it's contents is not robust. As you've seen it's easy to draw the wrong conclusion. Rather if the convention is it must be an object in this format (e.g. canonical) then there is no reason to even ask the question, it's simpler and more robust for most of our (internal) code, we only have to worry about it at the interface boundaries. So who enforces the canonical format? The only place we have to be concerned is when it's user provided, any item we produce will be guaranteed to be in the canonical format (hopefully :-). That just means at our interface boundaries we *must* specify the canonical format. If we're taking input from the user on the command line we offer them the option of input as pem, input as der, input as base64, try to validate as best we can trusting the user has told us the correct format and then convert to the canonical format. Think about the openssl x509 utilities, with those you must specify the input format. If we're taking input through an exposed API we do essentially the same thing. Require the format be passed along with the data, validate as best we can, and convert to the canonical format as it enters our system. BTW, by having the user/caller indicate the format they're providing will make the validation more robust, for example if it's stated the data is in DER format then there is no reason to even try to see if it can be base64 decoded which might lead to a false positive. Likewise if it's stated it's in pem format it must have the header and footer. Bottom line, I'm leery of trying to guess at random points what the format is, it's too easy for the guessing logic to draw the wrong conclusion, I'd much rather see it be explicit. Perhaps but validators take a single argument so there is no way to pass in type. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 360 be smarter about decoding certs
On 01/29/2010 11:26 AM, Rob Crittenden wrote: John Dennis wrote: On 01/29/2010 09:28 AM, Rob Crittenden wrote: John Dennis wrote: On 01/28/2010 10:30 PM, Rob Crittenden wrote: John Dennis wrote: On 01/28/2010 04:15 PM, Rob Crittenden wrote: Gah, got the description mixed up with the last patch :-( Be a bit smarter about decoding certificates that might be base64 encoded. First see if it only contains those characters allowed before trying to decode it. This reduces the number of false positives. I'm not sure the test is doing what you want or even if it's the right test. The test is saying If there is one or more characters in the bas64 alphabet then try and decode. That means just about anything will match, which doesn't seem like a very strong test. Why not just try and decode it and let the decoder decide if it's really base64, the decoder has much strong rules about the input, including assuring the padding is correct. The reason is I had a binary cert that was correctly decoded by the base64 encoder. I don't know the why's and wherefores but there it is. Then testing to see if each byte is in the base64 alphabet would not have prevented this error. And yet it did in practice. I think you're assuming too much about the input testing in base64.b64decode(). It gladly takes binary data, as long as it fits the expected padding. You're right, I just went and checked the code, it skips any char not in the base64 alphabet :-( For a while now I've been feeling like we need to associate a format attribute to the certificate (e.g. DER, PEM, BASE64, etc.). There is simply no good way to carry that extra data when all you have is a blob of data. We'd still need some mechanism to look at it and ask what are you? That or we simply reject some types of input. My concern is that correctly deducing what an object is just by scanning it's contents is not robust. As you've seen it's easy to draw the wrong conclusion. Rather if the convention is it must be an object in this format (e.g. canonical) then there is no reason to even ask the question, it's simpler and more robust for most of our (internal) code, we only have to worry about it at the interface boundaries. So who enforces the canonical format? The only place we have to be concerned is when it's user provided, any item we produce will be guaranteed to be in the canonical format (hopefully :-). That just means at our interface boundaries we *must* specify the canonical format. If we're taking input from the user on the command line we offer them the option of input as pem, input as der, input as base64, try to validate as best we can trusting the user has told us the correct format and then convert to the canonical format. Think about the openssl x509 utilities, with those you must specify the input format. If we're taking input through an exposed API we do essentially the same thing. Require the format be passed along with the data, validate as best we can, and convert to the canonical format as it enters our system. BTW, by having the user/caller indicate the format they're providing will make the validation more robust, for example if it's stated the data is in DER format then there is no reason to even try to see if it can be base64 decoded which might lead to a false positive. Likewise if it's stated it's in pem format it must have the header and footer. Bottom line, I'm leery of trying to guess at random points what the format is, it's too easy for the guessing logic to draw the wrong conclusion, I'd much rather see it be explicit. Perhaps but validators take a single argument so there is no way to pass in type. I wasn't thinking the validator would take a type, rather there would be a unique validators associated with the pem arg, the der arg, and the base64 arg. I think we're back at the same issue I was discussing with Pavel concerning having a different arg to pass for files. It seems like we're hit a real issue with the command parameter mechanism. Namely there doesn't seem to be a way to have multiple args that map onto a single command parameter. -- John Dennis jden...@redhat.com Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
