[PATCH] numericoid OIDs validation

[Kamen Mazdrashki]

Hi Metze,

Thanks for joining in :)

On Wed, Oct 20, 2010 at 15:38, Stefan (metze) Metzmacher
<metze at samba.org> wrote:
> Am 20.10.2010 13:13, schrieb Kamen Mazdrashki:
>> Hi Tridge,
>>
>> I am currently working on a prefixMap corruption in S4.
>> I am going to change OIDs validation function in dsdb/schema_syntax.c
>> so that numericoid OIDs are validated in a different way than using
>> prefixMap/Schema to validate those.
>> I want to use ber_write_*/ber_read_* functions to do the validation
>> and while implementing this I've noticed that ber_write_OID_String()
>> function is not tested for error cases. So I did :).
>> Please check at:
>> http://git.samba.org/?p=kamenim/samba.git;a=shortlog;h=refs/heads/pfm-save-review
>>
>> But what I am wondering now is, should I push this patch at all.
>> It will make ber_write_OID_String() a little bit slower with the added value
>> of being more robust against invalid OIDs.
>> Perhaps it is good thing for the function, but I am afraid it is very often
>> used with OIDs that are for sure a valid OIDs.
>>
>> (2) Another way is to implement just OID_validation() function
>> that is to be used when validating numeric OIDs (usually when we add new
>> objects in Schema).
>>
>> (3) And yet another way I could think of is let syntax->validate() functions
>> to validate those and just make sure they do really validate correctly by
>> implementing unit-test for dsdb_syntax syntaxes.
>> (validate() functions usually do write/read/compare, so they should
>> fail on invalid OIDs even when ber_write_OID_() doesn't fail)
>
> Shouldn't the schema_data_add() function make sure that we have
> the new prefix map already, if not this is the place to add it.
> I think we also need a schema_data_modify() function...
>
Right. The problem comes way before schema_data module.
It comes from objectclass_attr.c module that validates values passed.
And it validates those values by calling dsdb_syntax->validate().
Currently schema_data_add() works just as you described.

I don't think we should do the same in schema_data_modify() handler
as I don't think we can change attributeID or governsID in the schema?

> The syntax->validate() function should keep validating against the schema.
>
Yes. It still validates against schema, but not numericoid OIDs.
This patch covers only governsID, attributeID, attrributeSyntax OIDs.
We should not validate those against our current schema, as we can't do
this in case we are adding new attributeSchema or classSchema object.
(may be should implement more thorough validation for attributeSyntax though,
as syntaxes are still a finite set... hm).
So, in case of numericoid OID, I think validate should check it is valid OID.
schema_data.c module will validate it against prefixMap and add it to prefixMap
(doing all necessary steps involved in this process)


-- 
CU,
Kamen Mazdrashki
Samba Team                                            http://samba.org
http://gitweb.samba.org/?p=kamenim/samba.git;a=summary