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

Integration specific file reading #151

Open
jdx opened this issue Dec 15, 2024 · 4 comments
Open

Integration specific file reading #151

jdx opened this issue Dec 15, 2024 · 4 comments

Comments

@jdx
Copy link
Contributor

jdx commented Dec 15, 2024

I'm exploring adopting rops for use in mise, however I can't get rops to work in this basic test:

bash-5.2$ age-keygen -o age.txt
Public key: age10v0k6lrd2hzjamlnatqzpcl7ky0n04f6vum9hvjxst0ce0nmsvdshjjly6
bash-5.2$ echo "foo.bar = 'baz'" > file.toml
bash-5.2$ rops encrypt --age age10v0k6lrd2hzjamlnatqzpcl7ky0n04f6vum9hvjxst0ce0nmsvdshjjly6 file.toml > enc.toml
bash-5.2$ ROPS_AGE_KEY_FILE=$PWD/age.txt rops decrypt enc.toml
Error: unable to decrypt file metadata

Caused by:
    0: unable to retrieve data key
    1: integration error
    2: unnable to parse private key string: invalid Bech32 encoding

admittedly I'm unfamiliar with these tools so I may be doing something wrong

@jdx
Copy link
Contributor Author

jdx commented Dec 15, 2024

update: I figured it out, I needed to remove the comments from age.txt which age includes by default

so this ticket should probably be for rops to ignore comments in age.txt files

@gibbz00 gibbz00 changed the title unnable [sic] to parse private key string: invalid Bech32 encoding Integration specific file reading Dec 16, 2024
@gibbz00
Copy link
Owner

gibbz00 commented Dec 16, 2024

Yeah, you're right, almost.

The documentation in https://gibbz00.github.io/rops/concepts.html#private-integration-key tries to explain how ROPS_{}_KEY_FILE is rops specific and expects the same format regardless of integration. This is a result of adhering to one of the project goals; "Be consistent in how credentials are used, set and retrieved across integrations.".

The key retrieval documentation does on the other hand contain:

The strategy for where to look for a private key goes as follows:

In the environment variables.
In the rops key files.
(Future) In the default integration key files.

And https://gibbz00.github.io/rops/concepts.html#to-supply-private-keys-using-integration-key-files-future contains:

Many integrations already store their keys in a dedicated location but in wildly different structures. rops does currently not parse these files, but it aims to so in the future:

So yes, integration specific file reading is something that is currently missing, but ROPS_{}_KEY_FILE probably shouldn't be entrypoint for it. AWS stores for example their config as:

[default]
aws_access_key_id=AKIAIOSFODNN7EXAMPLE
aws_secret_access_key=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY

[user1]
aws_access_key_id=AKIAI44QH8DHBEXAMPLE
aws_secret_access_key=je7MtGbClwBF/2Zp9Utk/h3yCo8nvbEXAMPLEKEY

...and it would be nice i rops could pick it up automatically from the default location. Also, it should probably be the integration specific env variable which overrides the its default location, e.i AWS_SHARED_CREDENTIALS_FILE. (source)

I can understand that this brings confusion unless you carefully read the documentation, because it's how SOPS does it with SOPS_AGE_KEY_FILE.

@jdx
Copy link
Contributor Author

jdx commented Dec 16, 2024

I was able to work around this for my usage: https://github.com/jdx/mise/blob/728bd0211e3e90cef8f23ccb48e4fd87911dec6a/src/sops.rs#L31-L37

however I'd be happy to submit a patch for this upstream—for the end users not being able to directly use a file generated by age-keygen would be pretty unacceptable IMO. If not that, I think we could at least show a warning or better error message.

@gibbz00
Copy link
Owner

gibbz00 commented Dec 19, 2024

I was able to work around this for my usage: https://github.com/jdx/mise/blob/728bd0211e3e90cef8f23ccb48e4fd87911dec6a/src/sops.rs#L31-L37

Cool cool. Impressed that you were able to use the library API, even when it lacks the most basic documentation.
(Gotten a bit better with this on my more recent project by linting them with missing_docs = "deny" 😅)

However I'd be happy to submit a patch for this upstream—for the end users not being able to directly use a file generated by age-keygen...

Contributions are always welcomed 😊 I do agree that ease of use should be a top priority.

Including the workaround upstream would be done here:

match integration_key_file.exists() {
true => std::fs::read_to_string(integration_key_file)?
.lines()
.map(|line| line.trim())
.filter(|line| !line.is_empty())
.map(Self::parse_private_key)
.collect(),

I'm not fully convinced, however, that this would be a good idea (yet). We'd be
shooting ourselves in the foot the second a new integration is added which
may include a pound sign as the initial character in the secret key. But maybe
that's a compromise worth taking for now.

"Fully" implementing integration specific file reads would be the more
correct solution. Problem is that I can't seem to find any authoritative
definition for how age does in terms of: env variable name, expected
output format, and default file location. Only thing I found was this:

FiloSottile/age#15

However, nowhere am I able find what @st4d is referring to when they write:

"The specification gives the Unix path ~/.config/age/* as the default location it looks for keys and aliases."

Improving the error message would very helpful and appreciated. I'm unfortunately a bit caught up in work right now to do this myself :/

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

No branches or pull requests

2 participants