[PATCH] numericoid OIDs validation

[Andrew Bartlett]

On Wed, 2010-10-20 at 14:13 +0300, Kamen Mazdrashki wrote:
> 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)
> 
> Sorry, it became a rather long mail for a small thing but... nevertheless,
> what you thing about above mentioned approaches?

It is almost always worth adding error checks - for the price of an
isdigit() we may save hours or days of debugging strange stuff later.
In particular anywhere near asn.1 I've seen too many security holes to
dare to say 'we don't need to check errors here'.

As to if we should have a validate function that does not need to push
and pull, then yes, that is a reasonable way to implement this
requirement, and may reduce our memory allocation in these very common
code-paths.  

But start with what you have done, and consider taking the remaining
steps after that - we should be checking those errors anyway,
particularly for the other callers, not just in the validation code. 

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Cisco Inc.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20101021/02a03254/attachment.pgp>