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

Certs.d manpage & *.5.md suffix #594

Merged
merged 2 commits into from Mar 6, 2019
Merged

Conversation

vrothberg
Copy link
Member

@vrothberg
Copy link
Member Author

Ah, the base branch was wrong. Rebasing now.

Addresses: https://bugzilla.redhat.com/show_bug.cgi?id=1677264
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Mar 1, 2019

LGTM

@rhatdan
Copy link
Member

rhatdan commented Mar 1, 2019

@TomSweeneyRedHat PTAL

```
$ openssl genrsa -out client.key 4096
$ openssl req -new -x509 -text -key client.key -out client.cert
```
Copy link
Member

Choose a reason for hiding this comment

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

My only question is either of these examples valid to get a Container Registry up and running? The one that I cobbled together with @edsantiago 's help is:

openssl req -newkey rsa:4096 -nodes -sha256 -keyout /root/auth/domain.key -x509 -days 2 -out /root/auth/domain.crt -subj "/C=US/ST=Foo/L=Bar/O=Red Hat, Inc./CN=localhost"

I don't know if that's entirely up to snuff, you'd definitely want to tweak the /root/auth and Red Hat, but I think it may be closer.

Copy link
Member Author

@vrothberg vrothberg Mar 1, 2019

Choose a reason for hiding this comment

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

They do the same, but your command generates the key in the call -newkey rsa:4096 and does not prompt the user to enter data such as country, province, organization, etc. as those are set in the subject.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Neither of these is really likely to work: if the server is requiring client certificates, it can’t just accept any certificate any attacker can trivially generate like this!

An accepted certificate typically needs to be signed by a server-trusted CA (or, maybe better, a single-purpose CA created specifically for creating certificates accepted by that one server), and creating such a key+certificate pair is therefore somewhat CA-specific.

(Maybe for local testing one can get away with generating a single cert+key pair, and configuring the server to accept that single certificate (instead of a CA that signs other certificates); but that’s very helpful for any kind of production use.)

Overall I’d suggest removing this section completely.

Copy link
Member Author

Choose a reason for hiding this comment

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

Overall I’d suggest removing this section completely.

done

Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

One example suggestion for consideration, otherwise LGTM

```
$ openssl genrsa -out client.key 4096
$ openssl req -new -x509 -text -key client.key -out client.cert
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Neither of these is really likely to work: if the server is requiring client certificates, it can’t just accept any certificate any attacker can trivially generate like this!

An accepted certificate typically needs to be signed by a server-trusted CA (or, maybe better, a single-purpose CA created specifically for creating certificates accepted by that one server), and creating such a key+certificate pair is therefore somewhat CA-specific.

(Maybe for local testing one can get away with generating a single cert+key pair, and configuring the server to accept that single certificate (instead of a CA that signs other certificates); but that’s very helpful for any kind of production use.)

Overall I’d suggest removing this section completely.

docs/containers-certs.d.5.md Outdated Show resolved Hide resolved
docs/containers-certs.d.5.md Outdated Show resolved Hide resolved
docs/containers-certs.d.5.md Outdated Show resolved Hide resolved
Suffix the manpages accordingly to be placed in the correct manpage
folder during package installation.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
@vrothberg
Copy link
Member Author

Updated. @mtrmac, can you have another look?

@mtrmac
Copy link
Collaborator

mtrmac commented Mar 6, 2019

LGTM. Thanks!

@mtrmac mtrmac merged commit 8e82e04 into containers:master Mar 6, 2019
wking added a commit to wking/containers-image that referenced this pull request Jun 1, 2019
Catching up with be91505 (docs: rename manpages to *.5.md, 2019-03-01, containers#594).

Generated with:

  $ sed -i 's/policy.json.md/containers-policy.json.5.md/g' $(git grep -l policy.json.md)
rhatdan pushed a commit to rhatdan/image that referenced this pull request Jul 21, 2019
Catching up with be91505 (docs: rename manpages to *.5.md, 2019-03-01, containers#594).

Generated with:

  $ sed -i 's/policy.json.md/containers-policy.json.5.md/g' $(git grep -l policy.json.md)

Looking to carry this over the finish line for Wking.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
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.

None yet

4 participants