Re: [ros-dev] Can anybody commit this patch ?

[Alex Ionescu]

Javier Muñoz Mellid wrote:

>   Hello Alex,
>
>  I have a very bad english so give me a chance if you don't understand 
>  all my words :(
>
>  This is my first contact with ReactOS development. I am looking in 
>  code and learning a lot of things here. I want to contribute and i 
>  think that  i am a "acceptable" reverser so i followed a blackbox 
>  approach with this patch byte to byte. 
I don't have anything against that. Btw, you missed a call to 
SeCaptureSubjectContext in SeCreateAccessState.

>  
> >   And I would very much appreciate to know
> >
> >  1) Why you had to reverse an opaque structure:
> >     a) It's easy to guess the layout since it was created in NT4 to 
> >  manage something new added post NT 3.5.1
> >     b) There's no point in cloning something so opaque that it's not 
> >  even in the symbols, since nobody could possibly be using it. 
>
>  You're right but i think that if we get the best match against opaque 
>  structures we aren't going to have to change (a lot of) code in the 
>  future to adjust "undocumented" drivers by Msoft or 3rd parties. It is 
>  my opinion only. For example, i am viewing some drivers incorporating 
>  undocumented calls and structures from books like "Undocumented NT" 
>  and similar. If we know those structures i think that we can add them. 
>  It isn't a design problem and it is only a future's choice. 
I totally agree with this as well, this has always been my opinion. But 
the AUX_DATA structure has been specifically opaqued by Microsoft, and 
never published anywhere.

>  
> >   2) How you knew that the third member of that
> > structure (or that it 
> >  even exists) is an ACCESS_MASK called AccessesToAudit. 
>
>  When i began to reverse SeCreateAccessState it only touch 
>  PrivilegedUsed and GenericMapping 
Exactly.

>   so i get the types and sizes then i mail out my
> question in our 
>  University list at Coruna. I was replied with the structure that i add 
>  in the .h It's similar to previous choice. I only need two fields but 
>  i was provided with a structure and it has a better match that mine so 
>  i add the second (Copy&Paste) 
You ended up adding a structure from the Windows Source Code into your 
patch. Perhaps your university has legal access to it through the 
Microsoft Shared Source Code Initiative.

>  
> >   I've looked at the functions you implemented
> > and it isn't used 
> >  anywhere. I've looked with IDA at the binaries, and it's not used 
> >  anywhere either. 
>
>  Alex, i read TODO and Security is a beautiful field to me. I grep the 
>  unimplemented functions and i found three easy funtions in access.c 
>  They were a good choice because they aren't touched for more 
>  experienced programmers and so i could implement freely. They look 
>  like basic stones to more complicated functions so i can continue 
>  adding code in my possibilities. 
I know they are easy, I was talking so one of the developers about 
implementing them; that's not the point.

>  
>  If you want i can attach in this list my SeCreateAccessState's 
>  dead-listing from Windows XP no-sp (Spanish version). 
No, it's ok.

>  
>  -Javier
>  ______ 
The problem which I have is that the binary only accesses the first two 
members of that structure. There is no way anyone could've known the 
function of the third member ( I didn't even think/know one existed) 
since it is currently unused, even in Windows Server 2003. Therefore I 
must conclude it was added from internal microsoft headers which were 
emailed to you.

Best regards,
Alex Ionescu