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

Allow authorization also against insecure registries (at least for testing) #599

Closed
Hi-Fi opened this issue Jul 13, 2018 · 12 comments
Closed
Assignees
Milestone

Comments

@Hi-Fi
Copy link

Hi-Fi commented Jul 13, 2018

Description of the issue:
Currently if registry offers only insecure access, there's no way to authenticate against it (https://github.com/GoogleContainerTools/jib/blob/master/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java#L165). It would be good for testing to be able to authenticate over http (partly because of #543).

Expected behavior:
Parameter where user acknowledges that it's very bad idea to send credentials over http, but still allowing that.

Steps to reproduce:
Create local registry with authentication and without TLS.

Environment:
Apache Maven 3.5.0 (ff8f5e7444045639af65f6095c62210b5713f426; 2017-04-03T22:39:06+03:00)
Java version: 1.8.0_131, vendor: Oracle Corporation
Default locale: en_US, platform encoding: Cp1252
OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"
jib-plugin version: 0.9.6

jib-maven-plugin Configuration:

<plugin>
	<groupId>com.google.cloud.tools</groupId>
	<artifactId>jib-maven-plugin</artifactId>
	<version>${jib.version}</version>
	<configuration>
		<from>
			<image>${jib.fromImage}</image>
		</from>
		<to>
			<image>${jib.toImage}</image>
		</to>
		<container>
			<ports>
				<port>${jib.applicationPort}</port>
			</ports>
		</container>
		<allowInsecureRegistries>${jib.allowInsecureRegistries}</allowInsecureRegistries>
	</configuration>
	<executions>
		<execution>
			<phase>compile</phase>
			<goals>
				<goal>build</goal>
			</goals>
		</execution>
	</executions>
</plugin>

Log output:
Without allowInsecureRegitstries:

[ERROR] Failed to execute goal com.google.cloud.tools:jib-maven-plugin:0.9.6:build (default) on project jib-demo-project: Build image failed, perhaps you should use a registry that supports HTTPS or set the configuration parameter 'allowInsecureRegistries': Only secure connections are allowed, but tried to reach URL http://artifactory/project/baseImage/manifests/latest -> [Help 1]

With allowInsecureRegitstries:

[ERROR] Failed to execute goal com.google.cloud.tools:jib-maven-plugin:0.9.6:build (default) on project jib-demo-project: Build image failed, perhaps you should make sure your credentials for 'artifactory' are set up correctly: Unauthorized for http://artifactory/project/baseImage: 401 Unauthorized
[ERROR] {"errors":[{"code":"UNAUTHORIZED","message":"authentication required","detail":null}]}

Additional Information:
If this is not possible, at least error message should state that authentication (username+password) was not even used, because connection was made with http instead of https.

@coollog
Copy link
Contributor

coollog commented Jul 13, 2018

Thanks for filing the issue! Referencing the related comment: #545 (comment)

A summary of possible approaches:

  • An error message when 401 Unauthorized over HTTP saying that credentials were not used (as you suggested)
  • Just send credentials if allowInsecureRegistries is enabled (not safe)
  • Provide a command line option (not in build configuration) like sendCredentialsOverHttp to enable this for debug purposes
  • Add an additional build configuration for allowing this (confusing with allowInsecureRegistries)

@GoogleContainerTools/java-tools

@coollog coollog added this to the v0.9.7 milestone Jul 13, 2018
@TadCordle
Copy link
Contributor

+1 to some combination of options 1 and 3 (nicer error mentioning command line option by default, only allow sending credentials over http if a command line option is set)

@coollog
Copy link
Contributor

coollog commented Jul 13, 2018

Okay, finalized proposal is:

@chanseokoh
Copy link
Member

chanseokoh commented Jul 13, 2018

It seems worth having a discussion to sort out things around HTTP, HTTPS, and allowInsecureRegistries. For example about the behavior of allowInsecureRegistries,

  • It falls back to HTTP if HTTPS fails for any reason (I believe), which includes invalid or unverifiable certificates. In the certificate case, some people may expect Jib should just completely ignore the certificate problem and proceed with HTTPS (which might be risky and they should know what they are doing), like what curl --insecure does. For example, if a port is explicitly given like localhost:5000 that accepts HTTPS, the 5000 port won't accept HTTP at all.
  • It falls back to HTTP only after HTTPS fails. When a failure occurs, in many cases it is unclear if it is HTTP or HTTPS that failed. Also, I wonder if there should be a way to set Jib to just start with HTTP, instead of having this fallback feature. sendCredentialsOverHttp might make things more confusing when combined with the current behavior of allowInsecureRegistries.

@TadCordle
Copy link
Contributor

I'm going to work on adding a better error message, and we can discuss what to do with sending credentials over HTTP later.

@chanseokoh
Copy link
Member

chanseokoh commented Jul 18, 2018

@Hi-Fi you will be able to send the password over HTTP (which can be seen by anyone in the network) using the new system property sendCredentialsOverHttp (#641) once Jib 0.9.7 is released.

@coollog
Copy link
Contributor

coollog commented Jul 20, 2018

@Hi-Fi version 0.9.7 is released

@Hi-Fi
Copy link
Author

Hi-Fi commented Jul 23, 2018

This works, but seems that value is handled only with "toImage" (push) part.
When pushing, getting the Manifest error because test registry was artifactory (#534).
When pulling, getting just error:
[ERROR] Failed to execute goal com.google.cloud.tools:jib-maven-plugin:0.9.7:build (default) on project jib-demo-project: Build image failed, perhaps you should use a registry that supports HTTPS so credentials can be sent safely, or set the 'sendCredentialsOverHttp' system property to true: Required credentials for test-artifactory-registry/testProject/distroless-java were not sent because the connection was over HTTP -> [Help 1]

When pulling from local registry and pushing to artifactory, Manifest error is shown.

@coollog
Copy link
Contributor

coollog commented Jul 24, 2018

@Hi-Fi - what is the command you used to run your build? It looks like sendCredentialsOverHttp was not set to true based on the error message.

@Hi-Fi
Copy link
Author

Hi-Fi commented Jul 25, 2018

It was set, only change that I did to make that work was to add source image to other with "-DfromImage=localhost:5000/test/distroless-java"

@Hi-Fi
Copy link
Author

Hi-Fi commented Aug 6, 2018

0.9.8 seems have fixed this, so now authentication is sent to both directions correctly.

@chanseokoh
Copy link
Member

chanseokoh commented Aug 6, 2018

FTR:

Not verified, but I'm guessing it is #704 that solved #599 (comment). That is, previously with 0.9.7, even if sendCredentialsOverHttp is set, if the server responds with 401 Unauthorized on plain HTTP, we were errorring by throwing RegistryCredentialsNotSentException instead of throwing RegistryUnauthorizedException.

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

No branches or pull requests

4 participants