mbox series

[0/2] secilc/docs: add syntax highlighting for cil examples

Message ID e8b641c5-4e60-a264-1a4e-0c0b2dd98981@gmail.com (mailing list archive)
Headers show
Series secilc/docs: add syntax highlighting for cil examples | expand

Message

bauen1 Feb. 4, 2021, 5:05 p.m. UTC
Hi,

To make editing the secilc docs easier, I've modified it to use fenced code blocks around all cil examples.
This way editor syntax highlighting can do a better job and as a result I've found 3 minor bracket issues that are also fixed in patch 1.

To allow pandoc to also do syntax highlighting I've written a rudimentary syntax definition in the format consumed by pandoc (https://docs.kde.org/trunk5/en/applications/katepart/highlight.html#katehighlight-xml-format) and enabled it.
However pandocs default themes aren't the best, some of them don't highlight every keyword I've added, color scheme doesn't fit, etc ...
On the other side It's very hard to just find 6 colors that work well together.

I've uploaded an example using the default pandoc theme:
https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide.html
https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide.pdf

bauen1 (2):
  secilc/docs: use fenced code blocks for cil examples
  secilc/docs: add syntax highlighting for secil

 secilc/docs/Makefile                          |  12 +-
 secilc/docs/cil_access_vector_rules.md        |  33 ++-
 secilc/docs/cil_call_macro_statements.md      |  10 +
 .../cil_class_and_permission_statements.md    |  42 ++++
 secilc/docs/cil_conditional_statements.md     |  16 +-
 secilc/docs/cil_constraint_statements.md      |  16 ++
 secilc/docs/cil_container_statements.md       |  16 ++
 secilc/docs/cil_context_statement.md          |  10 +
 secilc/docs/cil_file_labeling_statements.md   |  14 ++
 secilc/docs/cil_infiniband_statements.md      |   9 +-
 secilc/docs/cil_mls_labeling_statements.md    |  50 +++-
 .../docs/cil_network_labeling_statements.md   |  16 ++
 secilc/docs/cil_policy_config_statements.md   |  12 +
 secilc/docs/cil_reference_guide.md            |  27 +++
 secilc/docs/cil_role_statements.md            |  26 ++
 secilc/docs/cil_sid_statements.md             |  12 +
 secilc/docs/cil_type_statements.md            |  50 ++++
 secilc/docs/cil_user_statements.md            |  42 +++-
 secilc/docs/cil_xen_statements.md             |  20 ++
 secilc/docs/secil.xml                         | 224 ++++++++++++++++++
 20 files changed, 644 insertions(+), 13 deletions(-)
 create mode 100644 secilc/docs/secil.xml

--
2.30.0

Comments

James Carter Feb. 4, 2021, 8:17 p.m. UTC | #1
On Thu, Feb 4, 2021 at 12:10 PM bauen1 <j2468h@googlemail.com> wrote:
>
> Hi,
>
> To make editing the secilc docs easier, I've modified it to use fenced code blocks around all cil examples.
> This way editor syntax highlighting can do a better job and as a result I've found 3 minor bracket issues that are also fixed in patch 1.
>
> To allow pandoc to also do syntax highlighting I've written a rudimentary syntax definition in the format consumed by pandoc (https://docs.kde.org/trunk5/en/applications/katepart/highlight.html#katehighlight-xml-format) and enabled it.
> However pandocs default themes aren't the best, some of them don't highlight every keyword I've added, color scheme doesn't fit, etc ...
> On the other side It's very hard to just find 6 colors that work well together.
>
> I've uploaded an example using the default pandoc theme:
> https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide.html
> https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide.pdf
>

I notice on page 10:
(block unconfined
    (user user)
   ...

That the second user (which is the name of the user) is highlighted as
well as the user keyword.

Similar thing happens further done on page 10 with the rule:
(portcon udp 12345 (unconfined.user object_r unconfined.object ((s0) (s0(c0)))))

The "user" part of "unconfined.user" is highlighted.

"unconfined.user" is used in other examples as well.

Changing the first statement to be (user user1) would be fine, but I
would like "unconfined.user" to remain as is.
I am not sure how hard it would be to fix that.

I am not sure if this matters to anyone, but if the document is
printed, the syntax highlighting (particularly for the comments) might
make it a little bit hard to read.

Thanks,
Jim

> bauen1 (2):
>   secilc/docs: use fenced code blocks for cil examples
>   secilc/docs: add syntax highlighting for secil
>
>  secilc/docs/Makefile                          |  12 +-
>  secilc/docs/cil_access_vector_rules.md        |  33 ++-
>  secilc/docs/cil_call_macro_statements.md      |  10 +
>  .../cil_class_and_permission_statements.md    |  42 ++++
>  secilc/docs/cil_conditional_statements.md     |  16 +-
>  secilc/docs/cil_constraint_statements.md      |  16 ++
>  secilc/docs/cil_container_statements.md       |  16 ++
>  secilc/docs/cil_context_statement.md          |  10 +
>  secilc/docs/cil_file_labeling_statements.md   |  14 ++
>  secilc/docs/cil_infiniband_statements.md      |   9 +-
>  secilc/docs/cil_mls_labeling_statements.md    |  50 +++-
>  .../docs/cil_network_labeling_statements.md   |  16 ++
>  secilc/docs/cil_policy_config_statements.md   |  12 +
>  secilc/docs/cil_reference_guide.md            |  27 +++
>  secilc/docs/cil_role_statements.md            |  26 ++
>  secilc/docs/cil_sid_statements.md             |  12 +
>  secilc/docs/cil_type_statements.md            |  50 ++++
>  secilc/docs/cil_user_statements.md            |  42 +++-
>  secilc/docs/cil_xen_statements.md             |  20 ++
>  secilc/docs/secil.xml                         | 224 ++++++++++++++++++
>  20 files changed, 644 insertions(+), 13 deletions(-)
>  create mode 100644 secilc/docs/secil.xml
>
> --
> 2.30.0
>
bauen1 Feb. 4, 2021, 9:28 p.m. UTC | #2
On 2/4/21 9:17 PM, James Carter wrote:
> I notice on page 10:
> (block unconfined
>     (user user)
>    ...
> 
> That the second user (which is the name of the user) is highlighted as
> well as the user keyword.
> 
> Similar thing happens further done on page 10 with the rule:
> (portcon udp 12345 (unconfined.user object_r unconfined.object ((s0) (s0(c0)))))
> 
> The "user" part of "unconfined.user" is highlighted.
> 
> "unconfined.user" is used in other examples as well.
> 
> Changing the first statement to be (user user1) would be fine, but I
> would like "unconfined.user" to remain as is.
> I am not sure how hard it would be to fix that.
> 

I thought this would be harder, but just highlighting the first cil keyword in a block is actually very easy, and I can rework the patch to do just that.

It becomes more difficult when trying to add (some) highlighting to everything else since keywords are reused as names very often (I do that by design even):

Some examples from the guide:

(macro all ((type x))
    (allow x bin_t (policy.file (execute)))
    (allowx x bin_t (ioctl policy.file (range 0x1000 0x11FF)))
)
(call all (bin_t))

(mlsvalidatetrans file (domby l1 h2))

(defaultrole char target)

(type t1)
(allow bb.t2 bb.t1 (policy.file (read write execute))))))

In the above e.g. all is name, but is usually a keyword with a very important meaning, so imho it should be highlighted, in a lesser way this also goes for t1, or domby, ...

The only way I avoid highlighting _all_ names as keywords is to implement a lot of the CIL grammar in the syntax highlighter, but I'm not really sure if it's worth the effort.
It could be done for some keywords, e.g. constraints, filecon.

A better alternative might be to just highlight less, e.g. drop `low`, `low-high` keywords entirely.

I've uploaded another version with some small fixes and a debug color theme to better show what-is-what:

https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide2.html

and with more keywords removed:

https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide3.html

; only in version 2: file is considered a keyword (builtin), but this should normally only be the case in filecon statements, here it references a class
(mlsvalidatetrans file (domby l1 h2))

; all, t1, low is considered a keyword (builtin), but is a name
(call all (bin_t))
(type t1)
(userlevel u5 low)

; but here t2, t1 are no longer highlighted erroneously.
(allow bb.t2 bb.t1 (policy.file (read write execute)))

I think Version 3 only has mismatches in the example policy due to usage of `all`, `t1` and `t2` as names, so that might be the way to go.

> I am not sure if this matters to anyone, but if the document is
> printed, the syntax highlighting (particularly for the comments) might
> make it a little bit hard to read.

pandoc can still be run without syntax highlighting (--no-highlight) if you want to actually print the document in greyscale. 

It also looks like I messed up when sending the patches, patch 1, the least problematic one ironically, got lost somewhere.
James Carter Feb. 4, 2021, 11:19 p.m. UTC | #3
On Thu, Feb 4, 2021 at 4:28 PM bauen1 <j2468h@googlemail.com> wrote:
>
> On 2/4/21 9:17 PM, James Carter wrote:
> > I notice on page 10:
> > (block unconfined
> >     (user user)
> >    ...
> >
> > That the second user (which is the name of the user) is highlighted as
> > well as the user keyword.
> >
> > Similar thing happens further done on page 10 with the rule:
> > (portcon udp 12345 (unconfined.user object_r unconfined.object ((s0) (s0(c0)))))
> >
> > The "user" part of "unconfined.user" is highlighted.
> >
> > "unconfined.user" is used in other examples as well.
> >
> > Changing the first statement to be (user user1) would be fine, but I
> > would like "unconfined.user" to remain as is.
> > I am not sure how hard it would be to fix that.
> >
>
> I thought this would be harder, but just highlighting the first cil keyword in a block is actually very easy, and I can rework the patch to do just that.
>
> It becomes more difficult when trying to add (some) highlighting to everything else since keywords are reused as names very often (I do that by design even):
>
> Some examples from the guide:
>
> (macro all ((type x))
>     (allow x bin_t (policy.file (execute)))
>     (allowx x bin_t (ioctl policy.file (range 0x1000 0x11FF)))
> )
> (call all (bin_t))
>
> (mlsvalidatetrans file (domby l1 h2))
>
> (defaultrole char target)
>
> (type t1)
> (allow bb.t2 bb.t1 (policy.file (read write execute))))))
>
> In the above e.g. all is name, but is usually a keyword with a very important meaning, so imho it should be highlighted, in a lesser way this also goes for t1, or domby, ...
>
> The only way I avoid highlighting _all_ names as keywords is to implement a lot of the CIL grammar in the syntax highlighter, but I'm not really sure if it's worth the effort.
> It could be done for some keywords, e.g. constraints, filecon.
>
> A better alternative might be to just highlight less, e.g. drop `low`, `low-high` keywords entirely.
>
> I've uploaded another version with some small fixes and a debug color theme to better show what-is-what:
>
> https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide2.html
>
> and with more keywords removed:
>
> https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide3.html
>
> ; only in version 2: file is considered a keyword (builtin), but this should normally only be the case in filecon statements, here it references a class
> (mlsvalidatetrans file (domby l1 h2))
>
> ; all, t1, low is considered a keyword (builtin), but is a name
> (call all (bin_t))
> (type t1)
> (userlevel u5 low)
>
> ; but here t2, t1 are no longer highlighted erroneously.
> (allow bb.t2 bb.t1 (policy.file (read write execute)))
>
> I think Version 3 only has mismatches in the example policy due to usage of `all`, `t1` and `t2` as names, so that might be the way to go.
>

I like version 3 the best, but I really don't like the color used for
"self", "object_r", "h1" , etc. It just stands out too much.

Thanks,
Jim

> > I am not sure if this matters to anyone, but if the document is
> > printed, the syntax highlighting (particularly for the comments) might
> > make it a little bit hard to read.
>
> pandoc can still be run without syntax highlighting (--no-highlight) if you want to actually print the document in greyscale.
>
> It also looks like I messed up when sending the patches, patch 1, the least problematic one ironically, got lost somewhere.
>
> --
> bauen1
> https://dn42.bauen1.xyz/
Nicolas Iooss Feb. 5, 2021, 9:31 a.m. UTC | #4
On Fri, Feb 5, 2021 at 12:20 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Thu, Feb 4, 2021 at 4:28 PM bauen1 <j2468h@googlemail.com> wrote:
> >
> > On 2/4/21 9:17 PM, James Carter wrote:
> > > I notice on page 10:
> > > (block unconfined
> > >     (user user)
> > >    ...
> > >
> > > That the second user (which is the name of the user) is highlighted as
> > > well as the user keyword.
> > >
> > > Similar thing happens further done on page 10 with the rule:
> > > (portcon udp 12345 (unconfined.user object_r unconfined.object ((s0) (s0(c0)))))
> > >
> > > The "user" part of "unconfined.user" is highlighted.
> > >
> > > "unconfined.user" is used in other examples as well.
> > >
> > > Changing the first statement to be (user user1) would be fine, but I
> > > would like "unconfined.user" to remain as is.
> > > I am not sure how hard it would be to fix that.
> > >
> >
> > I thought this would be harder, but just highlighting the first cil keyword in a block is actually very easy, and I can rework the patch to do just that.
> >
> > It becomes more difficult when trying to add (some) highlighting to everything else since keywords are reused as names very often (I do that by design even):
> >
> > Some examples from the guide:
> >
> > (macro all ((type x))
> >     (allow x bin_t (policy.file (execute)))
> >     (allowx x bin_t (ioctl policy.file (range 0x1000 0x11FF)))
> > )
> > (call all (bin_t))
> >
> > (mlsvalidatetrans file (domby l1 h2))
> >
> > (defaultrole char target)
> >
> > (type t1)
> > (allow bb.t2 bb.t1 (policy.file (read write execute))))))
> >
> > In the above e.g. all is name, but is usually a keyword with a very important meaning, so imho it should be highlighted, in a lesser way this also goes for t1, or domby, ...
> >
> > The only way I avoid highlighting _all_ names as keywords is to implement a lot of the CIL grammar in the syntax highlighter, but I'm not really sure if it's worth the effort.
> > It could be done for some keywords, e.g. constraints, filecon.
> >
> > A better alternative might be to just highlight less, e.g. drop `low`, `low-high` keywords entirely.
> >
> > I've uploaded another version with some small fixes and a debug color theme to better show what-is-what:
> >
> > https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide2.html
> >
> > and with more keywords removed:
> >
> > https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide3.html
> >
> > ; only in version 2: file is considered a keyword (builtin), but this should normally only be the case in filecon statements, here it references a class
> > (mlsvalidatetrans file (domby l1 h2))
> >
> > ; all, t1, low is considered a keyword (builtin), but is a name
> > (call all (bin_t))
> > (type t1)
> > (userlevel u5 low)
> >
> > ; but here t2, t1 are no longer highlighted erroneously.
> > (allow bb.t2 bb.t1 (policy.file (read write execute)))
> >
> > I think Version 3 only has mismatches in the example policy due to usage of `all`, `t1` and `t2` as names, so that might be the way to go.
> >
>
> I like version 3 the best, but I really don't like the color used for
> "self", "object_r", "h1" , etc. It just stands out too much.
>
> Thanks,
> Jim

Thanks for working on this. On my side, I have a minor comment,
related to the second patch:

+    <list name="keywords">
+        <item>auditallow</item>
+        <item>tunableif</item>
+        <item>allow</item>
+        <item>dontaudit</item>
...
I find it easier to maintain if the items were sorted in alphabetical
order (this enables inserting new items if the need comes one day,
without wondering whether the new items should be at the end of the
list or if the order should match the one used in some source
files...). Or, if you want to keep this order, please add comments
describing how the lists were created, in order to ease future
modifications.

Thanks,
Nicolas
bauen1 Feb. 6, 2021, 8:29 p.m. UTC | #5
On 2/5/21 12:19 AM, James Carter wrote:
> On Thu, Feb 4, 2021 at 4:28 PM bauen1 <j2468h@googlemail.com> wrote:
>
>>
>> https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide3.html
>>
> 
> I like version 3 the best, but I really don't like the color used for
> "self", "object_r", "h1" , etc. It just stands out too much.

This was originally meant to be a test color scheme to showcase all different "classes" of keywords that could be highlighted, not intended as an actual color theme.

I've changed the purple to a lighter pink:

https://glados.bauen1.xyz/misc_stuff_might_disappear/CIL_Reference_Guide4.html

In case someone can come up with 6 different colors that go well together, I can put together an updated theme (Colors for Strings+SpecialChars, Function, Operator, BuiltIns, Comment, Keyword).

I will send a v2 of the patch, including the color theme as a separate patch.

> ...
> I find it easier to maintain if the items were sorted in alphabetical
> order (this enables inserting new items if the need comes one day,
> without wondering whether the new items should be at the end of the
> list or if the order should match the one used in some source
> files...). Or, if you want to keep this order, please add comments
> describing how the lists were created, in order to ease future
> modifications.

Thanks for the feedback, will be implemented.