Comments (10)
How about replacing
if err := c.CheckCert(username, cert); err != nil {
with
if err := c.CheckCert(cert.ValidPrincipals[0], cert); err != nil {
This seems better than a using fixed string since the cert is guaranteed to contain cert.ValidPrincipals[0]
, and effectively bypasses checking principals here while still checking:
CriticalOptions,
ValidPrincipals, revocation, timestamp and the signature of the certificate.
from pam-ussh.
I don't think that works. The overall test becomes:
- the certificate must include
ValidPrincipals[0]
; and - the certificate must include any of
ValidPrincipals[0]
toValidPrincipals[len-1]
But that simplifies to:
- the certificate must include
ValidPrincipals[0]
i.e. the other members of ValidPrincipals are pointless.
from pam-ussh.
My suggestion is to stop checking principals in ssh.CheckCert
by feeding it the first principal in the cert (or an empty string if none are set), but to still do principal validation later in pam-ussh as expected.
I havenโt written a test with this change so I could definitely be missing something obvious.
from pam-ussh.
Oh I understand now, sorry. Yes, that cheat will work, pretty ugly though :-)
from pam-ussh.
pam_ussh.go actually used to do that:
I'm trying to remember why I changed this behavior, 3+ years ago now (the internal commit logs probably have more detailed information, but I don't have access to those anymore :))
from pam-ussh.
@pmoody- that internal commit log reads "check cert is good for the user trying to use it", and wasn't reviewed. Your guess is as good as mine!
from pam-ussh.
I think there is a solution here that could resolve multiple concerns at once:
- I think ussh should not unconditionally require that the local PAM_RUSER username is a principal on the cert. This could be made an option (maybe enabled by default for backward compatibility). This would allow users a lot more flexibility in how they manage their certificates. In particular, it would allow users to forward an agent that only contains the "sudoer" cert if they use ussh with sudo, without forwarding the cert they use to authenticate to sshd (this last option requires a bit of trickery on the client-side, but that's out of scope here).
- There's no need to patch Go's CertChecker to check that a certificate is valid for multiple principals, one can simply call CheckCert for each of these principals. This seems much better than re-implementing this validation logic in ussh, IMO.
- To check that a certificate is valid for all principals (i.e. that it lists no principals) one can just check len(cert.ValidPrincipals) == 0 and then call CheckCert as usual with any principal.
I'm going to work on a PR that does this.
from pam-ussh.
a certificate is valid for all principals (i.e. that it lists no principals)
I confirm that what you says is true for CheckCert, but this is a divergence from OpenSSH behaviour. See sshd_config(5)
where it says:
Note that certificates that lack a list of principals will not be permitted for authentication using TrustedUserCAKeys.
The only way I know to permit authentication where the certificate has no principals, is to add an entry to ~/.ssh/authorized_keys
which specifies cert-authority
without any principals="..."
I think it would be very dangerous to accept a certificate without principals as a sort of super-certificate that matches all principals. For example, if you are using Hashicorp Vault to issue certs, the client can request the set of principals to be included, and each one is checked to see if the user is authorized to request it. However they can request no principals - and if no default principal has been set in the signing role, then they'll get a certificate with no principals.
If pam_ussh were to reject certificates with no principals, I'd be much more comfortable with that.
from pam-ussh.
Oh, that is very interesting. The spec and OpenSSH's implementation are disagreeing. The spec says a certificate with no principals should be valid for all principals: https://cvsweb.openbsd.org/src/usr.bin/ssh/PROTOCOL.certkeys?rev=1.19&content-type=text/x-cvsweb-markup
"valid principals" is a string containing zero or more principals as
strings packed inside it. These principals list the names for which this
certificate is valid; hostnames for SSH_CERT_TYPE_HOST certificates and
usernames for SSH_CERT_TYPE_USER certificates. As a special case, a
zero-length "valid principals" field means the certificate is valid for
any principal of the specified type.
Which I agree is a massive foot-gun given how ssh-keygen will happily create principal-less certs if you omit -n
.
(edit: I have tested that OpenSSH does indeed work as documented, too)
Okay, I'll change course and add rejection of principal-less certs to the PR I have in progress.
from pam-ussh.
The spec says a certificate with no principals should be valid for all principals
Thank you for that! I ought to test it with host certificates too, i.e. whether a host certificate with no principals acts as a "wildcard" that matches all hosts (and would be equally dangerous).
from pam-ussh.
Related Issues (15)
- [TESTING] Create tests using cwrap HOT 2
- Can we have a release? HOT 3
- Restrict sudo command for SSH Certificate ForcedCommand HOT 3
- Exit status when user is not member of group HOT 6
- Clarify interaction with sudoers file HOT 3
- Reporting potential security issues HOT 2
- How security vulnerabilities for this package is tracked in NIST/NVD? HOT 2
- The name is similar to the USSR
- Project still alive? HOT 2
- Is it possible to build it on BSD now?
- Update README to reflect new Go build process
- Documentation should clearly state that the module is only meant for use with sudo, and any other use may be unsafe HOT 1
- Make fails on centos 6 HOT 3
- Please clarify that the client mush use ssh-agent forwarding (ssh -A option).
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. ๐๐๐
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google โค๏ธ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from pam-ussh.