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

WIndowsPb: gpg signature verification for ANT binary #3018

Merged
merged 15 commits into from
May 24, 2023

Conversation

Haroon-Khel
Copy link
Contributor

@Haroon-Khel Haroon-Khel commented Apr 5, 2023

  • commit message has one of the standard prefixes
  • faq.md updated if appropriate
  • other documentation is changed or added (if applicable)
  • playbook changes run through VPC or QPC (if you have access)
  • VPC/QPC not applicable for this PR
  • for inventory.yml changes, bastillion/nagios/jenkins updated accordingly

ref #2908

It seems only the Ant download supplies a .asc or .sig file. For the other downloads, we supply a fixed checksum string which, since we do not plan on changing, provide adequate verification of a download.

Theres still the java downloads in the playbook which are capable of having gpg verification. Getting the signature file is a bit tricky, on unix we do something like this

curl -s 'https://api.adoptium.net/v3/assets/feature_releases/{{ jdk_version }}/ga?architecture={{ api_architecture }}&heap_size=normal&image_type=jdk&jvm_impl={{ bootjdk }}&os={{ platformLinux }}&page=0&page_size=1&project=jdk&vendor=eclipse' | grep signature_link | awk '{split($0,a,"\""); print a[4]}'

which does not seem to be working well in a windows environment. So until I can figure this out, I thought id merge my work so far

@Haroon-Khel
Copy link
Contributor Author

@Haroon-Khel
Copy link
Contributor Author

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Apr 5, 2023

TASK [ANT : Import GPG Key] ****************************************************
fatal: [127.0.0.1]: FAILED! => {"changed": true, "cmd": "gpg --keyserver keyserver.ubuntu.com --recv-keys \"CE8075A251547BEE249BC151A2115AE15F6B8B72\"", "delta": "0:00:00.072935", "end": "2023-04-05 12:25:35.885799", "msg": "non-zero return code", "rc": 2, "start": "2023-04-05 12:25:35.812864", "stderr": "gpg: fatal: can't create directory `/home/runneradmin/.gnupg': No such file or directory\nsecmem usage: 0/0 bytes in 0/0 blocks of pool 0/65536\n", "stderr_lines": ["gpg: fatal: can't create directory `/home/runneradmin/.gnupg': No such file or directory", "secmem usage: 0/0 bytes in 0/0 blocks of pool 0/65536"], "stdout": "", "stdout_lines": []}

I wouldnt expect there to be a problem with making directories in /home/$user once cygwin is installed

@Haroon-Khel
Copy link
Contributor Author

Latest vpc hit the same error

14:47:20 TASK [ANT : Import GPG Key] ****************************************************
14:47:23 fatal: [127.0.0.1]: FAILED! => {"changed": true, "cmd": "gpg --keyserver keyserver.ubuntu.com --recv-keys \"CE8075A251547BEE249BC151A2115AE15F6B8B72\"", "delta": "0:00:00.297306", "end": "2023-04-05 01:30:39.610441", "msg": "non-zero return code", "rc": 2, "start": "2023-04-05 01:30:39.313134", "stderr": "gpg: fatal: can't create directory `/home/vagrant/.gnupg': No such file or directory\nsecmem usage: 0/0 bytes in 0/0 blocks of pool 0/65536\n", "stderr_lines": ["gpg: fatal: can't create directory `/home/vagrant/.gnupg': No such file or directory", "secmem usage: 0/0 bytes in 0/0 blocks of pool 0/65536"], "stdout": "", "stdout_lines": []}

Did not get this error when testing on test-azure-win2012r2-x64-1

@sxa
Copy link
Member

sxa commented Apr 5, 2023

Latest vpc hit the same error

14:47:20 TASK [ANT : Import GPG Key] ****************************************************
14:47:23 fatal: [127.0.0.1]: FAILED! => {"changed": true, "cmd": "gpg --keyserver keyserver.ubuntu.com --recv-keys \"CE8075A251547BEE249BC151A2115AE15F6B8B72\"", "delta": "0:00:00.297306", "end": "2023-04-05 01:30:39.610441", "msg": "non-zero return code", "rc": 2, "start": "2023-04-05 01:30:39.313134", "stderr": "gpg: fatal: can't create directory `/home/vagrant/.gnupg': No such file or directory\nsecmem usage: 0/0 bytes in 0/0 blocks of pool 0/65536\n", "stderr_lines": ["gpg: fatal: can't create directory `/home/vagrant/.gnupg': No such file or directory", "secmem usage: 0/0 bytes in 0/0 blocks of pool 0/65536"], "stdout": "", "stdout_lines": []}

Did not get this error when testing on test-azure-win2012r2-x64-1

I would guess that's because it's running under cmd.exe (The windows shell) and not cygwin's bash and therefore doesn't understand /home/vagrant. When you tested outside ansible was it from a cygwin shell?

@sxa
Copy link
Member

sxa commented Apr 5, 2023

You can set an environment variable GNUPGHOME=$WORKSPACE/.gnupg to a specific directory to use, so that may give you a way forward to use an alternate location (I use this for the Temurin GPG signing to store it in the jenkins workspace)

(There's probably a command line option to set it too, but I've not used that)

@Haroon-Khel
Copy link
Contributor Author

Haroon-Khel commented Apr 6, 2023

I would guess that's because it's running under cmd.exe (The windows shell) and not cygwin's bash and therefore doesn't understand /home/vagrant. When you tested outside ansible was it from a cygwin shell?

Works in a cmd.exe environment too. I think with cygwin installed, any shell can access /home/ directories?

image

@Haroon-Khel
Copy link
Contributor Author

(There's probably a command line option to set it too, but I've not used that)

--homedir looks like it should work, just need to figure out how to use it properly

image

(both forward and backslash path locations)

@sxa
Copy link
Member

sxa commented Apr 6, 2023

Interesting ... the other thing to check would be whether there is another gpg tool on the machine that's being picked up instead of the cygwin one. Could run a test where you fully qualify c:\cygwin\bin\gpg to run it and see if that makes a difference.

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but my only comment would be that it may not be necessary to import the key each time (e.g. it could be skipped if it already exists on the machine)

@karianna
Copy link
Contributor

Playbook checkers are failing, related?

@sxa
Copy link
Member

sxa commented Apr 13, 2023

Playbook checkers are failing, related?

Looks like it. (My review was based on reading the code, and so the check failures need to be understood and addressed before this is merged)

@karianna
Copy link
Contributor

@Haroon-Khel needs a linter fix

file: "{{playbook_dir}}/roles/GPG_signature_verification/tasks/main.yml"
vars:
file_path: c:\temp\ant.zip
signature_link: "https://archive.apache.org/dist/ant/binaries/apache-ant-1.10.5-bin.zip.asc"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ant is on 1.10.13 now - should we upgrade?

Copy link
Contributor

@karianna karianna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ant version

@sxa
Copy link
Member

sxa commented May 22, 2023

Ant version

I would suggest that we put the security checks in place and then discuss that as a separate issue. I'd want there to be additional testing on a new ant version in order to ensure we don't have compatibility issues before just bumping it up (especially sine this PR is specific to Windows and we use the same fixed version across all platforms IIRC) - perhaps create a new issue proposing an upgrade? It's probably about time we did look at that.

ant is critical to the test case executions and also now for the SBOM generation.

@Haroon-Khel
Copy link
Contributor Author

Ant contrib is failing to download on the 2022 windows workflow

ASK [ANT : Download ant-contrib] **********************************************
fatal: [127.0.0.1]: FAILED! => {"changed": false, "dest": "c:\\temp\\ant-contrib.zip", "elapsed": 0.8295483, "msg": "Error downloading 'https://sourceforge.net/projects/ant-contrib/files/ant-contrib/ant-contrib-1.0b2/ant-contrib-1.0b2-bin.zip' to 'c:\\temp\\ant-contrib.zip': The request was aborted: Could not create SSL/TLS secure channel.", "status_code": null, "url": "https://sourceforge.net/projects/ant-contrib/files/ant-contrib/ant-contrib-1.0b2/ant-contrib-1.0b2-bin.zip"}

however that error is unrelated to this pr, the gpg check on the ant binary just before it passes without error

A bit confused by this linter error


Run # Ansible code static analysis
WARNING  Listing 1 violation(s) that are fatal
syntax-check[missing-file]: Unable to retrieve file contents
ansible/playbooks/AdoptOpenJDK_Windows_Playbook/roles/ANT:1:1 Could not find or access '/tmp/roles/GPG_signature_verification/tasks/main.yml' on the Ansible Controller.

Error: Unable to retrieve file contents

                    Rule Violation Summary                     
 count tag                        profile rule associated tags 
     1 syntax-check[missing-file] min     core, unskippable    

Failed after : 1 failure(s), 0 warning(s) on 505 files.
Error: Process completed with exit code 2.

@Haroon-Khel
Copy link
Contributor Author

Linter issue is solved. The pr checks are running now because I had to rebase (branch was out of date) but this pr is ready to be merged

@Haroon-Khel
Copy link
Contributor Author

The ant contrib error mentioned in #3018 (comment) is intermittent

Copy link
Contributor

@steelhead31 steelhead31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Haroon-Khel Haroon-Khel merged commit d9b705d into adoptium:master May 24, 2023
6 of 7 checks passed
@Haroon-Khel Haroon-Khel deleted the windows.gpg branch June 5, 2023 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants