Added support for cert-only login without user and password #121
Conversation
|
Thanks for this. Since the hang on unnegotiated SASL is serious, can you please include a regression test that exercises that behavior that this patch would fix? Underscores aren't used in exported functions. Also we won't need a new Dial, given that DialConfig can be used in this case. |
|
Ah yes, regression test... During weekend I'll prepare minimal example exhibiting unwanted behaviour (including RMQ config and all the necessary certs & keys). From server's point of view it looks unverbose:
From client's point of view it hangs in amqp.DialTLS() never returning to calling process. As to new DialTLS_CertAuth() I agree, you can manually prepare DialConfig - which makes Dial and DialTLS obsolete as well, according to the very rule. Patching of auth.go can also be avoided by defining CertAuth mechanism in client code. Still, userless logging-on via certificate is supported by RMQ natively and as such - it would be nice to have a DialXXX specific for this case. Non-native solutions with custom EXTERNAL methods per-definition need not be supported - but ssl_cert_login does not belong to this class (IMHO). Thanks for response. P.S. How should I provide test material? Commit to my fork? |
|
In a discussion with @streadway and @gerhard we agreed to make this an example/extend the API to make existing |
|
Looks like |
We'd like to add more TLS-related examples, e.g. for #121, and duplicating the comments would be unfortunate.
|
We agreed on |
We'd like to add more TLS-related examples, e.g. for streadway#121, and duplicating the comments would be unfortunate.
|
@streadway @michaelklishin wouldn't usage of Options pattern help to not fall with different suffixes to what do I mean // Option callback for connection option
type Option func(*impl) error
// adding Options templates does not introduce compatibility issues
// as Dial(url) is still valid but adds flexibility in configuring AMQP connection as default
// configuration parameters still taking place
func Dial(url string, opts ...Option) (*Connection, error) {
config := Config{
Heartbeat: defaultHeartbeat,
Locale: defaultLocale,
}
return DialConfig(url, )
}
// SetOptions set amqp connection options
func (a *Config) SetOptions(opts ...Option) error {
for _, opt := range opts {
if err := opt(a); err != nil {
return err
}
}
return nil
}
func TLS(val *tls.Config) Option {
return func(t * Config) error {
t.TLSClientConfig = val
return nil
}
}
func Auth(val []amqp.Authentication) Option {
return func(t *Config) error {
t.SASL = val
return nil
}
}
// other parameters like connection timeout can be set through options toofrom using prospective instead if t.cfg.TLS {
conn = amqp.DialTLS(url, t.cfg.TLS)
} else {
conn = amqp.Dial(url)
}can be as conn = amqp.Dial(url, amqp.TLS(t.cfg.TLS), amqp.Auth(t.cfg.Auth))All above is pseudo-code though we have done similar thing by making wrapper on top of |
|
When is this getting merged, I need this. |
|
@prateek2408 @troian I don't use Go enough to tell if this pattern the way to go today. It is truly unfortunate that this PR has been sitting there for years. I would welcome any reasonable solution that would allow the users to use the |
| } | ||
|
|
||
| func (me *CertAuth) Response() string { | ||
| return fmt.Sprintf("\000*\000*") |
| @@ -30,8 +30,21 @@ func (me *PlainAuth) Response() string { | |||
| return fmt.Sprintf("\000%s\000%s", me.Username, me.Password) | |||
| } | |||
|
|
|||
| // CertAuth for RabbitMQ-auth-mechanism-ssl. | |||
| type CertAuth struct { | |||
There was a problem hiding this comment.
Can we rename this ExternalAuth to match the RabbitMQ Java library naming convention?
| type CertAuth struct { | ||
| } | ||
|
|
||
| func (me *CertAuth) Mechanism() string { |
There was a problem hiding this comment.
And for naming convention again, @streadway seems to use auth instead of me.
| // Finds the first mechanism preferred by the client that the server supports. | ||
| func pickSASLMechanism(client []Authentication, serverMechanisms []string) (auth Authentication, ok bool) { | ||
|
|
| // DialTLS_CertAuth uses the provided tls.Config when encountering an amqps:// | ||
| // scheme. | ||
| func DialTLS_CertAuth(url string, amqps *tls.Config) (*Connection, error) { | ||
|
|
|
@streadway Can we get this merged ASAP? I've been trying to get EXTERNAL to work for a few days now before finding this PR. @p-kraszewski's solution works perfectly for me, both with correct |
|
Hey folks, I'm posting this on behalf of the core team. As you have noticed, this client hasn't seen a lot of activity recently. Because this client has a long tradition of "no breaking public API changes", certain We would like to thank @streadway Team RabbitMQ has adopted a "hard fork" of this client What do we mean by "hard fork" and what does it mean for you? The entire history of the project What does change is that this new fork will accept reasonable breaking API changes according If your PR hasn't been accepted or reviewed, you are welcome to re-submit it for Note that it is a high season for holidays in some parts of the world, so we may be slower Thank you for using RabbitMQ and contributing to this client. On behalf of the RabbitMQ core team, |
Added new DialTLS_CertAuth function for RabbitMQ configured for EXTERNAL auth with ssl_cert_login plugin for userless/passwordless logons.
As of now, amqp just silently hangs, when server presents authentication metod not supported by amqp (when pickSASLMechanism finds no suitable match). I guess it should bail out with some sort of "No common SASL methods found" message.
Hope this helps,
best regards.