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

Don't put sensitive credentials such as Android keystore properties in export_presets.cfg #1156

Closed
andy-noisyduck opened this issue Jul 3, 2020 · 6 comments · Fixed by godotengine/godot#76165

Comments

@andy-noisyduck
Copy link

andy-noisyduck commented Jul 3, 2020

I had a feeling I've come across this before, but I couldn't find any proposals....

Describe the project you are working on:
Mobile game (Android)

Describe the problem or limitation you are having in your project:
Godot stores export settings including which resources to bundle, app icons, export capabilities in the export_presets.cfg. These settings are a core part of development, and we normally want to include these in source control. Unfortunately Godot also stores the keystore credentials in the same export file. This means we either have to commit credentials to our repo, or we have to manually recreate the presets between developers, and somehow sync changes. Both are really bad options.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
It would be better to have keystore credentials in their own config file alongside exports_presets.cfg. Something like exports_credentials.cfg or similar. This file could then be local to each developer and not need committing. It also makes it much easier to apply specific keys for CI/CD if being used.

Another alternative would be to allow any properties of exports_presets.cfg to be overriden locally, such as a exports_presets.local file with the same format as the presets file. This might be more appropriate if further settings plan to be stored in the presets file too - paths would be a reasonable use case here as they can often be machine specific.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
I can't think of a nice way around it.

Is there a reason why this should be core and not an add-on in the asset library?:
Android exporting is a core feature of Godot. Anyone exporting to android with a team of more than 1 will end up hitting this issue.

@Calinou
Copy link
Member

Calinou commented Jul 3, 2020

This will be addressed by godotengine/godot#35930 once it's merged.

@Calinou Calinou changed the title Don't put keystore properties in export_presets.cfg Don't put Android keystore properties in export_presets.cfg Jul 3, 2020
@Calinou Calinou added this to the 4.0 milestone Jul 25, 2022
@Calinou Calinou changed the title Don't put Android keystore properties in export_presets.cfg Don't put sensitive credentials such as Android keystore properties in export_presets.cfg Feb 1, 2023
@Calinou
Copy link
Member

Calinou commented Feb 1, 2023

Renamed to also cover Windows and macOS codesigning credentials.

In the meantime, the current recommendation is to add export_presets.cfg to .gitignore, or avoiding specifying any credentials in it. You can specify credentials at build-time using sed if running a script or using a CI platform to export the project in production.

@and-rad
Copy link

and-rad commented Mar 27, 2023

I'm about to pick this up where @Calinou left off. These are the properties that I consider sensitive information (Names in parentheses are the key names stored in the config file):

All:

  • Encryption Key (script_encryption_key)

Not quite sure about these:

  • SSH Remote Deploy Host (ssh_remote_deploy/host)
  • SSH Remote Deploy Port (ssh_remote_deploy/port)
  • SSH Remote Deploy Extra Args SSH (ssh_remote_deploy/extra_args_ssh)
  • SSH Remote Deploy Extra Args SCP (ssh_remote_deploy/extra_args_scp)

Is remote deploy supposed to be used for pushing to production machines? If not, I can see value in having these properties in the git repository if it's a larger place with internal test servers that every team member should be able to deploy to for testing, but I don't know if that is an intended scenario here.

Windows:

  • Codesign Identity Type (codesign/identity_type)
  • Codesign Identity (codesign/identity)
  • Codesign Password (codesign/password)
  • Codesign Digest Algorithm (codesign/digest_algorithm)

Android:

  • Keystore Debug (keystore/debug)
  • Keystore Debug User (keystore/debug_user)
  • Keystore Debug Password (keystore/debug_password)
  • Keystore Release (keystore/release)
  • Keystore Release User (keystore/release_user)
  • Keystore Release Password (keystore/release_password)

iOS:

  • Application App Store Team ID (application/app_store_team_id)
  • Application Provisioning Profile UUID Debug (application/provisioning_profile_uuid_debug)
  • Application Conde Sign Identity Debug (application/code_sign_identity_debug)
  • Application Provisioning Profile UUID Release (application/provisioning_profile_uuid_release)
  • Application Conde Sign Identity Release (application/code_sign_identity_release)

UWP:

  • Signing Certificate (signing/certificate)
  • Signing Password (signing/password)
  • Signing Algorithm (signing/algorithm)

Not sure about these:

  • Identity Product GUID (identity/product_guid)
  • Identity Publisher GUID (identity/publisher_guid)

macOS:

  • Codesign Certificate File (codesign/certificate_file)
  • Codesign Certificate Password (codesign/certificate_password)
  • Notarization API UUID (notarization/api_uuid)
  • Notarization API Key (notarization/api_key)
  • Notarization API Key ID (notarization/api_key_id)

These exist in the config file, but I couldn't see them in the export UI:

  • codesign/identity
  • notarization/apple_id_name
  • notarization/apple_id_password
  • notarization/apple_team_id

The plan is to create a separate export_credentials.cfg file that stores these and should not be committed to git. You will also be able to set each of these properties by environment variables. If an environment variable is present, it will always take precedence over the export_credentials.cfg file.

@Calinou
Copy link
Member

Calinou commented Mar 27, 2023

Is remote deploy supposed to be used for pushing to production machines? If not, I can see value in having these properties in the git repository if it's a larger place with internal test servers that every team member should be able to deploy to for testing, but I don't know if that is an intended scenario here.

To my knowledge, remote deploy always deploys debug export templates, so it's not meant for deploying to production. (Deploying release export templates could be added to make performance testing more representative of real-world scenarios, but it'll disable live scene editing and other debug client features.)

@and-rad
Copy link

and-rad commented Mar 27, 2023

If that's the case, I'm inclined to leave the remote deploy properties where they are. I can see good reasons for committing them to VCS and for moving them out, so I think I'll stick with how it is now. We can still change it later if people prefer it the other way.

What about UWP's Identity Product GUID and Identity Publisher GUID? I have no experience with UWP, but from my limited research it seems that these are public knowledge.

@and-rad
Copy link

and-rad commented Apr 11, 2023

Should values that are stored in environment variables show up in the export UI? I'm thinking probably no because it might be a little too confusing. On the other hand, if the UI only displays the values that are stored in the *.cfg files, (a) people have to remember that they set up environment variables and should make any changes there and (b) the UI would effectively lie to them, claiming that an export option has value X (defined in *.cfg) when in reality it is Y (defined in env var).

The reason why I still lean towards keeping environment vars out of the UI is that I think the majority of users won't use them anyway. I think I changed my mind on this. I thought about it some more and it might be a cleaner and overall better solution to read from environment variables too when populating the export UI.

What may be a useful addition, though, is a button somewhere in the export UI that lets you copy all environment variable names in a given preset to the clipboard.

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

Successfully merging a pull request may close this issue.

5 participants