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

Cleanup cert-id rules #386

Merged
merged 22 commits into from
Feb 6, 2024
Merged

Cleanup cert-id rules #386

merged 22 commits into from
Feb 6, 2024

Conversation

J08nY
Copy link
Member

@J08nY J08nY commented Feb 2, 2024

The cert-id rules work, but are a mess. This PR will clean them up for presentation in the paper.

Before this, the regular cert_id regexes were used to extract
the cert_id from the report filename. However, the filenames often
do not use the same cert_id format, but contain all of the information
necessary to reconstruct the cert_id, but with different order for example.

This commit along with those before it introduce a new set of regular
expressions that better match the ones in the filenames. To extract
the correctly formatted canonical cert_id, the regexes are used to
obtain the parts of the cert_id (using named groups in regexes) and
those are then reconstructed into a canonical version of the cert_id
via one of the scheme-dependent functions.

@J08nY J08nY added the cc Related to CC certification label Feb 2, 2024
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

Attention: 40 lines in your changes are missing coverage. Please review.

Comparison is base (be702d4) 69.36% compared to head (40d89ff) 68.54%.
Report is 13 commits behind head on main.

❗ Current head 40d89ff differs from pull request most recent head ec0162c. Consider uploading reports for the commit ec0162c to get more accurate results

Files Patch % Lines
src/sec_certs/sample/cc_certificate_id.py 85.99% 22 Missing ⚠️
src/sec_certs/sample/cc.py 8.34% 11 Missing ⚠️
src/sec_certs/sample/cc_scheme.py 73.92% 6 Missing ⚠️
src/sec_certs/configuration.py 83.34% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #386      +/-   ##
==========================================
- Coverage   69.36%   68.54%   -0.82%     
==========================================
  Files          62       62              
  Lines        7457     7535      +78     
==========================================
- Hits         5172     5164       -8     
- Misses       2285     2371      +86     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Before this, the regular cert_id regexes were used to extract
the cert_id from the report filename. However, the filenames often
do not use the same cert_id format, but contain all of the information
necessary to reconstruct the cert_id, but with different order for example.

This commit along with those before it introduce a new set of regular
expressions that better match the ones in the filenames. To extract
the correctly formatted canonical cert_id, the regexes are used to
obtain the parts of the cert_id (using named groups in regexes) and
those are then reconstructed into a canonical version of the cert_id
via one of the scheme-dependent functions.
@J08nY J08nY merged commit 2fc72fc into main Feb 6, 2024
4 checks passed
@J08nY J08nY deleted the feat/cert-id-cleanup branch February 6, 2024 18:21
@J08nY
Copy link
Member Author

J08nY commented Feb 7, 2024

Now that this is merged and I did a full run I will use this PR for some insights/issues.

  • CA rule causes many false positives like 1 1 1 or 2021-02-13.
  • Some US ids are missed due to bad ? placement in regex.
  • The number of certificates without ID increased a bit, but I think that is due to us being overly eager before and actually matching stuff we should not have. Though a proper evaluation should be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cc Related to CC certification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant