[PATCH] s4:torture/rpc/backupkey: Require 2048 bit RSA key

Garming Sam garming at catalyst.net.nz
Tue Dec 23 15:09:48 MST 2014


Hi Arvid,

Considering there are actually users who are seeing this, it would be 
nice to see this test and the first patch getting in sometime soon.


Just a few things I noticed:

We're still using classic C style declarations, so

	hx509_context_init(&hctx);

should be after the last declaration.

ndr_err is also unused.


Running the test and failing on current Samba, it appears that 2048 bit 
is surprisingly rare. I would've assumed that it was only off half the 
time based on the final bit, but briefly looking at the default Heimdal 
implementation, the code seems content with providing only roughly the 
required amount of bits. Mostly it returns 2047, but it also returns 
2043 quite a bit and even fewer sometimes.

Assuming Windows accepts more than 2048 bits, I don't know if it might 
be a good idea to increase what we're asking for, or maybe even use a 
different generation scheme. I don't know what kind of implications this 
might have, so someone else should probably comment.

Something else to consider is if the implementation never returns the 
required amount of bits, we might get stuck in the loop waiting for 2048 
bits forever. Placing a limit on this could be useful, although what 
this limit might be isn't exactly obvious since 2048 isn't as common as 
you would think.


Cheers,

Garming Sam


On 24/12/14 07:30, Arvid Requate wrote:
> Hello,
>
> the attached patch extends the torture tests for the backupkey protocol (MS-
> BKRP) implementation to ensure that the generated x509 certificate actually has
> an RSA key with the full 2048 bits required by the protocol (and by Windows
> DPAPI).
>
> This testcase accompanies the patches for the backupkey protocol
> implementation I proposed in July, which are available for review at
>
>   http://repo.or.cz/w/Samba/reqa.git/shortlog/refs/heads/BKRP
>
> Feedback welcome.
> Arvid
>
> On 24/11/14 10:01, Andrew Bartlett wrote:
>> Indeed, the patch was proposed, but didn't get the attention of a
>> suitable reviewer, and didn't have any tests to prevent regressions.
>> Someone needs to take it back up and re-submit.
>
>
> On 07/08/14 12:31, Arvid Requate wrote:
>> Hello,
>>
>> we received positive customer feedback on the effectiveness of my recently
>> posted patches for the backupkey protocol implementation in Samba.
>> The patches are now available via
>>
>> http://repo.or.cz/w/Samba/reqa.git/shortlog/refs/heads/BKRP
>>
>> Feedback appreciated.
>>
>> Cheers,
>> Arvid
>


More information about the samba-technical mailing list