eclipse-jkube / jkube

Build and Deploy java applications on Kubernetes
https://www.eclipse.dev/jkube/
Eclipse Public License 2.0
775 stars 521 forks source link

ImageName should throw exception when digest length does not match expected length #2543

Open rohanKanojia opened 10 months ago

rohanKanojia commented 10 months ago

Component

None

Task description

Description

ImageName seems to do a very simple validation on Digest in image name: https://github.com/eclipse/jkube/blob/b47fa4a9ab9a839bd4f8a093b4b0a4d8e9c88e9f/jkube-kit/config/image/src/main/java/org/eclipse/jkube/kit/config/image/ImageName.java#L353

This allows even image names with invalid digest like repo@sha256:ffffffffffffffffffffffffffffffffff to pass validation check.

We need to update the Digest regexp to match what's used in Docker Distribution:

[A-Za-z][A-Za-z0-9]*(?:[-_+.][A-Za-z][A-Za-z0-9]*)*[:][[:xdigit:]]{32,}

Expected Behavior

ImageName digest regexp should be more strict

Acceptance Criteria

ShantKhatri commented 9 months ago

Hi @rohanKanojia, I have tested this part on my local, and I found that the test is failing due to the part [[:xdigit:]] which represents character class that matches any hexadecimal digit. What if we replace [[:xdigit:]] with more generic one [0-9a-fA-F] ? I think it may be possible that POSIX syntax character class is might not be supported with our regexp engine(Might be some version of Java(Java 8) is not supports POSIX character class). Reference: https://www.regular-expressions.info/posixbrackets.html

The updated regexp might be is [A-Za-z][A-Za-z0-9]*(?:[-_+.][A-Za-z][A-Za-z0-9]*)*:[0-9a-fA-F]{32,}

rohanKanojia commented 9 months ago

I'm sorry guys, I rechecked docker sources, the regex mentioned above doesn't seem to be getting used during reference.Parse() call in reference_test.go.

reference.Parse() is relying on opencontains/go-digest library for parsing digest component. It uses regex shown below to validate the digest string:

See opencontains/go-digest/digest.go . Even in docker's distribution/reference.go there is a TODO comment to align it's regexp with this one instead:

[a-z0-9]+(?:[.+_-][a-z0-9]+)*:[a-zA-Z0-9=_-]+

Regarding length validation, there is an explicit check in opencontainers/go-digest/algorithm.go

Maybe we can also add an explicit check that the length of the encoded string should match digest / 4 (assuming it will always be a hex string)?

ShantKhatri commented 9 months ago

Hi @rohanKanojia, If I'm not wrong, now this issue should be or something like this [ImageName should throw exception when digest length not match with expected length].

rohanKanojia commented 9 months ago

@ShantKhatri : Thanks for pointing out! I've updated the title