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

Notation CLI support for Timestamp Authority signatures #59

Open
1 task
iamsamirzon opened this issue Mar 2, 2022 · 16 comments
Open
1 task

Notation CLI support for Timestamp Authority signatures #59

iamsamirzon opened this issue Mar 2, 2022 · 16 comments
Labels
notation-CLI triage Issues for which we need to revisit the right release timeline
Milestone

Comments

@iamsamirzon
Copy link
Contributor

iamsamirzon commented Mar 2, 2022

Summary - Notation client to support TSA signatures and verification support as per RFC 3161
Intended Outcome - The implementation matches with the specification

@iamsamirzon iamsamirzon added this to the RC-2 milestone Mar 2, 2022
@iamsamirzon
Copy link
Contributor Author

@gokarnm - This is the roadmap item related to timestamping.

@iamsamirzon
Copy link
Contributor Author

@FeynmanZhou - We discussed this in our NV2 community meeting today. Propose we include the "Sign" part in RC-1 and the "Verify" part of it can come in "RC-2". Do discuss with @shizhMSFT on it.

@FeynmanZhou
Copy link
Member

@iamsamirzon
Copy link
Contributor Author

Yes, there was support added back in Alpha-1. This roadmap item is to ensure the implementation ( along with tests) meets the agreed on spec

@FeynmanZhou
Copy link
Member

Yes, there was support added back in Alpha-1. This roadmap item is to ensure the implementation ( along with tests) meets the agreed on spec

Okay, we need to verify it for the next step.

@gokarnm
Copy link
Contributor

gokarnm commented Jun 20, 2022

There are open questions and work related to

  • The default TSA to use during signing
  • Distribute public TSA roots in a default named trust store x509/tsa/public-tsa
  • Improvements to custom CMS verification code for TSA verification

@iamsamirzon
Copy link
Contributor Author

@shizhMSFT , @dtzar - We need to bring in the signing part back into RC-1. This item is not yet complete for RC-1.

@dtzar
Copy link
Contributor

dtzar commented Jul 13, 2022

Looks like this work could be included in whomever implements notaryproject/notation-go#78
I would recommend putting the three bullets from @gokarnm above either into that issue or a separate issue(s) depending on the rough size of the work to be done.

@dtzar
Copy link
Contributor

dtzar commented Jul 13, 2022

This issue also relevant to the completion of this item: notaryproject/notation-go#13

@iamsamirzon
Copy link
Contributor Author

@dtzar - There was an item in the spreadsheet for this , row #22. It is marked green ( to indicate complete), but it is not. @shizhMSFT team was looking to implement this. Lets touch base on this with them

@iamsamirzon iamsamirzon added the triage Issues for which we need to revisit the right release timeline label Jul 14, 2022
@shizhMSFT
Copy link

The default TSA to use during signing

As discussed in previous Notary community meetings, we will not provide a default TSA for signing. Users must specify their trusted TSA when signing.

Distribute public TSA roots in a default named trust store x509/tsa/public-tsa

This item is a successor of distributing roots for x509/ca/....

Improvements to custom CMS verification code for TSA verification

We need more clarification on the "improvements".

@iamsamirzon
Copy link
Contributor Author

@gokarnm , @rgnote - Could you elaborate on the "improvements" #59 (comment)

@iamsamirzon
Copy link
Contributor Author

Based on the agreement in NV2 community call on 12/5/2022, moving this out of RC-2

@iamsamirzon iamsamirzon modified the milestones: RC-2, future Dec 6, 2022
@iamsamirzon iamsamirzon modified the milestones: future, Discuss Jan 31, 2023
@zosocanuck
Copy link

It would be great if we can accelerate TSA signature support for an upcoming release, and as such would like to get feedback around the potential to leverage an existing golang timestamping library to implement this roadmap item.

I have a positive experience with the library and it is even being used by the Sigstore/cosign project.

@priteshbandi

@shizhMSFT
Copy link

The prerequisites of TSA signature support are

  • A go library for RFC 3161 a.k.a. timestamp
  • A go library for RFC 2315 since Timestamp relies on CMS a.k.a PKCS7.

Unfortunately, there are no known reliable mature go libraries implementing RFC 3161 and RFC 2315.

The timestamp library github.com/digitorus/timestamp, which is also used by cosign, is built on top of github.com/digitorus/pkcs7, which is a fork of https://github.com/mozilla-services/pkcs7 with enhanced features (but not security). However, the maturity of those libraries are still in an early stage and should not be used for production.

Here are some code snippets from github.com/digitorus/pkcs7:

  • https://github.com/digitorus/pkcs7/blob/51331ccfc40f27dab73cbc42e99f765f618fca70/ber.go#L57-L75
    func ber2der(ber []byte) ([]byte, error) {
        if len(ber) == 0 {
      	  return nil, errors.New("ber2der: input ber is empty")
        }
        //fmt.Printf("--> ber2der: Transcoding %d bytes\n", len(ber))
        out := new(bytes.Buffer)
    
        obj, _, err := readObject(ber, 0)
        if err != nil {
      	  return nil, err
        }
        obj.EncodeTo(out)
    
        // if offset < len(ber) {
        //	return nil, fmt.Errorf("ber2der: Content longer than expected. Got %d, expected %d", offset, len(ber))
        //}
    
        return out.Bytes(), nil
    }
  • https://github.com/digitorus/pkcs7/blob/51331ccfc40f27dab73cbc42e99f765f618fca70/ber.go#L147-L162
    if tag == 0x1F {
            tag = 0
            for ber[offset] >= 0x80 {
                    tag = tag*128 + ber[offset] - 0x80
                    offset++
                    if offset > berLen {
                            return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
                    }
            }
            // jvehent 20170227: this doesn't appear to be used anywhere...
            //tag = tag*128 + ber[offset] - 0x80
            offset++
            if offset > berLen {
                    return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
            }
    }

Note: Unit tests in github.com/digitorus/pkcs7 are failing due to lack of maintenance :(

@shizhMSFT
Copy link

To ensure security of notation, we need to ensure that we have production-level CMS and Timestamp go libraries (we don't need to implement the full spec but implement what we need).

There was an attempt in notation-core-go but it was at a prototype maturity (insufficient unit tests) and has its own security vulnerability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notation-CLI triage Issues for which we need to revisit the right release timeline
Projects
Status: Todo
Development

No branches or pull requests

6 participants