Hi,
I have reviewed your patch. I have tested it on my Ubuntu Server 16.04.3, and
it works great.
The patch overall looks good, but I have a few minor findings:
src/OVAL/probes/unix/linux/apparmorstatus.c :
* It shouldn't fail hard if you don't run it as root. In general OpenSCAP
doesn't terminate
just because it couldn't get some data. I suggest `return 0` on line 362.
* Please use `snprintf` instead of `sprintf`. It's safer.
* On line 320, SEXP_free(item) shouldn't be commented out. But please make sure
everything is freed properly then.
* Please don't mix spaces and tabs. Tabs should be used to indent everywhere in
C code.
* On line 200 you close a file that was already closed on line 177.
* I guess that the sizeof operator on line 273 should take "struct aa_profile".
* Compiler reports some warnings:
```
unix/linux/apparmorstatus.c: In function ‘probe_main’:
unix/linux/apparmorstatus.c:330:24: warning: variable ‘over’ set but not used
[-Wunused-but-set-variable]
oval_schema_version_t over;
^
unix/linux/apparmorstatus.c: In function ‘aa_isenabled’:
unix/linux/apparmorstatus.c:90:2: warning: ignoring return value of ‘read’,
declared with attribute warn_unused_result [-Wunused-result]
(void) read(fd, &status, 1);
```
Regards
Jan Černý
Security Technologies | Red Hat, Inc.
----- Original Message -----
> From: "Jan Cerny" <jce...@redhat.com>
> To: "Bruno Ducrot" <br...@poupinou.org>
> Cc: open-scap-list@redhat.com
> Sent: Friday, September 15, 2017 10:26:28 AM
> Subject: Re: [Open-scap] Implementation for an AppArmor probe.
>
> Hi,
>
> The new patch looks great. I'll review and test. I'll let you know.
>
> Thanks
>
> Regards
>
> Jan Černý
> Security Technologies | Red Hat, Inc.
>
> ----- Original Message -----
> > From: "Bruno Ducrot" <br...@poupinou.org>
> > To: "Jan Cerny" <jce...@redhat.com>
> > Cc: open-scap-list@redhat.com, "William Munyan"
> > <william.mun...@cisecurity.org>
> > Sent: Monday, September 11, 2017 6:18:59 PM
> > Subject: Re: [Open-scap] Implementation for an AppArmor probe.
> >
> > Hi Jan,
> >
> > On Mon, Sep 11, 2017 at 09:44:40AM -0400, Jan Cerny wrote:
> > > Hi Bruno,
> > >
> > > this is awesome.
> > >
> > > However, as Bill pointed out, AppArmor support was added to OVAL standard
> > > in version 5.11.2.
> >
> > Indeed.
> >
> > >
> > > If you remove the schema changes of 5.11.0 it would be better.
> > > We already have 5.11.2 schemas in the repository, so it should be enough
> > > to change the version in your OVAL files.
> > > I think we shouldn't add any custom extensions to the schemas in
> > > schemas/oval
> > > directory in OpenSCAP repository. One of the use-cases of oscap is to
> > > verify whether the content complies with OVAL standard, which would be
> > > broken with the patch :-)
> > >
> > > Also, since AppArmor probe is in Linux namespace, I don't see a need to
> > > create any new options in ./configure. The probes aren't Red Hat
> > > specific.
> > > For example we have DPKG info probe, which is used only on Ubuntu and
> > > Debian,
> > > and we don't have a special option for that. It just doesn't compile the
> > > probe
> > > binary on RHEL/Fedora. I think AppArmor probe is a similar case.
> >
> > Ok. But there is no real library dependancies, so it will be
> > compiled under systems without AppArmor.
> >
> > The next iteration can be found here :
> > http://poupinou.org/SCAP/openscap-apparmor-20170911.diff
> >
> > That one is against current git, instead of 1.2.15. I'm planing to clone
> > the
> > openscap
> > git, just in case I'll have to do more stuff.
> >
> > There is still the unit tests to be written though. I hope doing so
> > this week, but I'm a bit busy atm.
> >
> > >
> > > Overall, I think that there is a very high chance to include the probe to
> > > upstream.
> > > I'm looking forward to your contributions.
> >
> > Thanks !
> >
> >
> > --
> > Bruno Ducrot
> >
> > -- Which is worse: ignorance or apathy?
> > -- Don't know. Don't care.
> >
>
> _______________________________________________
> Open-scap-list mailing list
> Open-scap-list@redhat.com
> https://www.redhat.com/mailman/listinfo/open-scap-list
_______________________________________________
Open-scap-list mailing list
Open-scap-list@redhat.com
https://www.redhat.com/mailman/listinfo/open-scap-list
