All of lore.kernel.org
 help / color / mirror / Atom feed
* URL-safe base64 encoding for keys
@ 2012-07-03 12:22 Wido den Hollander
  2012-07-03 15:00 ` Florian Haas
  2012-07-03 15:35 ` Tommi Virtanen
  0 siblings, 2 replies; 14+ messages in thread
From: Wido den Hollander @ 2012-07-03 12:22 UTC (permalink / raw)
  To: ceph-devel@vger.kernel.org

Hi,

With my CloudStack integration I'm running into a problem with the cephx 
keys due to '/' being possible in the cephx keys.

CloudStack's API expects a URI to be passed when adding a storage pool, 
e.g.:

addStoragePool?uri=rbd://user:cephxkey@monitor.addr/poolname

If 'cephxkey' contains a / the URI parser in Java fails (java.net.URI) 
and splits the URI in the wrong place.

For base64 there is a specification [0] that describes the usage of - 
and _ instead of +  and /

Is there a way that we change the bits in src/common/armor.c and replace 
the + and / for - and _?

Thanks,

Wido

[0]: http://en.wikipedia.org/wiki/Base64#URL_applications

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: URL-safe base64 encoding for keys
  2012-07-03 12:22 URL-safe base64 encoding for keys Wido den Hollander
@ 2012-07-03 15:00 ` Florian Haas
  2012-07-03 15:04   ` Yehuda Sadeh
  2012-07-03 15:35 ` Tommi Virtanen
  1 sibling, 1 reply; 14+ messages in thread
From: Florian Haas @ 2012-07-03 15:00 UTC (permalink / raw)
  To: Wido den Hollander; +Cc: ceph-devel@vger.kernel.org

On Tue, Jul 3, 2012 at 2:22 PM, Wido den Hollander <wido@widodh.nl> wrote:
> Hi,
>
> With my CloudStack integration I'm running into a problem with the cephx
> keys due to '/' being possible in the cephx keys.
>
> CloudStack's API expects a URI to be passed when adding a storage pool,
> e.g.:
>
> addStoragePool?uri=rbd://user:cephxkey@monitor.addr/poolname
>
> If 'cephxkey' contains a / the URI parser in Java fails (java.net.URI) and
> splits the URI in the wrong place.
>
> For base64 there is a specification [0] that describes the usage of - and _
> instead of +  and /
>
> Is there a way that we change the bits in src/common/armor.c and replace the
> + and / for - and _?

FWIW (only semi-related), some S3 clients -- s3cmd from s3tools, for
example -- seem to choke on the forward slash in radosgw
auto-generated secret keys, as well.

Cheers,
Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: URL-safe base64 encoding for keys
  2012-07-03 15:00 ` Florian Haas
@ 2012-07-03 15:04   ` Yehuda Sadeh
  2012-07-03 17:23     ` Florian Haas
  0 siblings, 1 reply; 14+ messages in thread
From: Yehuda Sadeh @ 2012-07-03 15:04 UTC (permalink / raw)
  To: Florian Haas; +Cc: Wido den Hollander, ceph-devel@vger.kernel.org

On Tue, Jul 3, 2012 at 8:00 AM, Florian Haas <florian@hastexo.com> wrote:
> On Tue, Jul 3, 2012 at 2:22 PM, Wido den Hollander <wido@widodh.nl> wrote:
>> Hi,
>>
>> With my CloudStack integration I'm running into a problem with the cephx
>> keys due to '/' being possible in the cephx keys.
>>
>> CloudStack's API expects a URI to be passed when adding a storage pool,
>> e.g.:
>>
>> addStoragePool?uri=rbd://user:cephxkey@monitor.addr/poolname
>>
>> If 'cephxkey' contains a / the URI parser in Java fails (java.net.URI) and
>> splits the URI in the wrong place.
>>
>> For base64 there is a specification [0] that describes the usage of - and _
>> instead of +  and /
>>
>> Is there a way that we change the bits in src/common/armor.c and replace the
>> + and / for - and _?
>
> FWIW (only semi-related), some S3 clients -- s3cmd from s3tools, for
> example -- seem to choke on the forward slash in radosgw
> auto-generated secret keys, as well.
>

With radosgw we actually switch a while back to use the alternative
encoding. If you still have some old access keys, just replace them.

Yehuda

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: URL-safe base64 encoding for keys
  2012-07-03 12:22 URL-safe base64 encoding for keys Wido den Hollander
  2012-07-03 15:00 ` Florian Haas
@ 2012-07-03 15:35 ` Tommi Virtanen
  2012-07-03 19:18   ` Wido den Hollander
  2012-07-04 12:46   ` [PATCH] Generate URL-safe base64 strings " Wido den Hollander
  1 sibling, 2 replies; 14+ messages in thread
From: Tommi Virtanen @ 2012-07-03 15:35 UTC (permalink / raw)
  To: Wido den Hollander; +Cc: ceph-devel@vger.kernel.org

On Tue, Jul 3, 2012 at 5:22 AM, Wido den Hollander <wido@widodh.nl> wrote:
> CloudStack's API expects a URI to be passed when adding a storage pool,
> e.g.:
>
> addStoragePool?uri=rbd://user:cephxkey@monitor.addr/poolname
>
> If 'cephxkey' contains a / the URI parser in Java fails (java.net.URI) and
> splits the URI in the wrong place.

First, I actually agree with you -- urlsafe b64 just makes sense. We'd
have to go through some sort of a transition period, accepting both,
perhaps generating old-style, for some time.

Second, have you tried quoting the unsafe characters? / is %2f, + is
%2b, % is %25.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: URL-safe base64 encoding for keys
  2012-07-03 15:04   ` Yehuda Sadeh
@ 2012-07-03 17:23     ` Florian Haas
  2012-07-03 17:25       ` Yehuda Sadeh
  0 siblings, 1 reply; 14+ messages in thread
From: Florian Haas @ 2012-07-03 17:23 UTC (permalink / raw)
  To: Yehuda Sadeh; +Cc: ceph-devel@vger.kernel.org"

On Tue, Jul 3, 2012 at 5:04 PM, Yehuda Sadeh <yehuda@inktank.com> wrote:
>> FWIW (only semi-related), some S3 clients -- s3cmd from s3tools, for
>> example -- seem to choke on the forward slash in radosgw
>> auto-generated secret keys, as well.
>>
>
> With radosgw we actually switch a while back to use the alternative
> encoding. If you still have some old access keys, just replace them.

Is "a while back" after 0.47.3? Because I was definitely keys with "/"
from that version.

Cheers,
Florian

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: URL-safe base64 encoding for keys
  2012-07-03 17:23     ` Florian Haas
@ 2012-07-03 17:25       ` Yehuda Sadeh
  0 siblings, 0 replies; 14+ messages in thread
From: Yehuda Sadeh @ 2012-07-03 17:25 UTC (permalink / raw)
  To: Florian Haas; +Cc: ceph-devel@vger.kernel.org"

On Tue, Jul 3, 2012 at 10:23 AM, Florian Haas <florian@hastexo.com> wrote:
> On Tue, Jul 3, 2012 at 5:04 PM, Yehuda Sadeh <yehuda@inktank.com> wrote:
>>> FWIW (only semi-related), some S3 clients -- s3cmd from s3tools, for
>>> example -- seem to choke on the forward slash in radosgw
>>> auto-generated secret keys, as well.
>>>
>>
>> With radosgw we actually switch a while back to use the alternative
>> encoding. If you still have some old access keys, just replace them.
>
> Is "a while back" after 0.47.3? Because I was definitely keys with "/"
> from that version.
>
There are the access keys and there are the secrets. The access keys
are now url safe, the secrets are not.

Yehuda

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: URL-safe base64 encoding for keys
  2012-07-03 15:35 ` Tommi Virtanen
@ 2012-07-03 19:18   ` Wido den Hollander
  2012-07-04 12:46   ` [PATCH] Generate URL-safe base64 strings " Wido den Hollander
  1 sibling, 0 replies; 14+ messages in thread
From: Wido den Hollander @ 2012-07-03 19:18 UTC (permalink / raw)
  To: Tommi Virtanen; +Cc: ceph-devel@vger.kernel.org



On 07/03/2012 05:35 PM, Tommi Virtanen wrote:
> On Tue, Jul 3, 2012 at 5:22 AM, Wido den Hollander <wido@widodh.nl> wrote:
>> CloudStack's API expects a URI to be passed when adding a storage pool,
>> e.g.:
>>
>> addStoragePool?uri=rbd://user:cephxkey@monitor.addr/poolname
>>
>> If 'cephxkey' contains a / the URI parser in Java fails (java.net.URI) and
>> splits the URI in the wrong place.
>
> First, I actually agree with you -- urlsafe b64 just makes sense. We'd
> have to go through some sort of a transition period, accepting both,
> perhaps generating old-style, for some time.
>

My thoughts exactly.

> Second, have you tried quoting the unsafe characters? / is %2f, + is
> %2b, % is %25.

Yes, I have thought about that, but it would mean extra docs in the 
CloudStack API docs.

Warning: Please URL-encode your cephx secret since it may contain 
invalid characters

In the WebGUI (which talks to the API) I could do a URL-encode in 
JavaScript and do the decode again in the CloudStack management server, 
but it's not what you want.

So yes, that has crossed my mind, but for now I took the easy way out 
and generated myself a new key which doesn't contain slashes.

Wido

> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] Generate URL-safe base64 strings for keys.
  2012-07-03 15:35 ` Tommi Virtanen
  2012-07-03 19:18   ` Wido den Hollander
@ 2012-07-04 12:46   ` Wido den Hollander
  2012-07-04 15:16     ` Sage Weil
  1 sibling, 1 reply; 14+ messages in thread
From: Wido den Hollander @ 2012-07-04 12:46 UTC (permalink / raw)
  To: ceph-devel; +Cc: Wido den Hollander

By using this we prevent scenarios where cephx keys are not accepted
in various situations.

Replacing the + and / by - and _ we generate URL-safe base64 keys

Signed-off-by: Wido den Hollander <wido@widodh.nl>
---
 src/common/armor.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/common/armor.c b/src/common/armor.c
index d1d5664..7f73da1 100644
--- a/src/common/armor.c
+++ b/src/common/armor.c
@@ -9,7 +9,7 @@
  * base64 encode/decode.
  */
 
-const char *pem_key = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+const char *pem_key = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
 
 static int encode_bits(int c)
 {
@@ -24,9 +24,9 @@ static int decode_bits(char c)
 		return c - 'a' + 26;
 	if (c >= '0' && c <= '9')
 		return c - '0' + 52;
-	if (c == '+')
+	if (c == '+' || c == '-')
 		return 62;
-	if (c == '/')
+	if (c == '/' || c == '_')
 		return 63;
 	if (c == '=')
 		return 0; /* just non-negative, please */
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate URL-safe base64 strings for keys.
  2012-07-04 12:46   ` [PATCH] Generate URL-safe base64 strings " Wido den Hollander
@ 2012-07-04 15:16     ` Sage Weil
  2012-07-04 16:10       ` Wido den Hollander
  0 siblings, 1 reply; 14+ messages in thread
From: Sage Weil @ 2012-07-04 15:16 UTC (permalink / raw)
  To: Wido den Hollander; +Cc: ceph-devel

On Wed, 4 Jul 2012, Wido den Hollander wrote:
> By using this we prevent scenarios where cephx keys are not accepted
> in various situations.
> 
> Replacing the + and / by - and _ we generate URL-safe base64 keys
> 
> Signed-off-by: Wido den Hollander <wido@widodh.nl>

Do already properly decode URL-sage base64 encoding?

sage

> ---
>  src/common/armor.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/common/armor.c b/src/common/armor.c
> index d1d5664..7f73da1 100644
> --- a/src/common/armor.c
> +++ b/src/common/armor.c
> @@ -9,7 +9,7 @@
>   * base64 encode/decode.
>   */
>  
> -const char *pem_key = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
> +const char *pem_key = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
>  
>  static int encode_bits(int c)
>  {
> @@ -24,9 +24,9 @@ static int decode_bits(char c)
>  		return c - 'a' + 26;
>  	if (c >= '0' && c <= '9')
>  		return c - '0' + 52;
> -	if (c == '+')
> +	if (c == '+' || c == '-')
>  		return 62;
> -	if (c == '/')
> +	if (c == '/' || c == '_')
>  		return 63;
>  	if (c == '=')
>  		return 0; /* just non-negative, please */
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate URL-safe base64 strings for keys.
  2012-07-04 15:16     ` Sage Weil
@ 2012-07-04 16:10       ` Wido den Hollander
  2012-07-04 16:18         ` Sage Weil
  0 siblings, 1 reply; 14+ messages in thread
From: Wido den Hollander @ 2012-07-04 16:10 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel


----- Oorspronkelijk bericht -----
> On Wed, 4 Jul 2012, Wido den Hollander wrote:
> > By using this we prevent scenarios where cephx keys are not accepted
> > in various situations.
> > 
> > Replacing the + and / by - and _ we generate URL-safe base64 keys
> > 
> > Signed-off-by: Wido den Hollander <wido@widodh.nl>
> 
> Do already properly decode URL-sage base64 encoding?
> 

Yes, it decodes URL-safe base64 as well.

See the if statements for 62 and 63, + and - are treated equally, just like / and _.

Wido


> sage
> 
> > ---
> > src/common/armor.c |       6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/common/armor.c b/src/common/armor.c
> > index d1d5664..7f73da1 100644
> > --- a/src/common/armor.c
> > +++ b/src/common/armor.c
> > @@ -9,7 +9,7 @@
> > * base64 encode/decode.
> > */
> > 
> > -const char *pem_key =
> > "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
> > +const char *pem_key =
> > "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
> > 
> > static int encode_bits(int c)
> > {
> > @@ -24,9 +24,9 @@ static int decode_bits(char c)
> >         return c - 'a' + 26;
> >     if (c >= '0' && c <= '9')
> >         return c - '0' + 52;
> > -    if (c == '+')
> > +    if (c == '+' || c == '-')
> >         return 62;
> > -    if (c == '/')
> > +    if (c == '/' || c == '_')
> >         return 63;
> >     if (c == '=')
> >         return 0; /* just non-negative, please */
> > -- 
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe ceph-devel"
> > in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at   http://vger.kernel.org/majordomo-info.html
> > 
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at   http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate URL-safe base64 strings for keys.
  2012-07-04 16:10       ` Wido den Hollander
@ 2012-07-04 16:18         ` Sage Weil
  2012-07-05 13:31           ` Wido den Hollander
  0 siblings, 1 reply; 14+ messages in thread
From: Sage Weil @ 2012-07-04 16:18 UTC (permalink / raw)
  To: Wido den Hollander; +Cc: ceph-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2693 bytes --]

On Wed, 4 Jul 2012, Wido den Hollander wrote:
> > On Wed, 4 Jul 2012, Wido den Hollander wrote:
> > > By using this we prevent scenarios where cephx keys are not accepted
> > > in various situations.
> > > 
> > > Replacing the + and / by - and _ we generate URL-safe base64 keys
> > > 
> > > Signed-off-by: Wido den Hollander <wido@widodh.nl>
> > 
> > Do already properly decode URL-sage base64 encoding?
> > 
> 
> Yes, it decodes URL-safe base64 as well.
> 
> See the if statements for 62 and 63, + and - are treated equally, just 
> like / and _.

Oh, got it.  The commit description confused me... I thought this was 
related encoding only.

I think we should break the encode and decode patches into separate 
versions, and apply the decode to a stable branch (argonaut) and the 
encode to the master.  That should avoid most problems with a 
rolling/staggered upgrade...

sage


> 
> Wido
> 
> 
> > sage
> > 
> > > ---
> > > src/common/armor.c |       6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/src/common/armor.c b/src/common/armor.c
> > > index d1d5664..7f73da1 100644
> > > --- a/src/common/armor.c
> > > +++ b/src/common/armor.c
> > > @@ -9,7 +9,7 @@
> > > * base64 encode/decode.
> > > */
> > > 
> > > -const char *pem_key =
> > > "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
> > > +const char *pem_key =
> > > "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
> > > 
> > > static int encode_bits(int c)
> > > {
> > > @@ -24,9 +24,9 @@ static int decode_bits(char c)
> > >         return c - 'a' + 26;
> > >     if (c >= '0' && c <= '9')
> > >         return c - '0' + 52;
> > > -    if (c == '+')
> > > +    if (c == '+' || c == '-')
> > >         return 62;
> > > -    if (c == '/')
> > > +    if (c == '/' || c == '_')
> > >         return 63;
> > >     if (c == '=')
> > >         return 0; /* just non-negative, please */
> > > -- 
> > > 1.7.9.5
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe ceph-devel"
> > > in the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at   http://vger.kernel.org/majordomo-info.html
> > > 
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at   http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate URL-safe base64 strings for keys.
  2012-07-04 16:18         ` Sage Weil
@ 2012-07-05 13:31           ` Wido den Hollander
  2012-07-05 14:31             ` Sage Weil
  0 siblings, 1 reply; 14+ messages in thread
From: Wido den Hollander @ 2012-07-05 13:31 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel



On 04-07-12 18:18, Sage Weil wrote:
> On Wed, 4 Jul 2012, Wido den Hollander wrote:
>>> On Wed, 4 Jul 2012, Wido den Hollander wrote:
>>>> By using this we prevent scenarios where cephx keys are not accepted
>>>> in various situations.
>>>>
>>>> Replacing the + and / by - and _ we generate URL-safe base64 keys
>>>>
>>>> Signed-off-by: Wido den Hollander <wido@widodh.nl>
>>>
>>> Do already properly decode URL-sage base64 encoding?
>>>
>>
>> Yes, it decodes URL-safe base64 as well.
>>
>> See the if statements for 62 and 63, + and - are treated equally, just
>> like / and _.
>
> Oh, got it.  The commit description confused me... I thought this was
> related encoding only.
>
> I think we should break the encode and decode patches into separate
> versions, and apply the decode to a stable branch (argonaut) and the
> encode to the master.  That should avoid most problems with a
> rolling/staggered upgrade...

I just submitted a patch for decoding only.

During some tests I did I found out that libvirt uses GNUlib and won't 
handle URL-safe base64 encoded keys.

So, as long as Ceph allows them we're good. Users can always replace the 
+ and / in their key knowing it will be accepted by Ceph.

This works for me for now. The exact switch to base64url should be done 
at a later stage I think.

The RFC on this: http://tools.ietf.org/html/rfc4648#page-7

Wido

>
> sage
>
>
>>
>> Wido
>>
>>
>>> sage
>>>
>>>> ---
>>>> src/common/armor.c |       6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/common/armor.c b/src/common/armor.c
>>>> index d1d5664..7f73da1 100644
>>>> --- a/src/common/armor.c
>>>> +++ b/src/common/armor.c
>>>> @@ -9,7 +9,7 @@
>>>> * base64 encode/decode.
>>>> */
>>>>
>>>> -const char *pem_key =
>>>> "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
>>>> +const char *pem_key =
>>>> "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
>>>>
>>>> static int encode_bits(int c)
>>>> {
>>>> @@ -24,9 +24,9 @@ static int decode_bits(char c)
>>>>          return c - 'a' + 26;
>>>>      if (c >= '0' && c <= '9')
>>>>          return c - '0' + 52;
>>>> -    if (c == '+')
>>>> +    if (c == '+' || c == '-')
>>>>          return 62;
>>>> -    if (c == '/')
>>>> +    if (c == '/' || c == '_')
>>>>          return 63;
>>>>      if (c == '=')
>>>>          return 0; /* just non-negative, please */
>>>> --
>>>> 1.7.9.5
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel"
>>>> in the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at   http://vger.kernel.org/majordomo-info.html
>>>>
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at   http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate URL-safe base64 strings for keys.
  2012-07-05 13:31           ` Wido den Hollander
@ 2012-07-05 14:31             ` Sage Weil
  2012-07-06  8:48               ` Wido den Hollander
  0 siblings, 1 reply; 14+ messages in thread
From: Sage Weil @ 2012-07-05 14:31 UTC (permalink / raw)
  To: Wido den Hollander; +Cc: ceph-devel

On Thu, 5 Jul 2012, Wido den Hollander wrote:
> On 04-07-12 18:18, Sage Weil wrote:
> > On Wed, 4 Jul 2012, Wido den Hollander wrote:
> > > > On Wed, 4 Jul 2012, Wido den Hollander wrote:
> > > > > By using this we prevent scenarios where cephx keys are not accepted
> > > > > in various situations.
> > > > > 
> > > > > Replacing the + and / by - and _ we generate URL-safe base64 keys
> > > > > 
> > > > > Signed-off-by: Wido den Hollander <wido@widodh.nl>
> > > > 
> > > > Do already properly decode URL-sage base64 encoding?
> > > > 
> > > 
> > > Yes, it decodes URL-safe base64 as well.
> > > 
> > > See the if statements for 62 and 63, + and - are treated equally, just
> > > like / and _.
> > 
> > Oh, got it.  The commit description confused me... I thought this was
> > related encoding only.
> > 
> > I think we should break the encode and decode patches into separate
> > versions, and apply the decode to a stable branch (argonaut) and the
> > encode to the master.  That should avoid most problems with a
> > rolling/staggered upgrade...
> 
> I just submitted a patch for decoding only.

Applied, thanks!

> During some tests I did I found out that libvirt uses GNUlib and won't handle
> URL-safe base64 encoded keys.
> 
> So, as long as Ceph allows them we're good. Users can always replace the + and
> / in their key knowing it will be accepted by Ceph.
> 
> This works for me for now. The exact switch to base64url should be done at a
> later stage I think.
> 
> The RFC on this: http://tools.ietf.org/html/rfc4648#page-7

We could:
 - submit a patch for gnulib; someday it'll support it
 - kludge the secret generation code in ceph so that it rejects secrets 
   with problematic encoding... :/  (radosgw-admin does something 
   similar with +'s in the s3-style user keys.)

sage



> 
> Wido
> 
> > 
> > sage
> > 
> > 
> > > 
> > > Wido
> > > 
> > > 
> > > > sage
> > > > 
> > > > > ---
> > > > > src/common/armor.c |       6 +++---
> > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/src/common/armor.c b/src/common/armor.c
> > > > > index d1d5664..7f73da1 100644
> > > > > --- a/src/common/armor.c
> > > > > +++ b/src/common/armor.c
> > > > > @@ -9,7 +9,7 @@
> > > > > * base64 encode/decode.
> > > > > */
> > > > > 
> > > > > -const char *pem_key =
> > > > > "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
> > > > > +const char *pem_key =
> > > > > "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
> > > > > 
> > > > > static int encode_bits(int c)
> > > > > {
> > > > > @@ -24,9 +24,9 @@ static int decode_bits(char c)
> > > > >          return c - 'a' + 26;
> > > > >      if (c >= '0' && c <= '9')
> > > > >          return c - '0' + 52;
> > > > > -    if (c == '+')
> > > > > +    if (c == '+' || c == '-')
> > > > >          return 62;
> > > > > -    if (c == '/')
> > > > > +    if (c == '/' || c == '_')
> > > > >          return 63;
> > > > >      if (c == '=')
> > > > >          return 0; /* just non-negative, please */
> > > > > --
> > > > > 1.7.9.5
> > > > > 
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe ceph-devel"
> > > > > in the body of a message to majordomo@vger.kernel.org
> > > > > More majordomo info at   http://vger.kernel.org/majordomo-info.html
> > > > > 
> > > > > 
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at   http://vger.kernel.org/majordomo-info.html
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Generate URL-safe base64 strings for keys.
  2012-07-05 14:31             ` Sage Weil
@ 2012-07-06  8:48               ` Wido den Hollander
  0 siblings, 0 replies; 14+ messages in thread
From: Wido den Hollander @ 2012-07-06  8:48 UTC (permalink / raw)
  To: Sage Weil; +Cc: ceph-devel



On 07/05/2012 04:31 PM, Sage Weil wrote:
> On Thu, 5 Jul 2012, Wido den Hollander wrote:
>> On 04-07-12 18:18, Sage Weil wrote:
>>> On Wed, 4 Jul 2012, Wido den Hollander wrote:
>>>>> On Wed, 4 Jul 2012, Wido den Hollander wrote:
>>>>>> By using this we prevent scenarios where cephx keys are not accepted
>>>>>> in various situations.
>>>>>>
>>>>>> Replacing the + and / by - and _ we generate URL-safe base64 keys
>>>>>>
>>>>>> Signed-off-by: Wido den Hollander <wido@widodh.nl>
>>>>>
>>>>> Do already properly decode URL-sage base64 encoding?
>>>>>
>>>>
>>>> Yes, it decodes URL-safe base64 as well.
>>>>
>>>> See the if statements for 62 and 63, + and - are treated equally, just
>>>> like / and _.
>>>
>>> Oh, got it.  The commit description confused me... I thought this was
>>> related encoding only.
>>>
>>> I think we should break the encode and decode patches into separate
>>> versions, and apply the decode to a stable branch (argonaut) and the
>>> encode to the master.  That should avoid most problems with a
>>> rolling/staggered upgrade...
>>
>> I just submitted a patch for decoding only.
>
> Applied, thanks!
>
>> During some tests I did I found out that libvirt uses GNUlib and won't handle
>> URL-safe base64 encoded keys.
>>
>> So, as long as Ceph allows them we're good. Users can always replace the + and
>> / in their key knowing it will be accepted by Ceph.
>>
>> This works for me for now. The exact switch to base64url should be done at a
>> later stage I think.
>>
>> The RFC on this: http://tools.ietf.org/html/rfc4648#page-7
>
> We could:
>   - submit a patch for gnulib; someday it'll support it

I already did, but IF they accept anything else than RFC4648 they'll 
implement a lot of the other format as well. That will be some work.

>   - kludge the secret generation code in ceph so that it rejects secrets
>     with problematic encoding... :/  (radosgw-admin does something
>     similar with +'s in the s3-style user keys.)

Seems the easy way out, but it will work though.

Wido

>
> sage
>
>
>
>>
>> Wido
>>
>>>
>>> sage
>>>
>>>
>>>>
>>>> Wido
>>>>
>>>>
>>>>> sage
>>>>>
>>>>>> ---
>>>>>> src/common/armor.c |       6 +++---
>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/src/common/armor.c b/src/common/armor.c
>>>>>> index d1d5664..7f73da1 100644
>>>>>> --- a/src/common/armor.c
>>>>>> +++ b/src/common/armor.c
>>>>>> @@ -9,7 +9,7 @@
>>>>>> * base64 encode/decode.
>>>>>> */
>>>>>>
>>>>>> -const char *pem_key =
>>>>>> "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
>>>>>> +const char *pem_key =
>>>>>> "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_";
>>>>>>
>>>>>> static int encode_bits(int c)
>>>>>> {
>>>>>> @@ -24,9 +24,9 @@ static int decode_bits(char c)
>>>>>>           return c - 'a' + 26;
>>>>>>       if (c >= '0' && c <= '9')
>>>>>>           return c - '0' + 52;
>>>>>> -    if (c == '+')
>>>>>> +    if (c == '+' || c == '-')
>>>>>>           return 62;
>>>>>> -    if (c == '/')
>>>>>> +    if (c == '/' || c == '_')
>>>>>>           return 63;
>>>>>>       if (c == '=')
>>>>>>           return 0; /* just non-negative, please */
>>>>>> --
>>>>>> 1.7.9.5
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel"
>>>>>> in the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at   http://vger.kernel.org/majordomo-info.html
>>>>>>
>>>>>>
>>>>> --
>>>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>>>> the body of a message to majordomo@vger.kernel.org
>>>>> More majordomo info at   http://vger.kernel.org/majordomo-info.html
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-07-06  9:14 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-03 12:22 URL-safe base64 encoding for keys Wido den Hollander
2012-07-03 15:00 ` Florian Haas
2012-07-03 15:04   ` Yehuda Sadeh
2012-07-03 17:23     ` Florian Haas
2012-07-03 17:25       ` Yehuda Sadeh
2012-07-03 15:35 ` Tommi Virtanen
2012-07-03 19:18   ` Wido den Hollander
2012-07-04 12:46   ` [PATCH] Generate URL-safe base64 strings " Wido den Hollander
2012-07-04 15:16     ` Sage Weil
2012-07-04 16:10       ` Wido den Hollander
2012-07-04 16:18         ` Sage Weil
2012-07-05 13:31           ` Wido den Hollander
2012-07-05 14:31             ` Sage Weil
2012-07-06  8:48               ` Wido den Hollander

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.