glenrsmith / phibes Goto Github PK

For some users, the workflow of having the CLI create-item command launch an editor may not be ideal. In particular, those whose preferred editor doesn't block the execution of a calling process may be frustrated.

For them (and possibly other reasons), it would be useful to have a command-line option to just add from the contents of an existing file.

Attempting to delete an anonymous locker with filesystem storage fails because the locker directory is not empty.

Need to create a test that exposes this problem and then fix the problem.

Earlier in development, I had in place a model where all items could be stored in plaintext, and the user could keep one or both copies around. There were use cases for creating the encrypted version from the plaintext version, and vice versa.

The main reason I dropped this was that I want to build the app in a way that minimizes the program flows where the plaintext "exists", especially for consideration of deploying as a web service so the owner of the sensitive data is no longer the owner of the storage where it resides.

At some point I'd like to revisit the concept, even if only to fill in holes in the cli to maximize the ability of a user to accomplish the basic tasks using native shell tools, e.g. piping decryption requests to file, piping file to encryption requests.

Current manual method is ok for a minute, but there should be an automated process to build and publish, like a github action that triggers on merges with release tags or a changed version.txt file or something.

Commands that expose add/edit item accept a --editor option, but the option is currently ignored, and only the values in environment variables PHIBES_EDITOR and EDITOR are used.

Need to create a test that exposes this issue, and then fix it.

Per @criswell, the CLI should have support for more "plumbing" commands, vs. "porcelain", in the git sense of the terms, to make it more scriptable. #43 is probably a taste of that.

This issue will be a place to flesh out what this means.

When the user specifies an editor via cli option, that's what's supposed to be used.

Need a test to expose this defect and then a fix.

Currently, there is zero logging in the project.

The only visibility is by raising exceptions.

This isn't the behavior I want:
python-poetry/poetry#2282

Not mentioned in that thread is that poetry actually deletes the previous venv.

Back to pip and setup.

Currently doc strings are incomplete and inconsistent (e.g. format for labeling function arguments)

Might consider using something like pydocstyle eventually

If lock file creation fails, the user shouldn't be left with a directory for the locker.

This also points up the dearth of such calls in testing.

The motivation for Template was to allow the user to start the creation process of a Secret with content from some other item they had created and stored in their locker. This can easily be accomplished by allowing the user to specify any item as a starter. They can also designate for themselves which items in the system are "templates" by using a Tag.

The motivation for Schema was the idea that users might want to have enforcement of content in items that have some form of structured text, e.g. JSON. The effort to design and develop a system that enables such a feature native to the application isn't anywhere on my radar. Moreover, the hooks for it would be entirely in the plain-text, pre-submit stages, and so could be much better addressed orthogonally. Finally, such a feature seems to me to make a lot more sense in a context with a GUI, so maybe will be reserved for a web app.

(This has really been an issue I've worried over from the outset of the project; as I've started trying to implement a solution recently, as it kind of blocks #72, best to get it documented.)

CLI users would be able to perform a series of operations without passing (or being prompted for) a password if the app supported a 'session'. This would also simplify #72.

One way to implement this would be provide an interactive shell (so that credentials remain in memory between commands), but that would block support for desired things like piping between OS commands and phibes commands.

So phibes must be able to find something that enables interaction with the locker, meaning it has to be able to get/derive the encryption key. It should not be able to find that something on disk, which leaves environment variables.

The most direct approach would be via an environment variable holding the password. I've just generally got misgivings about this. I'd be less wary of there were an automatic method for expiring environment variables (especially on all supported platforms), but there isn't, AFAIK. (Having, instead, the password hash would at least have it obfuscated, but the encryption key is, by design, not derivable from the password hash. And I'm even more uncomfortable having the encryption key exposed.)

Summarizing:

• The CLI needs to be able to derive the encryption key for the target user locker.
• That capability must depend on something found in the ephemeral environment (not on disk).

There are currently issues with the three abstract base classes defined in crypt_ifc, and their implementing child classes.
Some places it's the method on the child class that just deviates from a "good" signature on the abc, some places the abstract method signature is too specific to a particular implementation so that other implementations have mismatched signatures.
In general, the abstract methods should be defined with "least common denominator" parameters, always ending with **kwargs, and each implementation has to unpack its specific kwargs for required params. Probably the most disruptive part of the work to tidy this all up will be backtracking the implementations that inappropriately drove the abstract method signatures.

Attempted to add windows testing on github, ran into problems in plaintext-based lockers on windows due to the file naming using {item_type}:{item_name}, as colon is an illegal character for windows files.

I chose that format so that e.g. a (now defunct) template could have the same name as a secret and not cause a collision.

At the moment, the only item "types" are secret and tag. I do want to keep the tag feature (basically just a list of secrets), but I'm thinking about how to do that without this broken file naming. (My early approach was that each item type had a directory in the locker; I got rid of that because someone with access to the filesystem could intuit less about the data if the item files were stored in the same directory.)

Before opening to public, need to properly add a copyleft license notice

The native cli has an edit command that launches the user's text editor.
This can be used to update an existing item.
A "template" can be specified.

In the case of a new item, the initial content of the temp file being edited will be from the template, if specified.

In the case of an existing item, that content will be the current content from the item.

What should be the content of an existing item if a template is specified (and --overwrite is True)?

Right now I'm aiming at the existing content, a single label line like "current content above, template below" followed by the template content.

Thoughts?

I've directed substantially more effort toward CLI tests than otherwise.

This should be remedied. The side benefit is that they are a lot easier to debug, e.g. you can actually reliably hit breakpoints outside of the click context.

From now on, any time I break something in a CLI test, I'm going to reproduce it with a failing non-CLI test before attempting to fix it.

As part of the effort to assure BC, there needs to be test lockers/items kept representing all released crypt methods and naming with complete regression testing that reads all lockers/items. It would also be useful to devise tests that confirm that changes to things like the global registration cause those read attempts to fail.

I'm concerned about the possibility of causing a user to lose access to the contents of their locker.

For example, I could discover a glaring error in my encryption configuration and fix it, then a user who upgrades would be trying to decrypt using the wrong method.

This could be addressed by permanently retaining any encryption method ever used. Stipulating that as the approach, there would of course have to be a way to determine at runtime what method a given locker used. One way would be to somehow mark a "version" of encryption (probably distinct from the phibes version). Another way could be to, say, attempt to decrypt the lockfile with each available method, and when the timestamp returned is valid and sane, designate that as the correct method. If that method isn't the latest installed, maybe warn the user that it's out of date and offer a migration.

I'm open to other ideas about how to address this concern.

In the interest of possible future addition of an HTTP front end, there should be exposed methods that rely exclusively on string arguments. To raise the initial and maintained quality of such interface, this should be the only way the CLI interacts with the core.

(I considered at the outset creating the CLI as a distinct project from Phibes, but I didn't think the overhead of having a moving target developing against the dependency of a moving target was worth it yet. At some point, when the Phibes API is reasonably well settled, it might make sense to make that split. Possibly at the same time as starting development of a REST wrapper.)

It seems like Argon2 has been the preferred KDF for several years now, but of course pbkdf2_hmac is built in to (core) hashlib, where AFAICT the available sources for Argon2 are argon2-cffi and pynacl, both of which have dependencies on non-python binary packages.
Edit: there is also Passlib which also has a non-python binary dependency.

Currently, a storage path is determined, and a users directory is created, and lockers are directories under users (named by a hash from the user's specified name).
It needs to be possible for a cli user to have a locker in an arbitrary directory.
Of course there are questions. My first concern is that a user will unwittingly create a locker in a directory that is already populated with a large number of files. Adding the lock file and then a bunch of item files there will invite confusion. So perhaps the locker folder should be created even in this scenario. It's important to keep in mind that having the locker in a git repo is a first-order consideration of the cli tool. While we certainly don't want to inhibit a user from creatively adding their own plaintext artifacts to that repo, we want to steer them away from accidently starting with massive clutter.

I think this justifies the choice to always create a locker directory.

Currently, user has option (in config file) to hash or not hash locker names; this choice is applied to the name of the directory created for the locker.

Benefits of hashing the locker name:

• Avoids problems with e.g. allowed characters in file names
• Prevents a snooper with access to the file system from knowing the locker name
• Greatly reduces the possibility of naming collision (because salt is used)

Challenges hashing the locker name presents (as implemented):

• Each request to access a named locker requires attempting access to all lockers
• Complicates encryption implementation e.g. salt length for PBKDF2 vs. salt length for naming

Also, allowing a choice of whether to hash the name significantly complicates the code to access a locker.

I intend to remove the configuration choice from the user and implement a single scheme to name lockers in storage.

The new scheme will:

• not be dependent on the encryption/hashing method used within the locker
• will be file system safe (character-wise)
• will not be concerned with name collision
• will be deterministic, allowing access without scanning multiple lockers
• will obfuscate the user-selected name, not necessarily hash/encrypt it.

I'm waiving concern about name collision for several reasons:

• A single user attempting to create the same locker name twice is probably doing so mistakenly, and should fail
• Multiple users configured to share a storage location are probably trying to share, and the same rule applies
• In the possible future scenario where a hosted instance is being used for many users, there will be an intermediary service that will e.g. handle user auth before interacting with core Phibes; that service should address naming collisions.

I'm allowing "name obfuscation" instead of some higher bar of hashing because knowing the locker name doesn't improve the chances of an intruder accessing secrets; outside of using the Phibes CLI, the locker name isn't required for decryption.

At them moment, I'm considering just using base64 encoding of the locker names.

The "basic" command phibes and the "advanced" command phibesplus have different subcommands for the same operation, e.g. init vs. create for locker creation. They got that way sort of accidently, but stayed that way sort of on purpose, rationalizing that maybe it's better to make sure the user is using the correct command. Probably could use a little more thought/discussion.

This will be much easier to do when click 8 is available.