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

doc: rewrite TRC ceremony documentation to include scion-pki #4615

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

oncilla
Copy link
Contributor

@oncilla oncilla commented Sep 11, 2024

Rewrite the TRC ceremony documentation to include the scion-pki tool which is a lot more ergonomic than openssl based approach. The openssl based approach is still kept such that people do not need to trust the distributed scion-pki tool.

Furthermore, the documentation and tests are updated to use openssl 3.0.14.

And finally, the scion-pki tool is extended to support RFC3339 based timestamps when creating TRC payloads for both NotBefore and NotAfter fields. The legacy unix timestamp and duration based validity time are still supported.

@jiceatscion
Copy link
Contributor

This change is Reviewable

Copy link
Contributor

@jiceatscion jiceatscion left a comment

Choose a reason for hiding this comment

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

Nice scrubbing. I honestly did not try to find mistakes in the shell scripts.

Reviewed 21 of 21 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @oncilla)


doc/command/scion-pki/scion-pki_trc_inspect.rst line 8 at r1 (raw file):

---------------------

Represent TRC in a human readable form

did you not mean to replace "human" with "inspect" here too?


doc/command/scion-pki/scion-pki_trc_inspect.rst line 14 at r1 (raw file):

'inspect' outputs the TRC contents in a inspect readable form.

Or is this still supposed to say "in a human readable form". I'm trying to decide if "inspect" is a format (as, may be, the "inspect" plugin of a browser understands?)


scion-pki/conf/trc.go line 55 at r1 (raw file):

	}
	if err := cfg.Validity.Validate(); err != nil {
		return TRC{}, serrors.Wrap("validating validity", err)

I understand it is the literal truth, but, may be we can find something that doesn't sound like a Monty Python line. May be "validating time period" or "validating the "validity" section"?

Code quote:

validating validity

scion-pki/conf/validity.go line 62 at r1 (raw file):

	switch {
	case v.Validity.Duration == 0 && v.NotAfter.Time().IsZero():
		return serrors.New("at least one of 'validity' or 'not_after' must be set")

Do we need to be that precise? How about a single boolean expression and a message saying "Exactly one of..." ?


scion-pki/trcs/human.go line 57 at r1 (raw file):

		Example: fmt.Sprintf(`  %[1]s inspect ISD1-B1-S1.pld.der
  %[1]s inspect ISD1-B1-S1.trc`, pather.CommandPath()),
		Long: `'inspect' outputs the TRC contents in a inspect readable form.

Same question as for the doc: is "inspect" a specific subclass of human readable form, or was this meant to say "human"?

Code quote:

inspect

Copy link
Contributor

@jiceatscion jiceatscion left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @oncilla)

Copy link
Contributor Author

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jiceatscion)


doc/command/scion-pki/scion-pki_trc_inspect.rst line 8 at r1 (raw file):

Previously, jiceatscion wrote…

did you not mean to replace "human" with "inspect" here too?

Done.


doc/command/scion-pki/scion-pki_trc_inspect.rst line 14 at r1 (raw file):

Previously, jiceatscion wrote…

Or is this still supposed to say "in a human readable form". I'm trying to decide if "inspect" is a format (as, may be, the "inspect" plugin of a browser understands?)

Hm. Looks like I auto replaced too much. Rephrased a bit.


scion-pki/conf/trc.go line 55 at r1 (raw file):

Previously, jiceatscion wrote…

I understand it is the literal truth, but, may be we can find something that doesn't sound like a Monty Python line. May be "validating time period" or "validating the "validity" section"?

Done.


scion-pki/conf/validity.go line 62 at r1 (raw file):

Previously, jiceatscion wrote…

Do we need to be that precise? How about a single boolean expression and a message saying "Exactly one of..." ?

Done


scion-pki/trcs/human.go line 57 at r1 (raw file):

Previously, jiceatscion wrote…

Same question as for the doc: is "inspect" a specific subclass of human readable form, or was this meant to say "human"?

Done.

Rewrite the TRC ceremony documentation to include the scion-pki tool
which is a lot more ergonomic than openssl based approach. The openssl
based approach is still kept such that people do not need to trust
the distributed scion-pki tool.

Furthermore, the documentation and tests are updated to use openssl
3.0.14.

And finally, the scion-pki tool is extended to support RFC3339 based
timestamps when creating TRC payloads for both NotBefore and NotAfter
fields. The legacy unix timestamp and duration based validity time are
still supported.
Copy link
Contributor Author

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewed 17 of 21 files at r1, 10 of 10 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @jiceatscion)

Copy link
Contributor

@jiceatscion jiceatscion left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @oncilla)


scion-pki/conf/trc.go line 55 at r2 (raw file):

	}
	if err := cfg.Validity.Validate(); err != nil {
		return TRC{}, serrors.Wrap("validating 'validity' section", err)

How about an article? "validating the 'validity' section"


scion-pki/conf/validity.go line 64 at r2 (raw file):

		return serrors.New("exactly one of 'validity' or 'not_after' must be set")
	case v.Validity.Duration != 0 && !v.NotAfter.Time().IsZero():
		return serrors.New("only one of 'validity' or 'not_after' must be set")

Well. Same here too. There's not reason for the two messages to be different, nor really for using switch. It could be an "if" with a 4-term expression. That's what I meant, at least.

Copy link
Contributor Author

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @oncilla)


scion-pki/conf/validity.go line 64 at r2 (raw file):

Previously, jiceatscion wrote…

Well. Same here too. There's not reason for the two messages to be different, nor really for using switch. It could be an "if" with a 4-term expression. That's what I meant, at least.

Done

Copy link
Contributor

@jiceatscion jiceatscion left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @oncilla)

@oncilla oncilla merged commit a192c66 into scionproto:master Sep 17, 2024
5 checks passed
oncilla added a commit to Anapaya/os-scion that referenced this pull request Sep 19, 2024
…-pki (scionproto#4615)

Rewrite the TRC ceremony documentation to include the scion-pki tool
which is a lot more ergonomic than openssl based approach. The openssl
based approach is still kept such that people do not need to trust the
distributed scion-pki tool.

Furthermore, the documentation and tests are updated to use openssl
3.0.14.

And finally, the scion-pki tool is extended to support RFC3339 based
timestamps when creating TRC payloads for both NotBefore and NotAfter
fields. The legacy unix timestamp and duration based validity time are
still supported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants