[PATCH] SSH: Parse OpenSSH formatted public keys

[Jan Cholasta]

Hi,

this patch allows use of OpenSSH formatted public keys in LDAP. This is 
needed to fix https://fedorahosted.org/freeipa/ticket/2932.

Note that this is just a quick fix, only the public key portion of an 
OpenSSH formatted public key is used, options and comments are ignored. 
A full fix will need more extensive changes in the code.

Honza

[Jan Cholasta]

Dne 24.8.2012 10:04, Jan Cholasta napsal(a):
> Hi,
>
> this patch allows use of OpenSSH formatted public keys in LDAP. This is
> needed to fix https://fedorahosted.org/freeipa/ticket/2932.
>
> Note that this is just a quick fix, only the public key portion of an
> OpenSSH formatted public key is used, options and comments are ignored.
> A full fix will need more extensive changes in the code.
>
> Honza

Self-NACK, the parser is not very robust, I need to improve it.

Honza

[Jan Cholasta]

Dne 24.8.2012 12:01, Jan Cholasta napsal(a):
> Dne 24.8.2012 10:04, Jan Cholasta napsal(a):
> > Hi,
> >
> > this patch allows use of OpenSSH formatted public keys in LDAP. This is
> > needed to fix https://fedorahosted.org/freeipa/ticket/2932.
> >
> > Note that this is just a quick fix, only the public key portion of an
> > OpenSSH formatted public key is used, options and comments are ignored.
> > A full fix will need more extensive changes in the code.
> >
> > Honza
>
> Self-NACK, the parser is not very robust, I need to improve it.
>
> Honza

Updated the patch & split it in two.

Honza

[Jan Cholasta]

Dne 27.8.2012 11:36, Jan Cholasta napsal(a):
> Dne 24.8.2012 12:01, Jan Cholasta napsal(a):
> > Dne 24.8.2012 10:04, Jan Cholasta napsal(a):
> > > Hi,
> > >
> > > this patch allows use of OpenSSH formatted public keys in LDAP. This is
> > > needed to fix https://fedorahosted.org/freeipa/ticket/2932.
> > >
> > > Note that this is just a quick fix, only the public key portion of an
> > > OpenSSH formatted public key is used, options and comments are ignored.
> > > A full fix will need more extensive changes in the code.
> > >
> > > Honza
> >
> > Self-NACK, the parser is not very robust, I need to improve it.
> >
> > Honza
>
> Updated the patch & split it in two.
>
> Honza

Made several improvements to the parser to better match OpenSSH's behavior:
   - do not allow newlines in the string
   - allow non-ASCII and control characters in option values and comments
   - allow backslash-escaped quotes in option values

Honza

[Jakub Hrozek]

On Mon, Aug 27, 2012 at 04:08:48PM +0200, Jan Cholasta wrote:
> Dne 27.8.2012 11:36, Jan Cholasta napsal(a):
> > Dne 24.8.2012 12:01, Jan Cholasta napsal(a):
> > > Dne 24.8.2012 10:04, Jan Cholasta napsal(a):
> > > > Hi,
> > > >
> > > > this patch allows use of OpenSSH formatted public keys in LDAP. This is
> > > > needed to fix https://fedorahosted.org/freeipa/ticket/2932.
> > > >
> > > > Note that this is just a quick fix, only the public key portion of an
> > > > OpenSSH formatted public key is used, options and comments are ignored.
> > > > A full fix will need more extensive changes in the code.
> > > >
> > > > Honza
> > >
> > > Self-NACK, the parser is not very robust, I need to improve it.
> > >
> > > Honza
> >
> > Updated the patch & split it in two.
> >
> > Honza
>
> Made several improvements to the parser to better match OpenSSH's behavior:
> do not allow newlines in the string
> allow non-ASCII and control characters in option values and comments
> allow backslash-escaped quotes in option values
>
> Honza

We were discussing the patches with Honza off-list. In general I'm OK
with them, but we were talking about creating a simple Python binding so
that the regular expression could be reused in FreeIPA, too.

The Python bindings would also allow for a unit test, which would be
very welcome judging by the complexity of the regular expression.

[Jan Cholasta]

Dne 28.8.2012 14:15, Jakub Hrozek napsal(a):
> On Mon, Aug 27, 2012 at 04:08:48PM +0200, Jan Cholasta wrote:
> > Dne 27.8.2012 11:36, Jan Cholasta napsal(a):
> > > Dne 24.8.2012 12:01, Jan Cholasta napsal(a):
> > > > Dne 24.8.2012 10:04, Jan Cholasta napsal(a):
> > > > > Hi,
> > > > >
> > > > > this patch allows use of OpenSSH formatted public keys in LDAP. This is
> > > > > needed to fix https://fedorahosted.org/freeipa/ticket/2932.
> > > > >
> > > > > Note that this is just a quick fix, only the public key portion of an
> > > > > OpenSSH formatted public key is used, options and comments are ignored.
> > > > > A full fix will need more extensive changes in the code.
> > > > >
> > > > > Honza
> > > >
> > > > Self-NACK, the parser is not very robust, I need to improve it.
> > > >
> > > > Honza
> > >
> > > Updated the patch & split it in two.
> > >
> > > Honza
> >
> > Made several improvements to the parser to better match OpenSSH's behavior:
> > do not allow newlines in the string
> > allow non-ASCII and control characters in option values and comments
> > allow backslash-escaped quotes in option values
> >
> > Honza
>
> We were discussing the patches with Honza off-list. In general I'm OK
> with them, but we were talking about creating a simple Python binding so
> that the regular expression could be reused in FreeIPA, too.
>
> The Python bindings would also allow for a unit test, which would be
> very welcome judging by the complexity of the regular expression.

I have given this some thought and I think we don't actually need to 
fully parse OpenSSH public keys in SSSD. We can safely differentiate 
between raw public key blob and a textual public key, so all we need to 
do is to check that the textual public key does not contain nulls or 
newlines and let OpenSSH handle the rest.

Full parsing will probably be needed some time in the future. When that 
happens, I will revisit the idea of a python binding.

New patches attached.

Honza

[Jakub Hrozek]

On Mon, Sep 03, 2012 at 11:47:56AM +0200, Jan Cholasta wrote:
> Dne 28.8.2012 14:15, Jakub Hrozek napsal(a):
> > On Mon, Aug 27, 2012 at 04:08:48PM +0200, Jan Cholasta wrote:
> > > Dne 27.8.2012 11:36, Jan Cholasta napsal(a):
> > > > Dne 24.8.2012 12:01, Jan Cholasta napsal(a):
> > > > > Dne 24.8.2012 10:04, Jan Cholasta napsal(a):
> > > > > > Hi,
> > > > > >
> > > > > > this patch allows use of OpenSSH formatted public keys in LDAP. This is
> > > > > > needed to fix https://fedorahosted.org/freeipa/ticket/2932.
> > > > > >
> > > > > > Note that this is just a quick fix, only the public key portion of an
> > > > > > OpenSSH formatted public key is used, options and comments are ignored.
> > > > > > A full fix will need more extensive changes in the code.
> > > > > >
> > > > > > Honza
> > > > >
> > > > > Self-NACK, the parser is not very robust, I need to improve it.
> > > > >
> > > > > Honza
> > > >
> > > > Updated the patch & split it in two.
> > > >
> > > > Honza
> > >
> > > Made several improvements to the parser to better match OpenSSH's behavior:
> > > do not allow newlines in the string
> > > allow non-ASCII and control characters in option values and comments
> > > allow backslash-escaped quotes in option values
> > >
> > > Honza
> >
> > We were discussing the patches with Honza off-list. In general I'm OK
> > with them, but we were talking about creating a simple Python binding so
> > that the regular expression could be reused in FreeIPA, too.
> >
> > The Python bindings would also allow for a unit test, which would be
> > very welcome judging by the complexity of the regular expression.
>
> I have given this some thought and I think we don't actually need to
> fully parse OpenSSH public keys in SSSD. We can safely differentiate
> between raw public key blob and a textual public key, so all we need
> to do is to check that the textual public key does not contain nulls
> or newlines and let OpenSSH handle the rest.
>
> Full parsing will probably be needed some time in the future. When
> that happens, I will revisit the idea of a python binding.
>
> New patches attached.
>
> Honza

I've run a couple of sanity checks and the patches work for me.

Ack

[Jakub Hrozek]

On Mon, Sep 03, 2012 at 08:49:45PM +0200, Jakub Hrozek wrote:
> On Mon, Sep 03, 2012 at 11:47:56AM +0200, Jan Cholasta wrote:
> > Dne 28.8.2012 14:15, Jakub Hrozek napsal(a):
> > > On Mon, Aug 27, 2012 at 04:08:48PM +0200, Jan Cholasta wrote:
> > > > Dne 27.8.2012 11:36, Jan Cholasta napsal(a):
> > > > > Dne 24.8.2012 12:01, Jan Cholasta napsal(a):
> > > > > > Dne 24.8.2012 10:04, Jan Cholasta napsal(a):
> > > > > > >Hi,
> > > > > > >
> > > > > > >this patch allows use of OpenSSH formatted public keys in LDAP. This is
> > > > > > >needed to fix https://fedorahosted.org/freeipa/ticket/2932.
> > > > > > >
> > > > > > >Note that this is just a quick fix, only the public key portion of an
> > > > > > >OpenSSH formatted public key is used, options and comments are ignored.
> > > > > > >A full fix will need more extensive changes in the code.
> > > > > > >
> > > > > > >Honza
> > > > > > >
> > > > > >
> > > > > > Self-NACK, the parser is not very robust, I need to improve it.
> > > > > >
> > > > > > Honza
> > > > >
> > > > > Updated the patch & split it in two.
> > > > >
> > > > > Honza
> > > >
> > > > Made several improvements to the parser to better match OpenSSH's behavior:
> > > > do not allow newlines in the string
> > > > allow non-ASCII and control characters in option values and comments
> > > > allow backslash-escaped quotes in option values
> > > >
> > > > Honza
> > >
> > > We were discussing the patches with Honza off-list. In general I'm OK
> > > with them, but we were talking about creating a simple Python binding so
> > > that the regular expression could be reused in FreeIPA, too.
> > >
> > > The Python bindings would also allow for a unit test, which would be
> > > very welcome judging by the complexity of the regular expression.
> >
> > I have given this some thought and I think we don't actually need to
> > fully parse OpenSSH public keys in SSSD. We can safely differentiate
> > between raw public key blob and a textual public key, so all we need
> > to do is to check that the textual public key does not contain nulls
> > or newlines and let OpenSSH handle the rest.
> >
> > Full parsing will probably be needed some time in the future. When
> > that happens, I will revisit the idea of a python binding.
> >
> > New patches attached.
> >
> > Honza
>
> I've run a couple of sanity checks and the patches work for me.
>
> Ack

Pushed to master.