fix: encrypt an auth file after the fact, and stop asking for what is thrown away - #315
Merged
Merged
Conversation
`manage auth-file add` could always write an encrypted file, through `--password`. That option was the only one of the command without a prompt, so the questions it asks -- file name, audible user, password, marketplace -- walked past the one about encryption, and an answer given to all of them produced an auth file in the open. `quickstart` asks. It asks now too, with an empty answer meaning no password. A script that passed every other option has to pass `--password ''` as well from here on, to say that it wants none.
Three helpers next to `build_auth_file`, for the commands that work on an auth file that already exists. `detect_auth_file` says whether a file is a plain auth file, an encrypted one in either format, or none of those. The library's `detect_file_encryption` decides it from a single key and a single exception: a json document holding `adp_token` is unencrypted, one holding `ciphertext` is encrypted as json, and anything that is not json at all is called encrypted as bytes. A file of nonsense therefore reads as encrypted and a json document of something else reads as neither. This one asks what is there: the four keys the json encryption writes, one of the two field pairs that make a document an auth file, or the salt header and whole cipher blocks of the older format. `rewrite_auth_file` writes the file again with or without a password. The file carries the registration of a device, and half of one is only recoverable by registering again, so the content goes to a name from `mkstemp` in the same directory, is flushed to disk, and takes the place of the old file in a single step, with the permissions it had. `read_auth_file` reads one and reports what is wrong with it instead of raising it. The library validates field by field and raises whichever error fits the field it looked at first, so `remove` used to answer a wrong password with a traceback about PKCS7 padding.
Encryption could only be chosen while a device was being registered. An
auth file that was written in the open stayed that way, short of
deregistering the device and starting over.
audible manage auth-file encrypt -f Second.json -p <password>
audible manage auth-file decrypt -f Second.json -p <password>
Neither asks for anything. Both the file and the password come from the
command line, so a script can use them; the price is that the password
is in the shell history and, while the command runs, in the process
list, which the help says.
What they refuse rather than do:
- a file that is not an auth file, or one already in the shape the
command would produce
- an empty password. `required=True` only asks for the option to be
present, and an empty value would read as "no password" further down
and write the file in the open while reporting success
- a file with a second hard link, because the rewrite gives this name a
new file and every other name would keep pointing at the readable one
A symlink is followed to what it points at. Replacing the link itself
would put an encrypted file where the link was and leave the credentials
where they are.
`manage auth-file add --external-login` hands the login to a browser, which asks for whatever it needs itself. The command asked for an audible username and password all the same, and threw them away: `build_auth_file` passes them to `Authenticator.from_login`, and the external branch calls `from_login_external`, which takes the marketplace and a callback. The two options lose their prompts and are asked for in the command instead, only when the login happens here. Closes #314.
The file is the registration of a device, so most of these ask what happens when something goes wrong: a wrong password, an empty one, a file that is not an auth file, one that is not json at all, a field of the wrong type, a second hard link, a symlink, a write that fails half way, and a file already sitting where the temporary one would go. Each one checks that the auth file still holds what it held. `detect_auth_file` is exercised directly as well, with the byte strings that separate the older encryption from a file of nonsense: its salt header and its whole cipher blocks are all there is to go by.
Both commands took the file and the password from the command line only, so using them meant leaving the password in the shell history. They ask for what is not given now, as `remove` already asks for the file name, and a script that passes both options is asked nothing. `encrypt` repeats the password back, because a typo there would lock the file with something nobody knows. `decrypt` does not: a wrong password cannot destroy anything, it only fails to open the file. An empty answer is refused the same way an empty option is.
Three corrections to the same helper, each one found by pointing a file at it that a user could have: - an encrypted file without `info` was called no auth file at all. The library writes that key along with the encryption but reads only `salt`, `iv` and `ciphertext` back, so such a file still opens. - a document carrying the field names with nothing in them was called plain. `read_auth_file` caught it a step later, with a message about the contents rather than about the file. - a rename is a change to the directory holding it, and is only on disk once the directory is. The file was flushed, the directory was not. The question about a password for a new auth file said "or enter for none", while `confirmation_prompt` asks a second time before it takes an empty answer. It no longer promises one keypress.
mkb79
force-pushed
the
fix/auth-file-handling
branch
from
September 1, 2026 04:10
1e69cd6 to
e34679a
Compare
Both commands went through `Authenticator`: read the file into one, hand it back to `to_file` with or without a password. That put the library's parser and serialiser in the path of an operation that has nothing to ask either of them. `AESCipher` works on the text instead. What comes back out of a round trip is what went in, down to the key order and any field the library has no name for, where before it was whatever the serialiser made of what the parser had understood. A file with a field the parser rejects can be encrypted now, because whether the contents would make a working login is not the question being asked. And neither command has to know which marketplace a file belongs to, which `Authenticator` insists on being told when the file does not say. What is still checked is what belongs to this operation: the shape of the file before it is touched, and after decrypting, that the result is json holding credentials -- otherwise decrypting somebody else's encrypted file would lay its contents out in the open.
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.
Closes #313
Closes #314
Two reports about the same command, from the same person, in the same week.
auth-file addnever offered encryption (#313)--passwordonmanage auth-file addhas always written an encrypted file. Itwas the only option of that command without a prompt, so the questions it asks
— file name, audible user, password, marketplace — walked straight past the one
about encryption, while
quickstartasks outright. Anyone who used the commandthe way it is meant to be used got an auth file in the open and no hint that it
could be otherwise.
It asks now, with an empty answer meaning no password. A script that passed
every other option has to pass
--password ''as well from here on.auth-file add --external-loginasked for an account it never used (#314)build_auth_filehands the username and password toAuthenticator.from_login. With--external-loginit takes the other branch,from_login_external, which gets the marketplace,with_usernameand thecallback that opens the browser. The two values were asked for and dropped.
They lose their prompts and are asked for in the command instead, only when the
login happens here.
Encrypting an auth file that already exists (#313)
Whatever is not given is asked for, so a password need not be typed where the
shell keeps it, and a script that passes both options is asked nothing.
encryptrepeats the password back, because a typo there would lock the filewith something nobody knows;
decryptdoes not, because a wrong password onlyfails to open the file.
Both work on the text of the file through
AESCipher, not throughAuthenticator: putting a password on a file is a thing done to the file, notto the account behind it. A round trip therefore gives back what went in, down
to the key order and any field the library has no name for, and neither command
has to know which marketplace the file belongs to.
An auth file is the registration of a device, and half of one can only be
replaced by registering again. So:
mkstempin the same directory, isflushed to disk, and takes the place of the old file in one step, with the
permissions it had. The directory is flushed too, because a rename is a
change to the directory and is only durable once that is written out
put an encrypted file where the link was and leave the credentials where
they are
every other name pointing at the readable one
question. It would read as "no password" further down and write the file in
the open while reporting success
Telling an auth file from anything else
audible.aescipher.detect_file_encryptiondecides this from one key and oneexception: a json document holding
adp_tokenis unencrypted, one holdingciphertextis encrypted as json, and anything that is not json at all iscalled encrypted as bytes. So a file of nonsense reads as encrypted, and a json
document of something else reads as neither — and both then reach a command
that would write over them.
detect_auth_fileasks what is actually there:FalseplainjsonjsonbytesbytesNoneNonebytesNonebytesNoneaccess_tokenandrefresh_tokenNoneplainThe older format needs no password to recognise: it opens with the salt header
— the marker, the number of kdf iterations as a big-endian word, the marker
again — and what follows is an iv and whole cipher blocks.
Tests
34 new ones, 42 cases with the parametrised ones counted. Most ask what happens
when something goes wrong and check that the auth file still holds what it
held: a wrong password, an empty one, a file that is not an auth file, one that
is not json at all, a field of the wrong type, a second hard link, a symlink, a
write that fails half way, and a file already sitting where the temporary one
would go.
detect_auth_fileis exercised directly as well, with the byte strings thatseparate the older encryption from a file of nonsense.