Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trust: Use the same parser code for parsing and checking #483

Merged
merged 3 commits into from
Apr 6, 2023

Conversation

ueno
Copy link
Member

@ueno ueno commented Apr 6, 2023

The trust check-format command used a dedicated parsing code for the .p11-kit format, which behaves differently from the original parsing code: sometimes it is stricter (e.g., PEM block), while other times it is not (e.g., constant names and OIDs). This makes it use the same single parser to align with the original behavior.

@coveralls
Copy link

coveralls commented Apr 6, 2023

Coverage Status

Coverage: 70.386% (+0.1%) from 70.264% when pulling 01c944e on ueno:wip/dueno/trust-check-format into 52f9191 on p11-glue:master.

/* Count newlines in the PEM block */
pos = lexer->at;
for (; pos < end; pos++) {
pos = memchr (pos, '\n', end - pos);

Check notice

Code scanning / CodeQL

For loop variable changed in body

Loop counters should not be modified in the body of the [loop](1).
common/lexer.h Fixed Show fixed Hide fixed
@ueno ueno force-pushed the wip/dueno/trust-check-format branch from 4beacb9 to 4d9f9b5 Compare April 6, 2023 05:43
@ueno
Copy link
Member Author

ueno commented Apr 6, 2023

With this (bogus) input:

[p11-kit-object-v1]
class: bogus-class
label: bogus-label
object-id: 2.5.29.30.X
value: %30%2e%06%03%55%1d%1e%04%27%30%25%a0%23%30%0d%82%0b%65%78%61%6d%70%6c%65%2e%63%6f%6d%30%12%82%10%63%6f%72%70%2e%65%78%61%6d%70%6c%65%2e%63%6f%6d
-----BEGIN PUBLIC KEY-----
MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDRtTajie6qgC9T/RJ1PvN6ntav
+rwcYBBLJoETGlnj/kVsOAQ5J0ZX/dW8jYoQtjvUCoFaRS/sPoHw2U5Pl99LMg8I
sSaivWlhXWY5Yy8QcDX7B4UK/1cSwfSDHfnG06S2cCuAoUB/SE7ZreuAzM+SwdGD
ZAEjR469MZgFa2t8NwIDAQAB
-----END TEST KEY-----

Old:

$ _build/trust/trust check-format bogus.p11-kit
Failed at line 11: label in the END block does not match the label in the BEGIN block
bogus.p11-kit: FAIL

New:

$ _build/trust/trust check-format bogus.p11-kit
p11-kit: bogus.p11-kit:2: class: invalid value
p11-kit: bogus.p11-kit:3: label: invalid value
p11-kit: bogus.p11-kit:4: object-id: invalid value
p11-kit: bogus.p11-kit:5: value: invalid value
p11-kit: bogus.p11-kit:11: BEGIN ...: invalid pem block
bogus.p11-kit: FAIL

@ueno ueno force-pushed the wip/dueno/trust-check-format branch 2 times, most recently from 81455d8 to 002a527 Compare April 6, 2023 06:18
Copy link
Contributor

@ZoltanFridrich ZoltanFridrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just fixup some includes but looks good overall.

trust/persist.h Show resolved Hide resolved
trust/check-format.c Show resolved Hide resolved
ueno added 2 commits April 6, 2023 21:53
The trust check-format command used a dedicated parsing code for the
.p11-kit format, which behaves differently from the original parsing
code: sometimes it is stricter (e.g., PEM block), while other times it
is not (e.g., constant names and OIDs). This makes it use the same
single parser to align with the original behavior.

Signed-off-by: Daiki Ueno <[email protected]>
This adds the new field "line" to p11_lexer so any errors are printed
with the line numbers where they happen.  Also make the header to use
appropriate types for sizes and token types.

Signed-off-by: Daiki Ueno <[email protected]>
@ueno ueno force-pushed the wip/dueno/trust-check-format branch from 002a527 to 6275559 Compare April 6, 2023 13:06
@ueno ueno force-pushed the wip/dueno/trust-check-format branch from 6275559 to 01c944e Compare April 6, 2023 13:12
@ueno ueno merged commit eb6eeb6 into p11-glue:master Apr 6, 2023
@ueno ueno added this to the 0.25.0 milestone Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants