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

Misleading and inconsistent logs #416

Closed
yizha1 opened this issue Jun 26, 2024 · 4 comments · Fixed by #417
Closed

Misleading and inconsistent logs #416

yizha1 opened this issue Jun 26, 2024 · 4 comments · Fixed by #417
Assignees
Labels
bug Something isn't working
Milestone

Comments

@yizha1
Copy link
Contributor

yizha1 commented Jun 26, 2024

What is not working as expected?

The type of trust store is ca. When signature verification failed, the following logs were shown:

DEBU[xxx] Validating cert chain
DEBU[xxx] Validating trust identity
DEBU[xxx] Validating expiry
DEBU[xxx] Validating authentic timestamp
DEBU[xxx] Validating revocation
DEBU[xxx] not using authentic signing time due to error retrieving AuthenticSigningTime, err: authenticSigningTime not found
DEBU[xxx] no verification impacting errors encountered while checking revocation, status is OK
INFO[xxx] payload.TargetArtifact in signature: {MediaType:application/vnd.docker.distribution.manifest.v2+json Digest:sha256:xxx Size:xxx URLs:[] Annotations:map[] Data:[] Platform: ArtifactType:}
INFO[xxx] Target artifact that want to be verified: {MediaType:application/vnd.docker.distribution.manifest.v2+json Digest:sha256:xxx Size:xxx URLs:[] Annotations:map[] Data:[] Platform: ArtifactType:}
WARN[xxx] Signature sha256:xxx failed verification with error: content descriptor mismatch

There are several problems:

  • Log Severity and Reasoning: The log that helped identify the root cause is: WARN[***] Signature sha256:*** failed verification with error: content descriptor mismatch.
    However, it is both a “WARN” and an “ERROR” (from the details). Why use “WARN” instead of “ERROR”?
  • Misleading Log: The following log is misleading:
    DEBU[***] not using authentic signing time due to error retrieving AuthenticSigningTime, err: authenticSigningTime not found
    Although it starts with “DEBU,” it discusses an issue related to “AuthenticSigningTime,” which is not actually the problem for the failure. And this log appears for any successful verification as well.
  • Capitalization Consistency: Most logs start with a capital letter after the log level, but the following logs begin with lowercase letters, which lacks consistency:
    DEBU[xxx] not using authentic signing time due to error retrieving AuthenticSigningTime, err: authenticSigningTime not found
    DEBU[xxx] no verification impacting errors encountered while checking revocation, status is OK
    INFO[xxx] payload.TargetArtifact in signature: {MediaType:application/vnd.docker.distribution.manifest.v2+json Digest:sha256:xxx Size:xxx URLs:[] Annotations:map[] Data:[] Platform:<nil> ArtifactType:}

What did you expect to happen?

Here are my suggestions for the three problems

  • Log Severity and Reasoning:
    • Suggestion: Update the log level to ERRO. The error message content descriptor mismatch can be improved as well, but it is not the main purpose of this issue, it can be done with error message improvements.
  • Misleading Log
    • Suggestion: Rephase the log description as it is not an error
  • Capitalization Consistency:
    • Suggestion: Align the capitalization with all logs.

How can we reproduce it?

For the 1st problem, use oras fetch command to fetch a signature blob from one signed image, and then use oras attach to attach it to a different unsigned image.

For the 2nd and 3rd problems, use notation verify to verify any signed images, no matter if it is successful or failure.

Describe your environment

Linux OS

What is the version of your Notation CLI or Notation Library?

notation v1.1.1

@yizha1 yizha1 added bug Something isn't working triage Needs evaluation for feasibility, timeline, etc. labels Jun 26, 2024
@Two-Hearts
Copy link
Contributor

@yizha1 can you move the issue to notation-go as the confusing log is generated by notation-go library.

@yizha1 yizha1 transferred this issue from notaryproject/notation Jun 26, 2024
@Two-Hearts Two-Hearts self-assigned this Jun 26, 2024
@Two-Hearts
Copy link
Contributor

Should be a quick fix, on my list.

@Two-Hearts
Copy link
Contributor

Two-Hearts commented Jun 26, 2024

@yizha1

Log Severity and Reasoning: The log that helped identify the root cause is: WARN[] Signature sha256: failed verification with error: content descriptor mismatch.
However, it is both a “WARN” and an “ERROR” (from the details). Why use “WARN” instead of “ERROR”?

It's WARN instead of ERROR because there could be multiple signatures for an image. For a certain signature, if its verification fails, the overall verification doesn't have to fail (other signatures may pass the verification). Therefore, using ERROR here maybe not very accurate, because the desired behavior is an overall succeeded verification shouldn't have any 'ERROR' in its log.

@Two-Hearts Two-Hearts linked a pull request Jun 26, 2024 that will close this issue
@Two-Hearts
Copy link
Contributor

Two-Hearts commented Jun 26, 2024

@yizha1

Misleading Log: The following log is misleading:
DEBU[***] not using authentic signing time due to error retrieving AuthenticSigningTime, err: authenticSigningTime not found
Although it starts with “DEBU,” it discusses an issue related to “AuthenticSigningTime,” which is not actually the problem for the failure. And this log appears for any successful verification as well.

This is a tricky one, firstly, this log only appears under signing scheme notary.x509. Secondly, it is not an error now because we haven't supported the RFC 3161 timestamp. Once we support it, this error log would then become valid. Therefore, my suggestion is to keep it as is and revisit it before next release, i.e. v1.2.0-beta.1.

@yizha1 yizha1 removed the triage Needs evaluation for feasibility, timeline, etc. label Jun 28, 2024
@yizha1 yizha1 added this to the 1.3.0 milestone Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants