Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Some refactors #9

Merged
doodzik merged 7 commits into
mainfrom
refactor
Nov 1, 2021
Merged

Some refactors #9
doodzik merged 7 commits into
mainfrom
refactor

Conversation

@doodzik
Copy link
Copy Markdown

@doodzik doodzik commented Oct 29, 2021

fixes #5

TODO

  • change requires to be relative_requires as per rubygems convention
  • run rubocop

@doodzik doodzik marked this pull request as ready for review October 29, 2021 22:28
@doodzik doodzik force-pushed the refactor branch 2 times, most recently from afcc89b to e656827 Compare October 29, 2021 22:52
@doodzik doodzik changed the title add gemfile class Some refactors to fix #5 Oct 30, 2021
@doodzik doodzik changed the title Some refactors to fix #5 Some refactors Oct 30, 2021
# See the License for the specific language governing permissions and
# limitations under the License.

module Gem
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious: what is the motivation behind declaring this namespace upfront?

Related question: I see you declare the Gem::Sigstore namespace in many other files, and then define a class as class Gem::Sigstore::TheClass. Why not define the class inside nested module blocks?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's to follow the conventions set by rubygems.

connection.post("/api/v1/signingCert", {
publicKey: {
content: Base64.encode64(pub_key),
algorithm: "ecdsa"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's rsa now.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind. Fulcio's api only provides ecdsa. I wonder if another part of our code is wrong, then? I don't really understand what the aglo parameter does here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's the algo used to sign the signing certificate using the root Fulcio cert. If that's the case, then once we start retrieving the root cert via the signing cert's AIA field, we'll need to verify the signature using ecdsa as well.

@doodzik doodzik merged commit 3994ba0 into main Nov 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move the gem signing code under a single entrypoint

2 participants