search

apocas / docker-modem

Docker Remote API network stack driver.
Apache License 2.0
234 stars 112 forks source link

Bug: Authconfig is not base64 urlencoded (but simply base64) #123

Closed yaroslav-codefresh closed 1 year ago

yaroslav-codefresh commented 4 years ago

Docker api docs say that X-Registry-Auth should be base64urlencoded (here), but what I see from the code in the version 3.2.1 the value passed to this header is simply base64 encoded. (here)

This lead our company (codefresh.io) to errors when using credentials from Azure Container Registry for one of the customers.

Please check ASAP

apocas commented 4 years ago

Version 3.2.1 and all versions, this behaviour hasn't changed.

Will look into it when I have time, but feel free to open a PR :)

yaroslav-codefresh commented 4 years ago

sure, here is it: https://github.com/apocas/docker-modem/pull/124

yaroslav-codefresh commented 4 years ago

sorry, seems it's not so easy: other registries fail with such header passed with base64url (seems only acr supports and needs this).

Can you please take further investigations?

pysarenko-bohdan commented 2 years ago

@apocas @yaroslav-codefresh I bumped into this bug too. ) Starts from Docker Engin API 1.39 we can't use "\~" in credentials normally since encode algorithm was changed to ''Base64url". It's related only to "X-Registry-Auth" header. Looks like I found a workaround (took from one of the topics in nodejs issue tracker) and created PR. I tested it with DockerHub credentials only. It works for a normal password (e.g. 08bd4-6bab-4b39-bb07-b2e3e0bf9826) and for passwords containing specific chars (e.g. fsjf\~kjzadf\~!@#$%^&*()zjlfhlj?/_=+). My conclusion that native support in NodeJS of base64url (starts from 14.18.0) and also some separate libraries (base64url or base64-url) doesn't work since they remove trailing "="'s but the Docker Engine API needs a strict string in auth header. It looks like bug in docker engine API too. )

pysarenko-bohdan commented 2 years ago

After that should be updated Dockerod library accordingly.

pasha-codefresh commented 2 years ago

@apocas would be happy if you can review it , because it is affecting our production customers. Thank you

apocas commented 2 years ago

Going to look into this later today :)

pysarenko-bohdan commented 2 years ago

@apocas Good evening. Did you have a chance to look into PR? We are still blocked with that.

pasha-codefresh commented 2 years ago

@apocas could you please check PR :)

pasha-codefresh commented 2 years ago

@apocas thank you for review, is it possible that we can create release on it? so we can ask dockerrode update version of docker-modem

pysarenko-bohdan commented 2 years ago

@apocas I prepared PR for Dockerode too.