Added conversion of DER to PEM for verify#235
Added conversion of DER to PEM for verify#235patrickzheng200 wants to merge 2 commits intonotaryproject:mainfrom
Conversation
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
| } | ||
| if !verifier.VerifyOptions.Roots.AppendCertsFromPEM(data) { | ||
| return nil, fmt.Errorf("failed to parse PEM certificate: %q", path) | ||
| certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: data}) |
There was a problem hiding this comment.
Probably we can add a comment to explain that we're trying to convert DER to PEM? But it looks good to me overall.
There was a problem hiding this comment.
Probably we can add a comment to explain that we're trying to convert DER to PEM? But it looks good to me overall.
Sure. Will add a comment to it.
There was a problem hiding this comment.
Did you test it with some corner cases? like empty file, wrong cert file.
There was a problem hiding this comment.
Did you test it with some corner cases? like empty file, wrong cert file.
Yes, both scenarios are covered. Also tested end to end with the example net-monitor image.
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
SteveLasker
left a comment
SteveLasker
left a comment
There was a problem hiding this comment.
Thanks @patrickzheng200, this really helps cleanup the usability of #157
LGTM
|
@patrickzheng200, was this accidentally closed, vs. merged? |
I noticed that pkg/signature/jws.go file has been removed from notation due to this PR , so I closed this one. The original issue has also been closed as DER format certificate can be added to the new trust store. |
|
We are supporting notation verify with binary encoded DER in notation-core-go. |
4 participants
Signed-off-by: Patrick Zheng patrickzheng@microsoft.com
Resolves #157