Bug #2261

sshkey replaces + with space

Added by Simon Kuhn about 3 years ago. Updated almost 3 years ago.

Status:Closed Start date:05/14/2009
Priority:Normal Due date:
Assignee:James Turnbull % Done:

0%
Category:ssh
Target version:0.25.0
Affected Puppet version:0.25.0 Branch:
Keywords:
Votes: 0

Description

puppet-0.25.0beta1 ruby-1.8.7.72 OS: Ubuntu 9.04 amd64

This manifest:

class ssh::hostkey {
        @@sshkey { "$hostname": type => rsa, key => $sshrsakey, alias => $fqdn }
        Sshkey <<| |>>
}

yields:

hostname,hostname.domain ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAzbtin9E/YQFWqnO7WSDakQ9HZCkkOpi4Iyrnrd4Y  I6InJJhR/K17bLmQUH6quGJtFZ8Q2rulou9jn4O oAadGuaImFLlVHBIXu/rRCzl1IV22P6m4aW9yMZ0OaehhT0rs jpomrYpD4lcSxhsvDWAccyrHIF3ipdbBWQZsMjO8hUGuvKa68CYd9MLl8Cnb3a h6Un4mGmqUygL56ZhSQ83VI9aCqTv1FRfZr4HAj86fxf204efR T4Q0r8esVjhb0rCBmLVb/PZ8nLsaztGyIC8 ApK8DxsRi7ZRApKSAlJReQOq2mp0v8kg WwTpnz xW7vlGVPSTMka5dx0hrw==

Note that there are spaces. These are +’s in the original file and in facter.

Simply downgrading to 0.24.8 appears to resolve the issue. After downgrading and restarting puppet/puppetmaster:

May 14 00:15:18 ubuntu puppetd[3254]: (//Node[default]/ssh::server/Sshkey[hostname]/key) key changed 'AAAAB3NzaC1yc2EAAAABIwAAAQEAzbtin9E/YQFWqnO7WSDakQ9HZCkkOpi4Iyrnrd4Y I6InJJhR/K17bLmQUH6quGJtFZ8Q2rulou9jn4O oAadGuaImFLlVHBIXu/rRCzl1IV22P6m4aW9yMZ0OaehhT0rs jpomrYpD4lcSxhsvDWAccyrHIF3ipdbBWQZsMjO8hUGuvKa68CYd9MLl8Cnb3a h6Un4mGmqUygL56ZhSQ83VI9aCqTv1FRfZr4HAj86fxf204efR T4Q0r8esVjhb0rCBmLVb/PZ8nLsaztGyIC8 ApK8DxsRi7ZRApKSAlJReQOq2mp0v8kg WwTpnz xW7vlGVPSTMka5dx0hrw==' to 'AAAAB3NzaC1yc2EAAAABIwAAAQEAzbtin9E/YQFWqnO7WSDakQ9HZCkkOpi4Iyrnrd4Y++I6InJJhR/K17bLmQUH6quGJtFZ8Q2rulou9jn4O+oAadGuaImFLlVHBIXu/rRCzl1IV22P6m4aW9yMZ0OaehhT0rs+jpomrYpD4lcSxhsvDWAccyrHIF3ipdbBWQZsMjO8hUGuvKa68CYd9MLl8Cnb3a+h6Un4mGmqUygL56ZhSQ83VI9aCqTv1FRfZr4HAj86fxf204efR+T4Q0r8esVjhb0rCBmLVb/PZ8nLsaztGyIC8+ApK8DxsRi7ZRApKSAlJReQOq2mp0v8kg+WwTpnz+xW7vlGVPSTMka5dx0hrw=='

It would also appear that a 0.24.8 client –> 0.25.0beta1 puppermaster yields the correct results.

History

#1
Updated by James Turnbull about 3 years ago

  • Status changed from Unreviewed to Needs Decision
  • Assignee set to Francois Deppierraz

#2
Updated by James Turnbull almost 3 years ago

  • Category set to ssh
  • Target version set to 0.25.0

#3
Updated by Luke Kanies almost 3 years ago

  • Status changed from Needs Decision to Accepted

#4
Updated by Luke Kanies almost 3 years ago

The only thing that’s changed between 0.24.8 and the current master HEAD is whitespace, so I don’t see how that could have broken it.

And I could have sworn that we’ve seen this problem before, but I can’t find any evidence of it.

#5
Updated by Francois Deppierraz almost 3 years ago

This bug perhaps comes from facter, what if you print the value of $sshrsakey in your recipe ?

#6
Updated by Luke Kanies almost 3 years ago

  • Status changed from Accepted to Needs More Information
  • Assignee changed from Francois Deppierraz to Simon Kuhn

#7
Updated by Brice Figureau almost 3 years ago

Francois Deppierraz wrote:

This bug perhaps comes from facter, what if you print the value of $sshrsakey in your recipe ?

I’m not so sure, without looking at the code, this looks like that when the facts are transmitted as parameters when compiling, they are not url encoded (in an url a + is the encoding for a space), but they are properly decoded.

#8
Updated by Brice Figureau almost 3 years ago

Brice Figureau wrote:

Francois Deppierraz wrote:

This bug perhaps comes from facter, what if you print the value of $sshrsakey in your recipe ?

I’m not so sure, without looking at the code, this looks like that when the facts are transmitted as parameters when compiling, they are not url encoded (in an url a + is the encoding for a space), but they are properly decoded.

In fact, my limited testing show that: URI.escape doesn’t escape ‘+’, but Webrick (at least) parses the query string and apply its own unescape, which converts ‘+’ as space. So, I’m wondering if we shouldn’t escape everything with CGI.escape…

#9
Updated by Simon Kuhn almost 3 years ago

Sorry, I won’t be able to provide additional details from the environment I saw this in. If it is of use, the version of facter remained the same for both versions of puppet tested.

#10
Updated by Luke Kanies almost 3 years ago

  • Status changed from Needs More Information to Accepted
  • Assignee changed from Simon Kuhn to Brice Figureau

Yeah, I think we should CGI escape everything.

#11
Updated by Brice Figureau almost 3 years ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • 3 changed from Unknown to Easy

#12
Updated by Brice Figureau almost 3 years ago

  • Status changed from In Topic Branch Pending Review to Ready For Checkin
  • Assignee changed from Brice Figureau to James Turnbull

James,

The patch is ready to checkin, and reside in my github repository, branch tickets/master/2261 (how original): http://github.com/masterzen/puppet/tree/tickets/master/2261

Thanks, Brice

#13
Updated by James Turnbull almost 3 years ago

  • Status changed from Ready For Checkin to Closed

Pushed in commit:8f8240763b0a8ab74b5b78eeb2372a2aa7848049 in branch master.

Also available in: Atom PDF