eirslett / frontend-maven-plugin

"Maven-node-grunt-gulp-npm-node-plugin to end all maven-node-grunt-gulp-npm-plugins." A Maven plugin that downloads/installs Node and NPM locally, runs NPM install, Grunt, Gulp and/or Karma.
Apache License 2.0
4.22k stars 868 forks source link

fix: a nasty threading issue in password decryption code #998

Closed ctran closed 2 years ago

ctran commented 2 years ago

Summary This change fixes a nasty threading issue due to the way the DefaultSettingsDecrypter works.

Our build jobs began to fail randomly with error " Could not download Node.js: Got error code 401 from the server". We have hundreds of builds a day and getting about 3 to 10 failures a day. Took a while but I was finally able to reproduce this issue consistently with the following setup:

  1. Enter encrypted server credentials in settings.xml (with a settings-security.xml) as described here
  2. Create a reactor with at least 2 projects configured with frontend-maven-plugin to download node/npm from a private server that requires basic auth
  3. Run maven in multithreading mode ("-T2")

Observation is at least one of the above projects will fail to download with a 401 error.

Tests and Documentation

Here's additional debug output from HttpClient (https://hc.apache.org/httpcomponents-client-4.5.x/logging.html)

Single thread mode:

2021/09/11 07:56:49:278 PDT [DEBUG] MainClientExec - Opening connection {}->http://artifact:8081
2021/09/11 07:56:49:279 PDT [DEBUG] DefaultHttpClientConnectionOperator - Connecting to artifact/10.0.1.230:8081
2021/09/11 07:56:49:279 PDT [DEBUG] DefaultHttpClientConnectionOperator - Connection established 10.1.76.0:41992<->10.0.1.230:8081
2021/09/11 07:56:49:279 PDT [DEBUG] MainClientExec - Executing request GET /content/sites/npm-dist/npm-6.13.6.tgz HTTP/1.1
2021/09/11 07:56:49:280 PDT [DEBUG] MainClientExec - Target auth state: CHALLENGED
2021/09/11 07:56:49:280 PDT [DEBUG] MainClientExec - Proxy auth state: UNCHALLENGED
2021/09/11 07:56:49:280 PDT [DEBUG] headers - http-outgoing-1 >> GET /content/sites/npm-dist/npm-6.13.6.tgz HTTP/1.1
2021/09/11 07:56:49:281 PDT [DEBUG] headers - http-outgoing-1 >> Host: artifact:8081
2021/09/11 07:56:49:281 PDT [DEBUG] headers - http-outgoing-1 >> Connection: Keep-Alive
2021/09/11 07:56:49:281 PDT [DEBUG] headers - http-outgoing-1 >> User-Agent: Apache-HttpClient/4.5.2 (Java/1.8.0_275-snc1)
2021/09/11 07:56:49:281 PDT [DEBUG] headers - http-outgoing-1 >> Authorization: Basic dXNlcm5hbWU6cGFzc3dvcmQ=
2021/09/11 07:56:49:284 PDT [DEBUG] headers - http-outgoing-1 << HTTP/1.1 200 OK
2021/09/11 07:56:49:284 PDT [DEBUG] headers - http-outgoing-1 << Date: Sat, 11 Sep 2021 14:56:49 GMT
2021/09/11 07:56:49:284 PDT [DEBUG] headers - http-outgoing-1 << Server: Nexus/3.33.0-01 (PRO)
2021/09/11 07:56:49:285 PDT [DEBUG] headers - http-outgoing-1 << X-Content-Type-Options: nosniff
2021/09/11 07:56:49:285 PDT [DEBUG] headers - http-outgoing-1 << Content-Security-Policy: sandbox allow-forms allow-modals allow-popups allow-presentation allow-scripts allow-top-navigation
2021/09/11 07:56:49:285 PDT [DEBUG] headers - http-outgoing-1 << X-XSS-Protection: 1; mode=block
2021/09/11 07:56:49:285 PDT [DEBUG] headers - http-outgoing-1 << Last-Modified: Tue, 14 Jan 2020 21:16:57 GMT
2021/09/11 07:56:49:286 PDT [DEBUG] headers - http-outgoing-1 << ETag: "{SHA1{86df8305a4d8269d0934ec907920e7ab079cf5d9}}"
2021/09/11 07:56:49:286 PDT [DEBUG] headers - http-outgoing-1 << Content-Disposition: attachment
2021/09/11 07:56:49:286 PDT [DEBUG] headers - http-outgoing-1 << Content-Type: application/x-tgz
2021/09/11 07:56:49:286 PDT [DEBUG] headers - http-outgoing-1 << Content-Length: 6071169

Multithread mode:

2021/09/11 07:57:33:910 PDT [DEBUG] MainClientExec - Opening connection {}->http://artifact:8081
2021/09/11 07:57:33:915 PDT [DEBUG] DefaultHttpClientConnectionOperator - Connecting to artifact/10.0.1.230:8081
2021/09/11 07:57:33:916 PDT [DEBUG] DefaultHttpClientConnectionOperator - Connection established 10.1.76.0:42458<->10.0.1.230:8081
2021/09/11 07:57:33:917 PDT [DEBUG] MainClientExec - Executing request GET /content/sites/node-dist/v8.11.2/node-v8.11.2-linux-x64.tar.gz HTTP/1.1
2021/09/11 07:57:33:917 PDT [DEBUG] MainClientExec - Target auth state: CHALLENGED
2021/09/11 07:57:33:921 PDT [DEBUG] MainClientExec - Proxy auth state: UNCHALLENGED
2021/09/11 07:57:33:923 PDT [DEBUG] headers - http-outgoing-0 >> GET /content/sites/node-dist/v8.11.2/node-v8.11.2-linux-x64.tar.gz HTTP/1.1
2021/09/11 07:57:33:924 PDT [DEBUG] headers - http-outgoing-0 >> Host: artifact:8081
2021/09/11 07:57:33:924 PDT [DEBUG] headers - http-outgoing-0 >> Connection: Keep-Alive
2021/09/11 07:57:33:924 PDT [DEBUG] headers - http-outgoing-0 >> User-Agent: Apache-HttpClient/4.5.2 (Java/1.8.0_275-snc1)
2021/09/11 07:57:33:925 PDT [DEBUG] headers - http-outgoing-0 >> Authorization: Basic dXNlcm5hbWU6ezBocG40ZER0VUhRSExPTVVxelZtalpVNnJjcXVvdEhaT0tldk44bVFsZXM9fQ==
2021/09/11 07:57:33:933 PDT [DEBUG] headers - http-outgoing-0 << HTTP/1.1 401 Unauthorized
2021/09/11 07:57:33:934 PDT [DEBUG] headers - http-outgoing-0 << Date: Sat, 11 Sep 2021 14:57:33 GMT
2021/09/11 07:57:33:934 PDT [DEBUG] headers - http-outgoing-0 << Server: Nexus/3.33.0-01 (PRO)
2021/09/11 07:57:33:934 PDT [DEBUG] headers - http-outgoing-0 << X-Content-Type-Options: nosniff
2021/09/11 07:57:33:935 PDT [DEBUG] headers - http-outgoing-0 << WWW-Authenticate: BASIC realm="Sonatype Nexus Repository Manager"
2021/09/11 07:57:33:935 PDT [DEBUG] headers - http-outgoing-0 << Content-Length: 0

You can see the password specified in the "Authorization" header is not the decrypted version. Once I looked at the code of DefaultSettingsDecrypter.java, it's pretty obvious there's no synchronization done at this line:

server.setPassword( decrypt( server.getPassword() ) );
eirslett commented 2 years ago

Is this something that could potentially be fixed in Maven itself?

ctran commented 2 years ago

Logically speaking, it should be fixed there but I haven't been able to create a reproducible scenario in normal maven usage. I think it may be specific to how it's being done in DefaultWagonManager where the credentials are stored in a POJO whereas it's being accessed directly via the same server instance in frontend-maven-plugin:

    SettingsDecryptionResult result =
        settingsDecrypter.decrypt( new DefaultSettingsDecryptionRequest( server ) );
    server = result.getServer();

    AuthenticationInfo authInfo = new AuthenticationInfo();
    authInfo.setUserName( server.getUsername() );
    authInfo.setPassword( server.getPassword() );
    authInfo.setPrivateKey( server.getPrivateKey() );
    authInfo.setPassphrase( server.getPassphrase() );

    return authInfo;
eirslett commented 2 years ago

Ok, I guess we can fix it in this plugin then, until Maven maybe fixes it internally.

alan-czajkowski commented 2 years ago

when will this fix be released?